[Avocado-devel] [RFC] Recursive Test Discovery

[Lukáš Doktor]

Dne 30.5.2017 v 11:02 Amador Pahim napsal(a):
> 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
>          """
>          ...
I do like this directive.

> 
>    Alternative options here are: 1)command line option; 2)make the
>    recursive discovery the default behavior.
Given the history and complexity of such behavior, I'd only allow this 
when the docstring directive does that. We can think about command-line 
option to change it as well, but I don't think we should change the 
default. We are not running the code, therefor our discovery is 
different. People should cope with that.

> 
> - How deep is the recursion?
>    The proposal is that the recursion goes all the way up to the class
>    inheriting from `avocado.Test`.
>    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.
I don't think we should limit this as dynamic loaders (unittest) always 
go all the way down. Later, though, we could come up with other 
docstring directive to break the chain if needed, but I'd not give it a 
priority (as people can simply inherit from different classes to get the 
same functionality).

> 
> - 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
> 
My view on this is it should not. If we want this behavior I think we 
should come up with a different tag to break the chain, but as mentioned 
earlier I don't see it as a priority.

Anyway an example to show why I think we should not stop discovering on 
"disable"

```python
class BaseNetworkTests(Test):
     """
     :avocado: disable
     """
     def test_foo(self):
         pass

     def test_bar(self):
         pass

class MyNetworkTests(BaseNetworkTests):
     """
     :avocado: recursive
     """
```

When in one file, running `avocado $that_file` executes only 
MyNetworkTests combining tests from MyNetworkTests and BaseNetworkTests, 
but the BaseNetworkTests should not be executed (as it's disabled).

>    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.
> 
> Expected Results
> ================
> 
> The expected results of this RFC is to have a well defined behavior for
> the recursive discovery feature.
> 
> The expected result of the feature itself is to provide users more
> flexibility when creating the Avocado tests and consequent Avocado
> command lines.
So my expected result would by:

By default - our static analysis which does not descend to parents
On :avocado: recursive - act as dynamic loader (descend all parents of 
all parents till the end of the graph without any exceptions and create 
a set of `test*` classes)

eventually if asked by users, I'd support optional argument to switch 
between those two modes by default (still the docstring takes 
precedence) and if people really wants I'd support another tag to break 
the chain (:avocado: stop-recursion or so).

Anyway I'm open to other suggestions, this is only my basic view,
Lukáš

> 
> Additional Information
> ======================
> 
> Avocado uses only static analysis to examine the files and this feature
> should stick to this principle in its implementation.
> 
> 
> 
> 
> Looking forward to read your comments.
> --
> apahim
> 


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