[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7bb3b635-9fed-47ab-a640-ccac6d283b54@intel.com>
Date: Tue, 5 Mar 2024 16:48:06 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Jakub Kicinski <kuba@...nel.org>, Mark Brown <broonie@...nel.org>,
<keescook@...omium.org>
CC: <davem@...emloft.net>, <netdev@...r.kernel.org>, <edumazet@...gle.com>,
<pabeni@...hat.com>, <shuah@...nel.org>, <linux-kselftest@...r.kernel.org>,
<mic@...ikod.net>, <linux-security-module@...r.kernel.org>,
<jakub@...udflare.com>
Subject: Re: [PATCH v4 00/12] selftests: kselftest_harness: support using
xfail
On 3/5/24 00:04, Jakub Kicinski wrote:
> On Mon, 4 Mar 2024 22:20:03 +0000 Mark Brown wrote:
>> On Wed, Feb 28, 2024 at 04:59:07PM -0800, Jakub Kicinski wrote:
>>
>>> When running selftests for our subsystem in our CI we'd like all
>>> tests to pass. Currently some tests use SKIP for cases they
>>> expect to fail, because the kselftest_harness limits the return
>>> codes to pass/fail/skip. XFAIL which would be a great match
>>> here cannot be used.
>>>
>>> Remove the no_print handling and use vfork() to run the test in
>>> a different process than the setup. This way we don't need to
>>> pass "failing step" via the exit code. Further clean up the exit
>>> codes so that we can use all KSFT_* values. Rewrite the result
>>> printing to make handling XFAIL/XPASS easier. Support tests
>>> declaring combinations of fixture + variant they expect to fail.
>>
>> This series landed in -next today and has caused breakage on all
>> platforms in the ALSA pcmtest-driver test. When run on systems that
>> don't have the driver it needs loaded the test skip but since this
>> series was merged skipped tests are logged but then reported back as
>> failures:
>>
>> # selftests: alsa: test-pcmtest-driver
>> # TAP version 13
>> # 1..5
>> # # Starting 5 tests from 1 test cases.
>> # # RUN pcmtest.playback ...
>> # # SKIP Can't read patterns. Probably, module isn't loaded
>> # # playback: Test failed
>> # # FAIL pcmtest.playback
>> # not ok 1 pcmtest.playback # Can't read patterns. Probably, module isn't loaded
>> # # RUN pcmtest.capture ...
>> # # SKIP Can't read patterns. Probably, module isn't loaded
>> # # capture: Test failed
>> # # FAIL pcmtest.capture
>> # not ok 2 pcmtest.capture # Can't read patterns. Probably, module isn't loaded
>> # # RUN pcmtest.ni_capture ...
>> # # SKIP Can't read patterns. Probably, module isn't loaded
>> # # ni_capture: Test failed
>> # # FAIL pcmtest.ni_capture
>> # not ok 3 pcmtest.ni_capture # Can't read patterns. Probably, module isn't loaded
>> # # RUN pcmtest.ni_playback ...
>> # # SKIP Can't read patterns. Probably, module isn't loaded
>> # # ni_playback: Test failed
>> # # FAIL pcmtest.ni_playback
>> # not ok 4 pcmtest.ni_playback # Can't read patterns. Probably, module isn't loaded
>> # # RUN pcmtest.reset_ioctl ...
>> # # SKIP Can't read patterns. Probably, module isn't loaded
>> # # reset_ioctl: Test failed
>> # # FAIL pcmtest.reset_ioctl
>> # not ok 5 pcmtest.reset_ioctl # Can't read patterns. Probably, module isn't loaded
>> # # FAILED: 0 / 5 tests passed.
>> # # Totals: pass:0 fail:5 xfail:0 xpass:0 skip:0 error:0
>>
>> I haven't completely isolated the issue due to some other breakage
>> that's making it harder that it should be to test.
>>
>> A sample full log can be seen at:
>>
>> https://lava.sirena.org.uk/scheduler/job/659576#L1349
>
> Thanks! the exit() inside the skip evaded my grep, I'm testing this:
>
> diff --git a/tools/testing/selftests/alsa/test-pcmtest-driver.c b/tools/testing/selftests/alsa/test-pcmtest-driver.c
> index a52ecd43dbe3..7ab81d6f9e05 100644
> --- a/tools/testing/selftests/alsa/test-pcmtest-driver.c
> +++ b/tools/testing/selftests/alsa/test-pcmtest-driver.c
> @@ -127,11 +127,11 @@ FIXTURE_SETUP(pcmtest) {
> int err;
>
> if (geteuid())
> - SKIP(exit(-1), "This test needs root to run!");
> + SKIP(exit(KSFT_SKIP), "This test needs root to run!");
>
> err = read_patterns();
> if (err)
> - SKIP(exit(-1), "Can't read patterns. Probably, module isn't loaded");
> + SKIP(exit(KSFT_SKIP), "Can't read patterns. Probably, module isn't loaded");
>
> card_name = malloc(127);
> ASSERT_NE(card_name, NULL);
> diff --git a/tools/testing/selftests/mm/hmm-tests.c b/tools/testing/selftests/mm/hmm-tests.c
> index 20294553a5dd..356ba5f3b68c 100644
> --- a/tools/testing/selftests/mm/hmm-tests.c
> +++ b/tools/testing/selftests/mm/hmm-tests.c
> @@ -138,7 +138,7 @@ FIXTURE_SETUP(hmm)
>
> self->fd = hmm_open(variant->device_number);
> if (self->fd < 0 && hmm_is_coherent_type(variant->device_number))
> - SKIP(exit(0), "DEVICE_COHERENT not available");
> + SKIP(exit(KSFT_SKIP), "DEVICE_COHERENT not available");
> ASSERT_GE(self->fd, 0);
> }
>
> @@ -149,7 +149,7 @@ FIXTURE_SETUP(hmm2)
>
> self->fd0 = hmm_open(variant->device_number0);
> if (self->fd0 < 0 && hmm_is_coherent_type(variant->device_number0))
> - SKIP(exit(0), "DEVICE_COHERENT not available");
> + SKIP(exit(KSFT_SKIP), "DEVICE_COHERENT not available");
> ASSERT_GE(self->fd0, 0);
> self->fd1 = hmm_open(variant->device_number1);
> ASSERT_GE(self->fd1, 0);
>
>> but there's no more context. I'm also seeing some breakage in the
>> seccomp selftests which also use kselftest-harness:
>>
>> # # RUN TRAP.dfl ...
>> # # dfl: Test exited normally instead of by signal (code: 0)
>> # # FAIL TRAP.dfl
>> # not ok 56 TRAP.dfl
>> # # RUN TRAP.ign ...
>> # # ign: Test exited normally instead of by signal (code: 0)
>> # # FAIL TRAP.ign
>> # not ok 57 TRAP.ign
>
> Ugh, I'm guessing vfork() "eats" the signal, IOW grandchild signals,
> child exits? vfork() and signals.. I'd rather leave to Kees || Mickael.
>
Hi, sorry for not trying to reproduce it locally and still commenting,
but my vfork() man page says:
| The child must not return from the current function or call
| exit(3) (which would have the effect of calling exit handlers
| established by the parent process and flushing the parent's stdio(3)
| buffers), but may call _exit(2).
And you still have some exit(3) calls.
Powered by blists - more mailing lists