Inheriting avocado.test.Test

[Marius Vollmer]

Hi Jan,

as promised, here are some details about the class inheritance trouble I
ran into.

I wanted to have a intermediate class cockpit.CockpitTest that inherits
from avocado.test.Test, and then inherit our actual test classes from
cockpit.CockpitTest.

[ This might or might not be a good idea.  In general I agree that
  inheritance is a weak tool, but let's pretend for now that I really
  need to have this inheritance chain.
]

This is the CockpitTest class:

    # cockpit.py

    from avocado import test

    __all__ = [ 'CockpitTest' ]

    class CockpitTest(test.Test):
        pass

And this was my first try to use it:

    #! /usr/bin/python
    # test.py

    from avocado import job
    from cockpit import CockpitTest

    class MyTest(CockpitTest):
        def action(self):
            self.log.info("Yay!")

    if __name__ == "__main__":
        job.main()

This leads to this error:

    Traceback (most recent call last):
      File "/usr/lib/python2.7/site-packages/avocado/test.py", line 332, in
action
        raise NotImplementedError('Test subclasses must implement an action '
    NotImplementedError: Test subclasses must implement an action method

The reason is that Avocado picks cockpit.CockpitTest as the class to
test, not MyTest.  I believe this is essentially random: Avocado
enumerates all members of "test" and picks the first that is a subclass
of avocado.test.Test, which happens to be CockpitTest.

This variant of test.py fixes the problem:

    #! /usr/bin/python
    # test2.py

    from avocado import job
    import cockpit

    class MyTest(cockpit.CockpitTest):
        def action(self):
            self.log.info("Yay!")

    if __name__ == "__main__":
        job.main()

Now CockpitTest is no longer a member of the "test" module, and Avocado
will not consider it.


Thus, the test resolution algorithm of Avocado seems to lead to
undefined behavior when there is more than one subclass of
avocado.test.Test.

I think the "one class-under-test per module" rule is fine, but maybe
Avocado could warn/fail when there is more than one?

Also, I didn't find a way to control the test resolution and tell
Avocado to ignore CockpitTest, or explicitly tell it to use MyTest.

The test2.py hack will work for now, but maybe Avocado could be improved
by only considering classes that have been defined in the same module
that it calls getmembers on.

https://github.com/avocado-framework/avocado/pull/474

Thanks!

[Lukáš Doktor]

Hello Marius,

you're right about the way Avocado enumerates tests. Right now we're 
still cooking this part and we're inclined to support multiple classes 
the same way unittest does (which means it'd run booth, CockpitTest and 
MyTest). But it's not yet decided so any input is welcome.

I'm not sure what can be done to avoid parsing imported classes as they 
are internally just variables. So the solution with importing cockpit 
instead of CockpitTest is a clean solution and not just workaround.

About the inheritance, yes, it can lead to big and nasty problems. If 
you already have given test structure, you can simply wrap your tests 
inside generic CockpitTest, which executes your routine inside the 
`CockpitTest.action`. (or even better you can identify 
setup/action/cleanup and call them from the `CockpitTest`).

Thank you for the report, we'll take a look and see if there is anything 
we can do to about that. So far I think importing modules instead of 
classes is the right way.

Regards,
Lukáš


Dne 11.3.2015 v 09:03 Marius Vollmer napsal(a):
>  Hi Jan,
>
>  as promised, here are some details about the class inheritance trouble I
>  ran into.
>
>  I wanted to have a intermediate class cockpit.CockpitTest that inherits
>  from avocado.test.Test, and then inherit our actual test classes from
>  cockpit.CockpitTest.
>
>  [ This might or might not be a good idea.  In general I agree that
>     inheritance is a weak tool, but let's pretend for now that I really
>     need to have this inheritance chain.
>  ]
>
>  This is the CockpitTest class:
>
>       # cockpit.py
>
>       from avocado import test
>
>       __all__ = [ 'CockpitTest' ]
>
>       class CockpitTest(test.Test):
>           pass
>
>  And this was my first try to use it:
>
>       #! /usr/bin/python
>       # test.py
>
>       from avocado import job
>       from cockpit import CockpitTest
>
>       class MyTest(CockpitTest):
>           def action(self):
>               self.log.info("Yay!")
>
>       if __name__ == "__main__":
>           job.main()
>
>  This leads to this error:
>
>       Traceback (most recent call last):
>         File "/usr/lib/python2.7/site-packages/avocado/test.py", line 332, in
> action
>           raise NotImplementedError('Test subclasses must implement an action '
>       NotImplementedError: Test subclasses must implement an action method
>
>  The reason is that Avocado picks cockpit.CockpitTest as the class to
>  test, not MyTest.  I believe this is essentially random: Avocado
>  enumerates all members of "test" and picks the first that is a subclass
>  of avocado.test.Test, which happens to be CockpitTest.
>
>  This variant of test.py fixes the problem:
>
>       #! /usr/bin/python
>       # test2.py
>
>       from avocado import job
>       import cockpit
>
>       class MyTest(cockpit.CockpitTest):
>           def action(self):
>               self.log.info("Yay!")
>
>       if __name__ == "__main__":
>           job.main()
>
>  Now CockpitTest is no longer a member of the "test" module, and Avocado
>  will not consider it.
>
>
>  Thus, the test resolution algorithm of Avocado seems to lead to
>  undefined behavior when there is more than one subclass of
>  avocado.test.Test.
>
>  I think the "one class-under-test per module" rule is fine, but maybe
>  Avocado could warn/fail when there is more than one?
>
>  Also, I didn't find a way to control the test resolution and tell
>  Avocado to ignore CockpitTest, or explicitly tell it to use MyTest.
>
>  The test2.py hack will work for now, but maybe Avocado could be improved
>  by only considering classes that have been defined in the same module
>  that it calls getmembers on.
>
>  https://github.com/avocado-framework/avocado/pull/474
>
>  Thanks!
>

[Marius Vollmer]

Lukáš Doktor <ldoktor(a)redhat.com&gt; writes:

>  I'm not sure what can be done to avoid parsing imported classes
> as
>  they are internally just variables. 
You can check the module that they (probably) are defined in with
inspect.getmodule and identify imported classes this way.  That helps in
my case.

Another approach might be to ignore classes that have subclasses of
their own.

Of course, Avocade should also have a way to explicitly specify the
classes to test.

>  So the solution with importing cockpit instead of CockpitTest is a
>  clean solution and not just workaround. 
Hmm, it is a very non-obvious solution, IMO.  I at least had to read the
Avocado sources and sleep on it to come up with it.  It should at least
be documented, I'd say.