Re: [Qemu-devel] [PATCH 2/2] BootLinuxConsoleTest: Test the SmartFusion2 board

[Cleber Rosa <crosa@redhat.com>]

On Wed, Jun 05, 2019 at 05:43:23PM -0300, Eduardo Habkost wrote:
> On Wed, Jun 05, 2019 at 12:06:59PM -0400, Cleber Rosa wrote:
> > On Tue, May 21, 2019 at 12:06:35AM +0200, Philippe Mathieu-Daudé wrote:
> > > Similar to the x86_64/pc test, it boots a Linux kernel on an
> > > Emcraft board and verify the serial is working.
> > > 
> > > If ARM is a target being built, "make check-acceptance" will
> > > automatically include this test by the use of the "arch:arm" tags.
> > > 
> > > Alternatively, this test can be run using:
> > > 
> > >   $ avocado run -t arch:arm tests/acceptance
> > >   $ avocado run -t machine:emcraft-sf2 tests/acceptance
> > >
> > 
> > The pattern accepted as a tag currently doesn't include a dash, see:
> > 
> >    https://avocado-framework.readthedocs.io/en/68.0/ReferenceGuide.html#docstring-directives-rules
> > 
> 
> Why isn't this causing test jobs to fail?

Because docstring directives are searched for "gracefully", that is,
if it doesn't match the pattern, it's not considered a docstring
directive.

Also, there's nothing in the jobs that require or check for that
specific machine tag.  The "check-acceptance" rule uses the built
targets, and adds "arch:<$TARGET>" tags to the command line.

Now, if one run this (as exemplified in the commit message):

  $ avocado run -t machine:emcraft-sf2 tests/acceptance

The *tag* failure/limitation would have been noticed.  BTW, it should
be easy to expand the docstring directive pattern and fix this
limitation on newer Avocado versions.

You can also see the parsed key/val tags with:

  $ avocado list -V tests/acceptance

Although only on new Avocado versions (>69.0), you can also see the
parsed key:val tags.

> 
> This patch is already queued on python-next, should I remove it?
>

Or ammended it?  It's a minor issue IMO (s/-/_/).

> 
> > My suggestion is to replace the dash for an underline.  This was done
> > on the s390 test:
> > 
> >    :avocado: tags=machine:s390_ccw_virtio
> 
> I would love if we stopped using the docstring to store
> machine-readable data and used Python class attributes or
> methods, like most Python APIs do.  e.g.:
> 
>   class MyTest(avocado.Test):
>       tags = {'machine':'s390-ccw-virtio'}
> 
>       def test_my_feature(self):
>           ...
>
> 
> I understand the Avocado runner currently tries to not run any
> test code at the "list" step.  But can we have a mode where it
> will actually load the Python module and look at the class
> objects directly, instead of trying to parse the Python code
> without evaluating it?
> 
> > [...]
> 
> -- 
> Eduardo

I get your point, and I'd love to have no "no learning wheels" mode
too.  But given that the "safe loader" was a very basic architecture
design, and we probably wouldn't be dropping it, this means twice the
code to maintain.

I'd be interested in collecting more use cases (besides this s/-/_/)
to better justify such an effort.

Regards,
- Cleber.