Re: [PATCH 1/2] of: unittest: option to allow tests that trigger kernel stack dump

[Rob Herring]

On Wed, Mar 01, 2023 at 10:01:36AM -0600, Frank Rowand wrote:
> On 2/28/23 22:07, Guenter Roeck wrote:
> > On 2/28/23 17:21, Frank Rowand wrote:
> >> Commit 74df14cd301a ("of: unittest: add node lifecycle tests") added
> >> some tests that trigger a kernel stack dump.  Filtering the boot
> >> messages with scripts/dtc/of_unittest_expect detects that the stack
> >> dump is expected instead of being a test error.
> >>
> >> Test beds might interpret the stack dumps as errors, resulting in
> >> needless debugging and error reports.  These test beds are likely
> >> to remove unittests due to these stack dumps. To avoid these problems,
> >> have unittest default to skip the tests that trigger a stack dump.
> >>
> >> Add a kernel cmdline option to not skip those tests.  This option can
> >> be used by testers who are able to interpret the stack dumps as not
> >> an error.
> >>
> >> Signed-off-by: Frank Rowand <frowand.list@xxxxxxxxx>
> >> ---
> >>   drivers/of/unittest.c | 54 ++++++++++++++++++++++++++++++++++++++++---
> >>   1 file changed, 51 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> >> index b5a7a31d8bd2..3a9bc2bc4ba1 100644
> >> --- a/drivers/of/unittest.c
> >> +++ b/drivers/of/unittest.c
> >> @@ -70,6 +70,36 @@ static struct unittest_results {
> >>   #define EXPECT_NOT_END(level, fmt, ...) \
> >>       printk(level pr_fmt("EXPECT_NOT / : ") fmt, ##__VA_ARGS__)
> >>   +/*
> >> + * Some tests will cause the kernel to emit a stack dump, aka back trace,
> >> + * when the test is successful.  The tests should make it possible for
> >> + * test beds to detect that the trace is not an error via EXPECT_BEGIN().
> >> + *
> >> + * Most test beds do not process the EXPECT_BEGIN() information and may
> >> + * flag the stack dump as an error, thus reporting a false failure.  It
> >> + * is hoped that the KTAP version 4 specification will add the EXPECT_BEGIN()
> >> + * processing to test beds.
> >> + *
> >> + * By default, skip tests that cause a stack dump.  Test beds that process
> >> + * EXPECT_BEGIN() information should enable these tests via a kernel boot
> >> + * command line option.
> >> + */
> >> +static int stackdump_tests_enabled;
> >> +
> >> +static int __init enable_unittest_stackdump(char *str)
> >> +{
> >> +    stackdump_tests_enabled = 1;
> >> +    return 0;
> >> +}
> >> +
> >> +static int __init disable_unittest_stackdump(char *str)
> >> +{
> >> +    stackdump_tests_enabled = 0;
> >> +    return 0;
> >> +}
> >> +early_param("of_unittest_stackdump", enable_unittest_stackdump);
> >> +early_param("no_of_unittest_stackdump", disable_unittest_stackdump);
> > 
> > Does no_of_unittest_stackdump have any benefit or value ?
> 
> I would say no, but it is a common pattern to provide both
> foo and no_foo.

It is? I see one documented example. I see numerous ones that are 
'no_foo'.

This doesn't scale well if lots of tests need to disable it. Perhaps it 
should be more generic (at least documentation/naming wise even if the 
implmentation lives in DT unittest for now).

Rob