Re: [PATCH 1/2] kunit: Warn if tests are slow

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 11 Sep 2023, Maxime Ripard <mripard@kernel.org> wrote:
> Kunit recently gained support to setup attributes, the first one being
> the speed of a given test, then allowing to filter out slow tests.
>
> A slow test is defined in the documentation as taking more than one
> second. There's an another speed attribute called "super slow" but whose
> definition is less clear.
>
> Add support to the test runner to check the test execution time, and
> report tests that should be marked as slow but aren't.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  lib/kunit/test.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 49698a168437..a3b924501f3d 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -379,6 +379,9 @@ static void kunit_run_case_internal(struct kunit *test,
>  				    struct kunit_suite *suite,
>  				    struct kunit_case *test_case)
>  {
> +	struct timespec64 start, end;
> +	struct timespec64 duration;
> +
>  	if (suite->init) {
>  		int ret;
>  
> @@ -390,7 +393,20 @@ static void kunit_run_case_internal(struct kunit *test,
>  		}
>  	}
>  
> +	ktime_get_ts64(&start);
> +
>  	test_case->run_case(test);
> +
> +	ktime_get_ts64(&end);
> +
> +	duration = timespec64_sub(end, start);
> +
> +	if (duration.tv_sec >= 1 &&
> +	    (test_case->attr.speed == KUNIT_SPEED_UNSET ||
> +	     test_case->attr.speed >= KUNIT_SPEED_NORMAL))
> +		kunit_warn(test,
> +			   "Test should be marked slow (runtime: %lld.%09lds)",
> +			   duration.tv_sec, duration.tv_nsec);

Two thoughts:

Should there be some tolerance here? Otherwise we're flagging this on
the slowest machines, and we'll be defining tests slow based on
that. Like, warn if it takes more than 2 seconds.

What if someone makes a test faster, but forgets to update the
attribute? Should we also flag slow tests that are in fact fast?


BR,
Jani.


>  }
>  
>  static void kunit_case_internal_cleanup(struct kunit *test)

-- 
Jani Nikula, Intel Open Source Graphics Center