On 5/12/2026 9:36 AM, Daniel P. Berrangé wrote: > On Tue, May 12, 2026 at 09:19:45AM -0700, Pierrick Bouvier wrote: >> On 5/12/2026 9:06 AM, Daniel P. Berrangé wrote: >>> On Tue, May 12, 2026 at 08:56:54AM -0700, Pierrick Bouvier wrote: >>>> On 4/24/2026 8:42 AM, Daniel P. Berrangé wrote: >>>>> The nature of block I/O tests is such that there can be unexpected false >>>>> positive failures in certain scenarios that have not been encountered >>>>> before, and sometimes non-deterministic failures that are hard to >>>>> reproduce. >>>>> >>>>> Before enabling the I/O tests as gating jobs in CI, there needs to be a >>>>> mechanism to dynamically mark tests as skipped, without having to commit >>>>> code changes. >>>>> >>>>> This introduces the QEMU_TEST_IO_SKIP environment variable that is set >>>>> to a list of FORMAT-OR-PROTOCOL:NAME pairs. The intent is that this >>>>> variable can be set as a GitLab CI pipeline variable to temporarily >>>>> disable a test while problems are being debugged. >>>>> >>>>> Reviewed-by: Thomas Huth <[email protected]> >>>>> Signed-off-by: Daniel P. Berrangé <[email protected]> >>>>> --- >>>>> docs/devel/testing/main.rst | 7 +++++++ >>>>> tests/qemu-iotests/testrunner.py | 16 ++++++++++++++++ >>>>> 2 files changed, 23 insertions(+) >>>>> >>>>> diff --git a/docs/devel/testing/main.rst b/docs/devel/testing/main.rst >>>>> index 797111009a..f779a64415 100644 >>>>> --- a/docs/devel/testing/main.rst >>>>> +++ b/docs/devel/testing/main.rst >>>>> @@ -284,6 +284,13 @@ that are specific to certain cache mode. >>>>> More options are supported by the ``./check`` script, run ``./check -h`` >>>>> for >>>>> help. >>>>> >>>>> +If a test program is known to be broken, it can be disabled by setting >>>>> +the ``QEMU_TEST_IO_SKIP`` environment variable with a list of tests to >>>>> +be skipped. The values are of the form FORMAT-OR-PROTOCOL:NAME, the >>>>> +leading component can be omitted to skip the test for all formats and >>>>> +protocols. For example ``export QEMU_TEST_IO_SKIP="luks:149 185 >>>>> iov-padding`` >>>>> +will skip ``149`` for LUKS only, and ``185`` and ``iov-padding`` for all. >>>>> + >>>>> Writing a new test case >>>>> ~~~~~~~~~~~~~~~~~~~~~~~ >>>>> >>>>> diff --git a/tests/qemu-iotests/testrunner.py >>>>> b/tests/qemu-iotests/testrunner.py >>>>> index dbe2dddc32..ecb5d4529f 100644 >>>>> --- a/tests/qemu-iotests/testrunner.py >>>>> +++ b/tests/qemu-iotests/testrunner.py >>>>> @@ -145,6 +145,18 @@ def __init__(self, env: TestEnv, tap: bool = False, >>>>> >>>>> self._stack: contextlib.ExitStack >>>>> >>>>> + self.skip = {} >>>>> + for rule in os.environ.get("QEMU_TEST_IO_SKIP", "").split(" "): >>>>> + rule = rule.strip() >>>>> + if rule == "": >>>>> + continue >>>>> + if ":" in rule: >>>>> + fmt, name = rule.split(":") >>>>> + if fmt in ("", env.imgfmt, env.imgproto): >>>>> + self.skip[name] = True >>>>> + else: >>>>> + self.skip[rule] = True >>>>> + >>>>> def __enter__(self) -> 'TestRunner': >>>>> self._stack = contextlib.ExitStack() >>>>> self._stack.enter_context(self.env) >>>>> @@ -251,6 +263,10 @@ def do_run_test(self, test: str) -> TestResult: >>>>> description='No qualified output ' >>>>> f'(expected {f_reference})') >>>>> >>>>> + if f_test.name in self.skip: >>>>> + return TestResult(status='not run', >>>>> + description='Listed in QEMU_TEST_IO_SKIP') >>>>> + >>>>> args = [str(f_test.resolve())] >>>>> env = self.env.prepare_subprocess(args) >>>>> >>>> >>>> Why not simply remove the broken tests, and create issues to add them >>>> again in the future? >>> >>> In theory that's what our policy today is, but in practice it is >>> too much of a burden on the release co-ordinator, to expect them >>> to create such a patch themselves, or wait on a subsys maintainer >>> todo it for them. >>> >>> They end up just ignoring brokenness in CI which is a bad practice, >>> and will prevent us ever making CI truely gating or switching to >>> using MRs for pull requests. This gives us a super-fast way to skip >>> flaky tests, while the subsystem maintainers figure out the right >>> permanent answer. >>> >> >> I disagree on this one, merging a single patch doing a git rm, and a git >> revert later is not more expensive than merging a variable modifying a >> variable in a yaml file. > > Any code changes like that need to be sent back to the subsystem > maintainer to be acked. IMHO the release manager should not be > unilaterally deleting tests without peer review. So that's > got a non-negligible turn around time, during which CI is broken. >
I accept the argument, but it seems like a workaround for a human process, more than a proper solution to the problem. It would be better to have a proper policy for build/test fixes, instead of implementing local overrides to this. > Setting an env variable to skip a problematic test is something > reasonable to do with zero oversight. > >> The issue with this approach is that people running tests locally will >> not see which tests are skipped, and will see false positives. So you >> just keep CI green, but not the test base itself. > > I would still expect the release manager to file a bug about any > flaky test they disable via the env var, and the subsystem maintainer > should still be fixing it or disabling it such that tests won't fail > more broadly, or deciding to remove it if terminally broken. > > We're just decoupling the process so that there is an immediate > workaround possible. It can also be used by people working in > their forks - often I've been testing stuff in my fork, but > see spurious failures because git master has a non-deterministic > test failure merged. I would like to easily skip those in my fork > too, without adding extra commits to me working branches, as that > would require the same commit to be duped into several in-progress > branches, vs setting the env var once. > >> The risk I see is that some tests will stay forever in this skip >> variable, so it will be dead code for CI, but still alive and failing >> for people running tests manually who hit the regression. > > Again, there should be a bug filed for any flaky test. Anyone can > do this, if they see it locally or in their fork CI, or in staging > CI. If no one can see an obvious fix, then anyone can also propose > to disable the test. > >> If you still want an alternative to removing test, implementing a >> skip_list in tests/qemu-iotests/meson.build is better than an env var >> IMHO, and achieves the exact same effect, for CI and for users. >> >> What do you think? > > IMHO there needs to be a way to skip flaky tests which does not > require code changes as the only available option. Code changes > are the permanent fix, env var is the immediate workaround. > I'm not sure all this answers to my question about How to ensure users who run tests and the CI both see the same skip list. I don't mind having an env var, a black list in meson or any other solution, but having different results on a dev machine and in CI is not a good design. So whatever the solution is, the CI yaml file is not the proper place to store this information. >>>> Once it's green, in theory, code breaking existing tests should not be >>>> merged, right? So what would be the usage of this variable? >>> >>> We have had a pretty decent chunk of non-deterministic test failures, >>> so there is high likelihood we can merge stuff that passes once and >>> then subsequently fails some subset of the time. This non-determinism >>> is one of the key reasons that we currently only have a hand picked >>> selection of block I/O tests run by meson. >>> >>> While I've tested this series and haven't see any non-determinstic >>> failures in what I'm proposing to enable thus far, I think there is >>> still a pretty high chance we'll uncover some more. >>> >> >> Fair point. > > With regards, > Daniel
