From: "Mickaël Salaün" <mic@digikod.net>
To: Sean Christopherson <seanjc@google.com>
Cc: "Christian Brauner" <brauner@kernel.org>,
"Jakub Kicinski" <kuba@kernel.org>,
"Kees Cook" <keescook@chromium.org>,
"Mark Brown" <broonie@kernel.org>,
"Shengyu Li" <shengyu.li.evgeny@gmail.com>,
"Shuah Khan" <shuah@kernel.org>,
"David S . Miller" <davem@davemloft.net>,
"Günther Noack" <gnoack@google.com>,
"Will Drewry" <wad@chromium.org>,
"kernel test robot" <oliver.sang@intel.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v4 10/10] selftests/harness: Fix TEST_F()'s exit codes
Date: Fri, 3 May 2024 09:51:05 +0200 [thread overview]
Message-ID: <20240503.Ioj0ahCi7aeN@digikod.net> (raw)
In-Reply-To: <ZjQXghB6imRFU4HX@google.com>
On Thu, May 02, 2024 at 03:45:22PM GMT, Sean Christopherson wrote:
> On Thu, May 02, 2024, Mickaël Salaün wrote:
> > @@ -462,8 +462,10 @@ static inline pid_t clone3_vfork(void)
> > munmap(teardown, sizeof(*teardown)); \
> > if (self && fixture_name##_teardown_parent) \
> > munmap(self, sizeof(*self)); \
> > - if (!WIFEXITED(status) && WIFSIGNALED(status)) \
> > - /* Forward signal to __wait_for_test(). */ \
> > + /* Forward exit codes and signals to __wait_for_test(). */ \
> > + if (WIFEXITED(status)) \
> > + _exit(_metadata->exit_code); \
>
> This needs to be:
>
> if (WIFEXITED(status)) \
> _exit(WEXITSTATUS(status)); \
>
> otherwise existing tests that communicate FAIL/SKIP via exit() continue to yield
> exit(0) and thus false passes.
Yes of course.
>
> If that conflicts with tests that want to communicate via _metadata->exit_code,
> then maybe this?
>
> if (WIFEXITED(status)) \
> _exit(WEXITSTATUS(status) ?: _metadata->exit_code); \
I prefer this approach handling failed expectations in the fixture
teardown too.
However, the direct call to _exit() doesn't handle failed asserts. I'll
fix that.
>
> Or I suppose _metadata->exit_code could have priority, but that seems weird to
> me, e.g. if a test sets exit_code and then explodes, it seems like the explosion
> should be reported.
>
> > + if (WIFSIGNALED(status)) \
> > kill(getpid(), WTERMSIG(status)); \
> > __test_check_assert(_metadata); \
> > } \
> > --
> > 2.45.0
> >
>
prev parent reply other threads:[~2024-05-03 7:51 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-02 21:09 [PATCH v4 00/10] Fix Kselftest's vfork() side effects Mickaël Salaün
2024-05-02 21:09 ` [PATCH v4 01/10] selftests/pidfd: Fix config for pidfd_setns_test Mickaël Salaün
2024-05-02 21:09 ` [PATCH v4 02/10] selftests/landlock: Fix FS tests when run on a private mount point Mickaël Salaün
2024-05-02 21:09 ` [PATCH v4 03/10] selftests/harness: Fix fixture teardown Mickaël Salaün
2024-05-02 21:09 ` [PATCH v4 04/10] selftests/harness: Fix interleaved scheduling leading to race conditions Mickaël Salaün
2024-05-02 21:09 ` [PATCH v4 05/10] selftests/landlock: Do not allocate memory in fixture data Mickaël Salaün
2024-05-02 21:09 ` [PATCH v4 06/10] selftests/harness: Constify fixture variants Mickaël Salaün
2024-05-02 21:09 ` [PATCH v4 07/10] selftests/pidfd: Fix wrong expectation Mickaël Salaün
2024-05-02 21:09 ` [PATCH v4 08/10] selftests/harness: Share _metadata between forked processes Mickaël Salaün
2024-05-02 21:09 ` [PATCH v4 09/10] selftests/harness: Fix vfork() side effects Mickaël Salaün
2024-05-02 21:09 ` [PATCH v4 10/10] selftests/harness: Fix TEST_F()'s exit codes Mickaël Salaün
2024-05-02 22:45 ` Sean Christopherson
2024-05-03 7:51 ` Mickaël Salaün [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240503.Ioj0ahCi7aeN@digikod.net \
--to=mic@digikod.net \
--cc=brauner@kernel.org \
--cc=broonie@kernel.org \
--cc=davem@davemloft.net \
--cc=gnoack@google.com \
--cc=keescook@chromium.org \
--cc=kuba@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=oliver.sang@intel.com \
--cc=seanjc@google.com \
--cc=shengyu.li.evgeny@gmail.com \
--cc=shuah@kernel.org \
--cc=wad@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).