Re: [PATCH] gitlab: remove unreliable avocado CI jobs

[Philippe Mathieu-Daudé]

On 13/9/23 11:18, Peter Maydell wrote:
> On Tue, 12 Sept 2023 at 21:00, Thomas Huth <thuth@redhat.com> wrote:
> > Please don't remove the whole job! Just disable the failing tests within the 
> > job, e.g.:
> >
> > diff --git a/tests/avocado/replay_kernel.py b/tests/avocado/replay_kernel.py
> > --- a/tests/avocado/replay_kernel.py
> > +++ b/tests/avocado/replay_kernel.py
> > @@ -503,6 +503,7 @@ def do_test_mips_malta32el_nanomips(self, kernel_path_xz):
> >            console_pattern = 'Kernel command line: %s' % kernel_command_line
> >            self.run_rr(kernel_path, kernel_command_line, console_pattern, 
> > shift=5)
> >
> > +    @skipIf(os.getenv('GITLAB_CI'), 'Skipping unstable test on GitLab')
> >        def test_mips_malta32el_nanomips_4k(self):
> >            """
> >            :avocado: tags=arch:mipsel
>
> Please don't skip unstable tests on gitlab only. If they're
> unstable, then nobody wants to be running them and wondering
> if these are flaky tests or real issues, whether theyr'e doing
> it on gitlab or locally. (I know we already have a lot of these,
> but the effect is that instead of saying 'make check-avocado'
> you have to say 'GITLAB_CI=1 make check-avocado'.)

Good point, I'll simply use:

        @skip('Pending https://gitlab.com/qemu-project/qemu/-/issues/1884')

Looking at other ones:

$ git grep -w @skip tests/avocado/
tests/avocado/machine_sparc_leon3.py:17:    @skip("Test currently broken")
tests/avocado/netdev-ethtool.py:89:    @skip("Incomplete reg 0x00178 
support")
tests/avocado/netdev-ethtool.py:96:    @skip("Incomplete reg 0x00178 
support")
tests/avocado/replay_kernel.py:333:    @skip("Test currently broken") # 
Console stuck as of 5.2-rc1
tests/avocado/replay_kernel.py:368:    @skip("nios2 emulation is buggy 
under record/replay")
tests/avocado/virtio_check_params.py:119:    @skip("break multi-arch CI")

Looking at the first one:

commit 5baecf58ad9fb3ce24d331978526909d0beca482
Author: Philippe Mathieu-Daudé <f4bug@amsat.org>
Date:   Tue Mar 31 12:50:42 2020 +0200

    tests/acceptance/machine_sparc_leon3: Disable HelenOS test

    This test was written/tested around beginning of 2019, but was
    extracted from a bigger series and posted end of June 2019 [*].
    Unfortunately I did not notice commit 162abf1a8 was merged by
    then, which implements the AHB and APB plug and play devices.

    HelenOS 0.6 is expecting the PnP registers to be not implemented
    by QEMU, then forces the discovered AMBA devices (see [2]).

    Before 162abf1a8, the console was displaying:

      HelenOS bootloader, release 0.6.0 (Elastic Horse)
      Built on 2014-12-21 20:17:42 for sparc32
      Copyright (c) 2001-2014 HelenOS project
       0x4000bf20|0x4000bf20: kernel image (496640/128466 bytes)
       0x4002b4f2|0x4002b4f2: ns image (154195/66444 bytes)
       0x4003b87e|0x4003b87e: loader image (153182/66437 bytes)
       0x4004bc03|0x4004bc03: init image (155339/66834 bytes)
       0x4005c115|0x4005c115: locsrv image (162063/70267 bytes)
       0x4006d390|0x4006d390: rd image (152678/65889 bytes)
       0x4007d4f1|0x4007d4f1: vfs image (168480/73394 bytes)
       0x4008f3a3|0x4008f3a3: logger image (158034/68368 bytes)
       0x4009feb3|0x4009feb3: ext4fs image (234510/100301 bytes)
       0x400b8680|0x400b8680: initrd image (8388608/1668901 bytes)
      ABMA devices:
      <1:00c> at 0x80000100 irq 3
      <1:00d> at 0x80000200
      <1:011> at 0x80000300 irq 8
      Memory size: 64 MB

    As of this commit, it is now confused:

      ABMA devices:
      <1:3000> at 0x00000000 irq 0
      <1:3000> at 0x00000000 irq 0
      <1:3000> at 0x00000000 irq 0
      <1:3000> at 0x00000000 irq 0
      <1:3000> at 0x00000000 irq 0
      <1:3000> at 0x00000000 irq 0
      <1:3000> at 0x00000000 irq 0
      ...

    As this test is not working as expected, simply disable it (by
    skipping it) for now.

More than 3 years passed already, what a disappointment.
Offending commit is 4 years old.

commit 162abf1a83ddd06ce1618666f84f88ba4dbffe10
Author: KONRAD Frederic <frederic.konrad@adacore.com>
Date:   Wed May 15 14:31:32 2019 +0200

    leon3: introduce the plug and play mechanism

    This adds the AHB and APB plug and play devices.
    They are scanned during the linux boot to discover the various
    peripheral.

I'm not complaining about that particular commit, I wonder about
usefulness of disabling tests from unmaintained areas.

Maybe we can commit a date when disabling a test, having a disabled
test failing _after_ that date, so if it isn't fixed we remove it.
Smth like,

  @SkipBroken(date='2023-11-15',
              desc='Pending 
https://gitlab.com/qemu-project/qemu/-/issues/1884') # Will fail if run 
after 2023-11-15 and this test isn't fixed

Thoughts?