RE: [PATCH] selftests/kselftest/runner.sh: Add 45 second timeout per test

From: Tim.Bird
Date: Thu Sep 19 2019 - 16:49:25 EST




> -----Original Message-----
> From Tim.Bird@xxxxxxxx
>
> > -----Original Message-----
> > From: Kees Cook
> >
> > Commit a745f7af3cbd ("selftests/harness: Add 30 second timeout per
> > test") solves the problem of kselftest_harness.h-using binary tests
> > possibly hanging forever. However, scripts and other binaries can still
> > hang forever. This adds a global timeout to each test script run.
> >
> > To make this configurable (e.g. as needed in the "rtc" test case),
> > include a new per-test-directory "settings" file (similar to "config")
> > that can contain kselftest-specific settings. The first recognized field
> > is "timeout".
>
> OK - this is quite interesting. I have had on my to-do list an action
> item to propose the creation of a file (or a standard kerneldoc string)
> to hold CI-related meta-data (of which timeout is one example).
>
> What other meta-data did you have in mind?
>
> I would like (that Fuego, and probably other CI systems would like) to have
> access to data like test dependencies, descriptions, and results
> interpretation
> that would be beneficial for both CI systems (using them to control test
> invocations and scheduling), as
> well as users who are trying to interpret and handle the test results.
> So this concept is a very welcome addition to kselftest.
>
> LTP is in the process of adopting a new system for expressing and handling
> their test meta-data.
> See the discussion at:
> https://lists.yoctoproject.org/pipermail/automated-testing/2019-
> August/000471.html
> and the prototype implementation at:
> https://github.com/metan-ucw/ltp/tree/master/docparse
>
> I realize that that system is coupled pretty tightly to LTP, but conceptually
> some of the same type of information would be valuable for kselftest tests.
> One example of a specific field that would be handy is 'need_root'.
>
> It would be nice to avoid proliferation of such meta-data schemas (that is
> field names), so maybe we can have a discussion about this before adopting
> something?
>
> Just FYI, I'm OK with the name 'timeout'. I think that's pretty much
> universally
> used by all CI runners I'm aware of to indicate the test timeout value. But
> before adopting other fields it would be good to start comparing notes
> and not invent a bunch of new field names for concepts that are already in
> other systems.
>
> >
> > Additionally, this splits the reporting for timeouts into a specific
> > "TIMEOUT" not-ok (and adds exit code reporting in the remaining case).
> >
> > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> > ---
> > tools/testing/selftests/kselftest/runner.sh | 36 +++++++++++++++++++--
> > tools/testing/selftests/rtc/settings | 1 +
> > 2 files changed, 34 insertions(+), 3 deletions(-)
> > create mode 100644 tools/testing/selftests/rtc/settings
> >
> > diff --git a/tools/testing/selftests/kselftest/runner.sh
> > b/tools/testing/selftests/kselftest/runner.sh
> > index 00c9020bdda8..84de7bc74f2c 100644
> > --- a/tools/testing/selftests/kselftest/runner.sh
> > +++ b/tools/testing/selftests/kselftest/runner.sh
> > @@ -3,9 +3,14 @@
> > #
> > # Runs a set of tests in a given subdirectory.
> > export skip_rc=4
> > +export timeout_rc=124
> what are the units here? I presume seconds?

Nevermind. I misread this. This is the return code from the 'timeout' program, right?
-- Tim