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

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Sep 13, 2023 at 11:35:12AM +0100, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
> > 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?
> 
> I like the:
> 
>   @skip('Pending https://gitlab.com/qemu-project/qemu/-/issues/1884')

I'd remove "Pending " too - the URL alone is sufficient info
including the record of dates, to guide us when to finally
delete a skipped test for good.


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