Re: [PATCH v4 35/35] docs/devel/testing: Add documentation for functional tests

[Daniel P . Berrangé]

On Thu, Aug 29, 2024 at 01:35:21PM +0200, Thomas Huth wrote:
> On 29/08/2024 12.34, Daniel P. Berrangé wrote:
> > On Wed, Aug 21, 2024 at 10:27:36AM +0200, Thomas Huth wrote:
> > > Document the new functional testing framework. The text is originally
> > > based on the Avocado documentation, but heavily modified to match the
> > > new framework.
> > > 
> > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > ---
> > >   docs/devel/testing/functional.rst | 269 ++++++++++++++++++++++++++++++
> > >   docs/devel/testing/index.rst      |   1 +
> > >   docs/devel/testing/main.rst       |  12 ++
> > >   3 files changed, 282 insertions(+)
> > >   create mode 100644 docs/devel/testing/functional.rst
> > 
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > > +The tests should be written in the style of the Python `unittest`_
> > > +framework, using stdio for the TAP protocol. The folder
> > > +``tests/functional/qemu_test`` provides classes (e.g. the 
> > > ``QemuBaseTest``
> > > +and the ``QemuSystemTest`` classes) and utility functions that help
> > > +to get your test into the right shape.
> > 
> > One gotcha when using TAP protocol is that you can't just spew
> > debug info to stdout/stderr. Each line of debug info needs to
> > be prefixed with '#' so it is interpreted as diagnostic output.
> 
> Actually, that's the great thing about pycotap (in comparison to other
> Python TAP implementations that I've seen), it helps you to get this right:
> By instantiating the TAPTestRunner like this:
> 
>     tr = pycotap.TAPTestRunner(message_log = pycotap.LogMode.LogToError,
>                                test_output_log = pycotap.LogMode.LogToError)
> 
> The stdio output gets redirected to stderr. And the meson test runner is
> fine by collecting the error messages from stderr and showing them to the
> user in the right way in case the test failed (and puts them into the log
> file if the test succeeded).

I'm not sure that works in all scenarios. In the patch that converts the
acpi-bits test, I had to add this chunk:

@@ -264,8 +263,12 @@ def generate_bits_iso(self):

         try:
             if os.getenv('V') or os.getenv('BITS_DEBUG'):
-                subprocess.check_call([mkrescue_script, '-o', iso_file,
-                                       bits_dir], stderr=subprocess.STDOUT)
+                proc = subprocess.run([mkrescue_script, '-o', iso_file,
+                                       bits_dir],
+                                      stdout=subprocess.PIPE,
+                                      stderr=subprocess.STDOUT,
+                                      check=True)
+                self.logger.info("grub-mkrescue output %s" % proc.stdout)
             else:
                 subprocess.check_call([mkrescue_script, '-o',
                                       iso_file, bits_dir],

because I saw errors in TAP output parsing when V=1 was set,
due to mkrescue_script printing to the STDOUT file descriptor.

IIUC, I wonder if the pycotap runner is replacing the 'stdout'
python object, but leaving the underlying OS FD 1 open on its
original channel such that it gets inherited by child processes.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|