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

[Alex Bennée]

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 think trying to do anything more fancy is just going to lead to
frustration later on.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro