[Avocado-devel] [RFC] Recursive Test Discovery
Amador Pahim
apahim at redhat.com
Fri Jun 2 07:20:33 UTC 2017
On Thu, Jun 1, 2017 at 10:32 PM, Lucas Meneghel Rodrigues
<lookkas at gmail.com> wrote:
> My 2 cents, I agree with Cleber here 100%. The current docstring tags are
> more of a workaround, and ideally none of that would be necessary, and it's
> worth to spend more time trying to get rid of it.
>
> About using only AST static analysis, this is the one reason why the
> docstring tags are being used, it this approach is not used then they are
> not necessary to begin with. I believe the current approach can scale up to
> make the tags obsolete, we can see if astroid [1], the base pylint library
> could help us reach this goal.
>
> [1] https://github.com/PyCQA/astroid
>
>
> On Thu, Jun 1, 2017 at 7:22 AM Lukáš Doktor <ldoktor at redhat.com> wrote:
>>
>> 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.
Ok. se we agree in using the docstring ':avocado: recursive' to
trigger the recursion.
>>
>> >
>> > - 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).
Ok, given the feedbacks, seems there's a big preference for the
complete recursion (instead of one-level-up recursion).
>>
>> >
>> > - 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"
Ok, so the agreement is that we don't break the recursion on 'disabe'
docstring. At this point, I don't think we need a new docstring to
break the chain. If someone ask for that in the future, we can
re-visit this topic.
>>
>> ```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).
Ok
>>
>> > 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
>> >
>>
>>
>
More information about the Avocado-devel
mailing list