Re: [PATCH] tests: Disable migration-test

[Daniel P . Berrangé]

On Tue, Feb 21, 2023 at 03:36:39PM +0000, Dr. David Alan Gilbert wrote:
> * Peter Maydell (peter.maydell@linaro.org) wrote:
> > On Tue, 21 Feb 2023 at 15:21, Dr. David Alan Gilbert
> > <dgilbert@redhat.com> wrote:
> > > Damn this is really going to impact the stability of migration if we
> > > don't regularly test.
> > > But fundamentally, I've never been able to debug much of the reports
> > > that come from flakyness in gitlab ci; we're not getting the most basic
> > > information like which subtest or where we're upto in the test which
> > > makes it very very hard to debug.
> > 
> > Right, but if you want more information you need to change the
> > tests and/or test harness to provide it.
> 
> I don't think the migration test is doing anything odd in that respect;
> We've just got a bunch of qtest tests; having a test framework which
> doesn't tell you which test failed is very difficult.

Right so the problem here is the use of 'bail out'. From the POV
of the TAP output format that is an immediate termination, so there
is nothing to report about which test was being run.

To make the failing test visible, it needs to NOT trigger a bail
out, but instead report an "not ok" line, which will show the
test case name. AFAIK, this is a limitation of glib's test harness
and its adoption of TAP. You can't get the test case name printed
until the test case is finished. And glib tests fail by calling
assert, so they will inherantly trigger 'bail out' logic from a
TAP POV.

IIRC, the historical non-TAP output format would output the test
case name first, and then once done report ok/not ok.

The workaround would be for glib to use a TAP diagnostic line
to print the test case it is about to run. It already uses the
diagnostic lines to report test groups eg

$ ./build//tests/unit/test-io-channel-command
# random seed: R02S0718b3006d3dcf15099db36b61dff3e8
1..4
# Start of io tests
# Start of channel tests
# Start of command tests
# Start of fifo tests
ok 1 /io/channel/command/fifo/sync
ok 2 /io/channel/command/fifo/async
# End of fifo tests
# Start of echo tests
**
ERROR:../tests/unit/test-io-channel-command.c:102:test_io_channel_command_echo: 
assertion failed: (rand() < 0.5)
Bail out! 
ERROR:../tests/unit/test-io-channel-command.c:102:test_io_channel_command_echo: 
assertion failed: (rand() < 0.5)
Aborted (core dumped)


would have to be changed to report

$ ./build//tests/unit/test-io-channel-command
# random seed: R02S0718b3006d3dcf15099db36b61dff3e8
1..4
# Start of io tests
# Start of channel tests
# Start of command tests
# Start of fifo tests
# Running /io/channel/command/fifo/sync
ok 1 /io/channel/command/fifo/sync
# Running /io/channel/command/fifo/async
ok 2 /io/channel/command/fifo/async
# End of fifo tests
# Start of echo tests
# Running /io/channel/command/echo/sync
**
ERROR:../tests/unit/test-io-channel-command.c:102:test_io_channel_command_echo: 
assertion failed: (rand() < 0.5)
Bail out! 
ERROR:../tests/unit/test-io-channel-command.c:102:test_io_channel_command_echo: 
assertion failed: (rand() < 0.5)
Aborted (core dumped)

so we see exactly what was running.

Without this though, we can still figure out what was running. You
have to look back for the previous 'ok' line and see what it was.
Then locally run '/path/to/test -l' to list the test case names.
The one you want is the one immediately after the last 'ok' (not ok)
line.

Rather tedious for sure, but not impossible.

Worth an RFE for glib, but would be a while before we saw the benefit.

As a workaround, we could print out TAP diagnostic output ourselves
in any qtests that we see as especially unreliable. A diagnostic
output is any line printed on stdout that starts with a '# '

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 :|