avocado icon indicating copy to clipboard operation
avocado copied to clipboard

Improve message error when invalid syntax inside instrumented test

Open beraldoleal opened this issue 3 years ago • 3 comments

If you have a test_foo.py, like this:

from avocado import Test

class TransientDomain(Test):
    """Transient domain basic operations.

    The test case validates the core lifecycle and operations on transient
    domains.
    """

    def setUp(self):
        with open("/tmp/debug", "a") as fp 
            fp.write('Starting {}\n'.format(self.id()))
        super().setUp()

    def test_one(self):
        self.assertTrue(1)

    def test_two(self):
        self.assertTrue(1)

And then, when running you will get a generic error:

$ avocado run --test-runner='nrunner' --nrunner-max-parallel-tasks=1 tests/domain/test_foo.py
Could not resolve references: tests/domain/test_foo.py

We could go one step further to show the syntax error here.

beraldoleal avatar May 26 '21 18:05 beraldoleal

@clebergnu I'm hitting this constantly, IMO this has a high priority. What do you think?

beraldoleal avatar Jun 01 '21 19:06 beraldoleal

@beraldoleal I don't object to treating this as a high priority item at all... I just don't have a good feeling about how easy such a solution would turn out to be.

The reason is that while we parse the files that are candidates for containing avocado-instrumented and python-unittest tests, we create an abstract syntax tree. A lot of invalid Python code, that can't be evaluated, will be a valid abstract syntax tree.

For instance, the following gets parsed just fine as an AST:

from avocado import Test

class T(T):
    def test(self):
        pass

But, if evaluated, would complain:

NameError: name 'T' is not defined

The problem with evaluating code is that it basically gets executed, and bad test code, either malicious or buggy, could cause serious damages to people doing avocado list other/people/tests.py.

If you have any ideas, I'd love to hear them.

clebergnu avatar Jun 01 '21 23:06 clebergnu

I see.

We have two issues then: a) identify what is an avocado instrumented test; b) make a better output when running the test;

IMO, we could try to separate the logic here, to avoid executing the code during list. For instance, doing a introspect on all classes and see if is based on avocado.Test to solve a).

Makes sense?

beraldoleal avatar Jun 02 '21 17:06 beraldoleal