----- Original Message ----- > From: "Lukáš Doktor" <ldok...@redhat.com> > To: avocado-devel@redhat.com, "Jan Scotka" <jsco...@redhat.com>, "Cleber > Rosa" <cr...@redhat.com>, "Ademar Reis" > <ar...@redhat.com>, "Amador Pahim" <apa...@redhat.com>, "Lucas Meneghel > Rodrigues" <look...@gmail.com> > Sent: Tuesday, January 19, 2016 2:19:00 PM > Subject: RFC: Static analysis vs. import to discover tests > > Hello guys, > > as the topic suggests, we'll be talking about ways to discover whether > given file is an Avocado test, unittest, or simple test. > > > History > ======= > > At the beginning, Avocado used `inspect` to get all module-level classes > and queried if they were inherited from avocado.Test class. This worked > brilliantly, BUT it requires the module, to be actually loaded, which > means the module-level code is executed. This is fine when you intend to > run the test anyway, but it's potentially dangerous, when you just list > the tests. The problem is, that simple "avocado list /" could turn into > 'os.system("rm -rf / --no-preserve-root")' and we clearly don't want that. > > So couple of months back, we started using AST and static analysis to > discover tests. > > The problem is, that even the best static analysis can't cover > everything our users can come up with (for example creating Test classes > dynamically during import). So we added a way to override this behavior, > docstring "avocado: enable" and "avocado: disable". It can't handle > dynamically created `test` methods, but it can help you with inheritance > problems. > > Anyway it's not really friendly and you can't execute inherited `test` > methods, only the ones defined in your class. Recently we had questions > from multiple sources asking "Why is my class not recognized?" Or "Why > is my test executed as SIMPLE test?". > > > My solution > =========== > > I don't like static analysis. It's always tricky, it can never achieve > 100% certainty. So I'm proposing moving back to loading the code, while > keeping the static analysis for listing purposes. So how would that work? > > > avocado list > ------------ > > would use static analysis to discover tests. The output would be: > > ``` > SIMPLE /bin/true > INSTRUMENTED examples/tests/gdbtest.py:GdbTest.test_start_exit > INSTRUMENTED examples/tests/gdbtest.py:GdbTest.test_existing_commands_raw > INSTRUMENTED examples/tests/gdbtest.py:GdbTest.test_existing_commands > INSTRUMENTED > examples/tests/gdbtest.py:GdbTest.test_load_set_breakpoint_run_exit_raw > ?INSTRUMENTED? multiple_inheritance.py:Inherited.* > ?INSTRUMENTED? multiple_inheritance.py:BaseClass.* > ?INSTRUMENTED? library.py:* > ?INSTRUMENTED? simple_script.py:* > ``` > > Where ?xxx? would be yellow and it stands for "It looks like > instrumented test, but I can't tell". > > When the user makes sure, he wants to run those tests and they are not > nasty, he would use `avocado list --unsafe`, which would actually load > the modules and give precise results: > > ``` > SIMPLE /bin/true > INSTRUMENTED examples/tests/gdbtest.py:GdbTest.test_start_exit > INSTRUMENTED examples/tests/gdbtest.py:GdbTest.test_existing_commands_raw > INSTRUMENTED examples/tests/gdbtest.py:GdbTest.test_existing_commands > INSTRUMENTED > examples/tests/gdbtest.py:GdbTest.test_load_set_breakpoint_run_exit_raw > INSTRUMENTED multiple_inheritance.py:Inherited.test1 > INSTRUMENTED multiple_inheritance.py:Inherited.test2 > INSTRUMENTED multiple_inheritance.py:Inherited.test3 > SIMPLE simple_script.py > ``` > > You can see that this removed the `library.py`, it also discovered that > the `multiple_inheritance.py:BaseClass`` is not test. On the other hand, > it discovered that `multiple_inheritance.py:Inherited` holds 3 tests. > This result is, what `avocado run` would actually execute. > > > avocado run > ----------- > > I don't think `avocado run` should support safe and unsafe option, not > even when running `--dry-run`. When one decides to run the tests, he > wants to run them, so I think `avocado run` should always use the unsafe > way using `inspect`. That way there are no surprises and complex > workarounds to get things done. > > > Summary > ======= > > The current way is safe, `avocado list` and `avocado run` are always in > sync, but it's strange to users as they write tests, they use multiple > inheritance and they need BaseClasses. We have a solution, which > requires docstrings, but it's just not flexible enough and can never be > as flexible as actual loading. > > The proposed solution allows using `avocado list` safely, while `avocado > run` would execute what test developer wrote, without trying to be too > smart. The only cons I see is, that in some cases the `avocado list` > might fail to list all tests, but it should always notify about the > possibility. > > I hope it's understandable, feel free to add your comments and other > possible solutions, > > Lukáš >
So the whole proposal, in summary is: * avocado list: uses static based discovery * avocado list --unsafe: discover tests by loading/running Python code * avocado run: discover tests by loading/running Python code, and then, run them the same way. I like that, but I'd also suggest that `avocado run` gets a configuration, item, something like: [runner] discover_safely = False So that, very paranoid users can decide which method to use to find tests before they're set to be run. Having tests executed on a separate process is a very weak "protection", but notice that this may become more important if we choose to implement and support safer *execution* of tests, such as allowing the change of effective user id in the process that runs the tests themselves (think of something like `--run-as foo`). Also, we have to be careful about "discovering" tests "unsafely" on a local system that will eventually be run on a remote system (--remote-*/--vm-*). Thoughts? CR. _______________________________________________ Avocado-devel mailing list Avocado-devel@redhat.com https://www.redhat.com/mailman/listinfo/avocado-devel