Re: [PATCH] kunit: Reset suite count after running tests

[Daniel Latypov]

On Tue, Oct 5, 2021 at 9:41 PM David Gow <davidgow@xxxxxxxxxx> wrote:
>
> There are some KUnit tests (KFENCE, Thunderbolt) which, for various
> reasons, do not use the kunit_test_suite() macro and end up running
> before the KUnit executor runs its tests. This means that their results
> are printed separately, and they aren't included in the suite count used
> by the executor.
>
> This causes the executor output to be invalid TAP, however, as the suite
> numbers used are no-longer 1-based, and don't match the test plan.
> kunit_tool, therefore, prints a large number of warnings.
>
> While it'd be nice to fix the tests to run in the executor, in the
> meantime, reset the suite counter to 1 in __kunit_test_suites_exit.
> Not only does this fix the executor, it means that if there are multiple
> calls to __kunit_test_suites_init() across different tests, they'll each
> get their own numbering.
>
> kunit_tool likes this better: even if it's lacking the results for those
> tests which don't use the executor (due to the lack of TAP header), the
> output for the other tests is valid.
>
> Signed-off-by: David Gow <davidgow@xxxxxxxxxx>

Reviewed-by: Daniel Latypov <dlatypov@xxxxxxxxxx>

I had been hoping we could pass in a kunit_context object of sorts to
__kunit_test_suites_init() and exit.
That feels like that would be the right place to store this counter in
the future.

But that's currently blocked on the aforementioned tests going outside
the executor and using these funcs directly.
So for now, this seems like the simplest and most sensible fix.

I wonder if we should add a TODO about the counter...
But eh, it was already a static variable before, so this isn't really
increasing the hackiness too much.

> ---
>  lib/kunit/test.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index f246b847024e..3bd741e50a2d 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -190,10 +190,10 @@ enum kunit_status kunit_suite_has_succeeded(struct kunit_suite *suite)
>  }
>  EXPORT_SYMBOL_GPL(kunit_suite_has_succeeded);
>
> +static size_t kunit_suite_counter = 1;
> +
>  static void kunit_print_subtest_end(struct kunit_suite *suite)
>  {
> -       static size_t kunit_suite_counter = 1;
> -
>         kunit_print_ok_not_ok((void *)suite, false,
>                               kunit_suite_has_succeeded(suite),
>                               kunit_suite_counter++,
> @@ -583,6 +583,8 @@ void __kunit_test_suites_exit(struct kunit_suite **suites)
>
>         for (i = 0; suites[i] != NULL; i++)
>                 kunit_exit_suite(suites[i]);
> +
> +       kunit_suite_counter = 1;
>  }
>  EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
>
> --
> 2.33.0.800.g4c38ced690-goog
>