[Avocado-devel] [RFC] Recursive Test Discovery

Wed May 31 16:16:56 UTC 2017

On 05/31/2017 10:15 AM, Amador Pahim wrote:
> On Tue, May 30, 2017 at 11:02 AM, Amador Pahim <apahim at redhat.com> wrote:
>> Hello,
>>
>> This came up as an issue and I believe it deserves some discussion as
>> there are some different approaches to implement the feature.
>>
>> Motivation
>> ==========
>>
>> Currently we can discover test classes that does not inherit directly
>> from `avocado.Test`. To do so, we rely on the inclusion of a docstring
>> (`:avocado: enable`) in the mentioned class. Example below.
>>
>> File `/usr/share/avocado/tests/test_base_class.py`::
>>
>>     from avocado import Test
>>
>>
>>     class BaseClass(Test):
>>
>>         def test_basic(self):
>>             pass
>>
>> File `/usr/share/avocado/tests/test_first_child.py`::
>>
>>     from test_base_class import BaseClass
>>
>>
>>     class FirstChild(BaseClass):
>>         """
>>         :avocado: enable
>>         """
>>
>>         def test_first_child(self):
>>             pass
>>
>> In the example above, if we ask Avocado to list the tests from
>> `test_first_child.py`, `FirstChild.test_first_child` will be listed and
>> the `BaseClass.test_basic` won't::
>>
>>     $ avocado list test_first_child.py
>>     INSTRUMENTED test_first_child.py:FirstChild.test_first_child
>>
>> The request is that, in such cases, we have a way to include the
>> `BaseClass.test_basic` into the results.
>>
>> Proposal
>> ========
>>
>> To include the parent classes into the discovery results, we have three
>> main aspects to consider:
>>
>> - How to flag that we want that behaviour?
>>   The proposal is the creation of a new docstring `recursive`. Example::
>>
>>     class FirstChild(BaseClass):
>>         """
>>         :avocado: recursive
>>         """
>>         ...
>>
>>   Alternative options here are: 1)command line option; 2)make the
>>   recursive discovery the default behavior.
>>

I think we have to consider why the request came to be, in the first
place.  The reason is that we're not behaving like standard (Python
unittest.TestCase) classes when looking at the real methods that the
child classes will have.  To make it more clear, these are the methods
that, once instantiated, `FirstChild` will have:

 * test_basic
 * test_first_child

And this is what users would (and should expect).  IMO, the `:avocado:
enable` doscstring directive is nothing but a way to partially address
the limitation Avocado currently has.  Let's make it clear: the
limitation is that Avocado won't look at parent classes, to determine if
they inherit (either directly or indirectly) from `avocado.Test`.

>> - How deep is the recursion?

How deep is the recursion for standard Python classes?  All the way to
the base class.  In that's for each inherited class in the case of
multiple inheritance.

>>   The proposal is that the recursion goes all the way up to the class
>>   inheriting from `avocado.Test`.

Agreed.

>>   Alternative option here is to discover only the first parent of the
>>   class flagged with `recursive`. If the parent class also has the same
>>   docstring, then we go one more level up, and so on.
> 

This is a consistent approach, but it's not natural to Python
developers.  Just like the current state of things is also not natural.

> While working in this specific topic, I noticed that we have to
> support multiple inheritance and, in that case, if we are discovering
> all the chain of parents up to the base class, we will certainly
> follow a chain of classes that are expected to not be Avocado tests.
> So, I think a better idea for a first implementation is to go only one
> level up (first parent) and then, if the parent also has the docstring
> `recursive`, we go one more level up and so on.
> 

Instead of focusing on where to stop and which tags to use, I'd evaluate
if it's possible (or how complex it is) to do a syntactic only parsing
that goes all the way to the root class(es).  If we can do that, than we
can do things the *natural way*.  Even the `:avocado: enable` tag may be
deprecated.

Then, if we want to add *extra* value, we may add other docstring
directives that will prevent the natural behavior.  Something like
`:avocado: local-test-methods-only` would restrict it to the current
class only, discarding what is in the parent classes.

>>
>> - Will the recursion respect the parents docstrings?
>>   The proposal is that we do respect the docstrings in the parents when
>>   recursively discovering. Example:
>>
>>   File `/usr/share/avocado/tests/test_base_class.py`::
>>
>>     from avocado import Test
>>
>>
>>     class BaseClass(Test):
>>
>>         def test_basic(self):
>>             pass
>>
>>   File `/usr/share/avocado/tests/test_first_child.py`::
>>
>>     from test_base_class import BaseClass
>>
>>
>>     class FirstChild(BaseClass):
>>         """
>>         :avocado: recursive
>>         """
>>
>>         def test_first_child(self):
>>             pass
>>
>>   Will result in::
>>
>>     $ avocado list test_first_child.py
>>     INSTRUMENTED test_first_child.py:FirstChild.test_first_child
>>     INSTRUMENTED test_first_child.py:BaseClass.test_basic
>>
>>   While:
>>
>>   File `/usr/share/avocado/tests/test_base_class.py`::
>>
>>     from avocado import Test
>>
>>
>>     class BaseClass(Test):
>>         """
>>         :avocado: disable
>>         """
>>
>>         def test_basic(self):
>>             pass
>>
>>   File `/usr/share/avocado/tests/test_first_child.py`::
>>
>>     from test_base_class import BaseClass
>>
>>
>>     class FirstChild(BaseClass):
>>         """
>>         :avocado: recursive
>>         """
>>
>>         def test_first_child(self):
>>             pass
>>
>>   Will result in::
>>
>>     $ avocado list test_first_child.py
>>     INSTRUMENTED test_first_child.py:FirstChild.test_first_child
>>
>>   The alternative option is that the discovery ignores the parents
>>   docstrings when discovering recursively, meaning that the
>>   `:avocado: disable` (or any other current or future available
>>   docstrings) would have no effect in the recursive discovery.
>>

I believe my previous comments address these, albeit not specifically,
as the topic I consider the most important is whether Avocado can
implement the same standard Python behavior first.

>> Expected Results
>> ================
>>
>> The expected results of this RFC is to have a well defined behavior for
>> the recursive discovery feature.
>>

My understanding is that we should aim towards the natural Python class
behavior.

>> The expected result of the feature itself is to provide users more
>> flexibility when creating the Avocado tests and consequent Avocado
>> command lines.
>>

And I do think within the same work we can improve over the "standard"
behavior, such as listing only local methods as mentioned before, making
Avocado more flexible than other tools.

>> Additional Information
>> ======================
>>
>> Avocado uses only static analysis to examine the files and this feature
>> should stick to this principle in its implementation.

Agreed.

>>
>>
>>
>>
>> Looking forward to read your comments.
>> --
>> apahim
> 

-- 
Cleber Rosa
[ Sr Software Engineer - Virtualization Team - Red Hat ]
[ Avocado Test Framework - avocado-framework.github.io ]
[  7ABB 96EB 8B46 B94D 5E0F  E9BB 657E 8D33 A5F2 09F3  ]

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/avocado-devel/attachments/20170531/5f5d1f3e/attachment.sig>