Re: [PATCH 2/4] kunit: Fix indentation level of suite messages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 26 Sept 2023 at 01:58, Michal Wajdeczko
<michal.wajdeczko@xxxxxxxxx> 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 <michal.wajdeczko@xxxxxxxxx>
> Cc: David Gow <davidgow@xxxxxxxxxx>
> Cc: Rae Moar <rmoar@xxxxxxxxxx>
> ---

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
>

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature


[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux