On Tue, 26 Sept 2023 at 01:58, Michal Wajdeczko <[email protected]> wrote: > > A kunit suite is a top level test from the KTAP point of view but > all suite diagnostic messages are printed at the subtest level: > > $ ./tools/testing/kunit/kunit.py run --raw_output \ > --kunitconfig ./lib/kunit/.kunitconfig "example.*simple*" > > KTAP version 1 > 1..1 > # example: initializing suite > # example: failed to initialize (-ENODEV) > not ok 1 example > > KTAP version 1 > 1..1 > # example: initializing suite > KTAP version 1 > # Subtest: example > # module: kunit_example_test > 1..1 > # example_simple_test: initializing > # example_simple_test: cleaning up > ok 1 example_simple_test > # example: exiting suite > ok 1 example > > Replace hardcoded indent in kunit_printk() with flexible > indentation based on the argument type (test or suite): > > KTAP version 1 > 1..1 > # example: initializing suite > # example: failed to initialize (-ENODEV) > not ok 1 example > > KTAP version 1 > 1..1 > # example: initializing suite > KTAP version 1 > # Subtest: example > # module: kunit_example_test > 1..1 > # example_simple_test: initializing > # example_simple_test: cleaning up > ok 1 example_simple_test > # example: exiting suite > ok 1 example > > Signed-off-by: Michal Wajdeczko <[email protected]> > Cc: David Gow <[email protected]> > Cc: Rae Moar <[email protected]> > ---
I think this is looking pretty good overall. It'll need rebasing on
the current kselftest/kunit branch, though.
As Rae points out, some of this will probably need reworking if we
start to support more arbitrary nesting. Equally, we probably need a
name for the 'top level' container (of which suites are effectively
subtests). To add to the name bikeshedding, while 'executor' works
well enough, I think the (K)TAP spec refers to this as the 'test
document'. Is that right, Rae?
> include/kunit/test.h | 24 ++++++++++++++++++++++--
> lib/kunit/test.c | 7 -------
> 2 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 20ed9f9275c9..158876c4cb43 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -509,6 +509,21 @@ void __printf(2, 3) kunit_log_append(struct
> string_stream *log, const char *fmt,
> kunit_try_catch_throw(&((test_or_suite)->try_catch)); \
> } while (0)
>
> +/* Currently supported test levels */
> +enum {
> + KUNIT_LEVEL_SUITE = 0,
> + KUNIT_LEVEL_CASE,
> + KUNIT_LEVEL_CASE_PARAM,
> +};
> +
> +#define kunit_level(test_or_suite) \
> + _Generic((test_or_suite), \
> + struct kunit_suite * : KUNIT_LEVEL_SUITE, \
> + struct kunit * : KUNIT_LEVEL_CASE)
> +
> +#define kunit_indent_level(test_or_suite) \
> + (KUNIT_INDENT_LEN * kunit_level(test_or_suite))
> +
This looks good to me. I like the use of _Generic() -- I suspect there
might be one or two other places in the KUnit code it could be useful
in the future.
I don't think we need to handle kunit_case here: once a test is
actually being run (and therefore able to log anything), it's already
got a struct kunit*.
> /*
> * printk and log to per-test or per-suite log buffer. Logging only done
> * if CONFIG_KUNIT_DEBUGFS is 'y'; if it is 'n', no log is allocated/used.
> @@ -520,9 +535,14 @@ void __printf(2, 3) kunit_log_append(struct
> string_stream *log, const char *fmt,
> ##__VA_ARGS__); \
> } while (0)
>
> +#define kunit_log_indent(lvl, test_or_suite, fmt, ...) \
> + kunit_log(lvl, test_or_suite, "%*s" fmt, \
> + kunit_indent_level(test_or_suite), "", \
> + ##__VA_ARGS__)
> +
> #define kunit_printk(lvl, test, fmt, ...) \
> - kunit_log(lvl, test, KUNIT_SUBTEST_INDENT "# %s: " fmt, \
> - (test)->name, ##__VA_ARGS__)
> + kunit_log_indent(lvl, test, "# %s: " fmt, \
> + (test)->name, ##__VA_ARGS__)
>
> /**
> * kunit_info() - Prints an INFO level message associated with @test.
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index fb5981ce578d..d10e6d610e20 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -135,13 +135,6 @@ size_t kunit_suite_num_test_cases(struct kunit_suite
> *suite)
> }
> EXPORT_SYMBOL_GPL(kunit_suite_num_test_cases);
>
> -/* Currently supported test levels */
> -enum {
> - KUNIT_LEVEL_SUITE = 0,
> - KUNIT_LEVEL_CASE,
> - KUNIT_LEVEL_CASE_PARAM,
> -};
> -
> static void kunit_print_suite_start(struct kunit_suite *suite)
> {
> /*
> --
> 2.25.1
>
smime.p7s
Description: S/MIME Cryptographic Signature
