[Avocado-devel] [RFC] Recursive Test Discovery

Thu Jun 1 20:32:00 UTC 2017

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.
>
> >
> > - 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 --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/avocado-devel/attachments/20170601/4fd24e43/attachment.htm>