| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 7 Nov 2020 12:58:21 +0800
From: David Gow <davidgow@...gle.com>
To: Arpitha Raghunandan <98.arpi@...il.com>
Cc: Brendan Higgins <brendanhiggins@...gle.com>,
Shuah Khan <skhan@...uxfoundation.org>,
Marco Elver <elver@...gle.com>,
Iurii Zaikin <yzaikin@...gle.com>,
"Theodore Ts'o" <tytso@....edu>,
Andreas Dilger <adilger.kernel@...ger.ca>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@...r.kernel.org>,
KUnit Development <kunit-dev@...glegroups.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-kernel-mentees@...ts.linuxfoundation.org,
linux-ext4@...r.kernel.org
Subject: Re: [PATCH v6 1/2] kunit: Support for Parameterized Testing
On Sat, Nov 7, 2020 at 3:22 AM Arpitha Raghunandan <98.arpi@...il.com> wrote:
>
> Implementation of support for parameterized testing in KUnit.
> This approach requires the creation of a test case using the
> KUNIT_CASE_PARAM macro that accepts a generator function as input.
> This generator function should return the next parameter given the
> previous parameter in parameterized tests. It also provides
> a macro to generate common-case generators.
>
> Signed-off-by: Arpitha Raghunandan <98.arpi@...il.com>
> Co-developed-by: Marco Elver <elver@...gle.com>
> Signed-off-by: Marco Elver <elver@...gle.com>
> ---
This looks good to me! A couple of minor thoughts about the output
format below, but I'm quite happy to have this as-is regardless.
Reviewed-by: David Gow <davidgow@...gle.com>
Cheers,
-- David
> Changes v5->v6:
> - Fix alignment to maintain consistency
> Changes v4->v5:
> - Update kernel-doc comments.
> - Use const void* for generator return and prev value types.
> - Add kernel-doc comment for KUNIT_ARRAY_PARAM.
> - Rework parameterized test case execution strategy: each parameter is executed
> as if it was its own test case, with its own test initialization and cleanup
> (init and exit are called, etc.). However, we cannot add new test cases per TAP
> protocol once we have already started execution. Instead, log the result of
> each parameter run as a diagnostic comment.
> Changes v3->v4:
> - Rename kunit variables
> - Rename generator function helper macro
> - Add documentation for generator approach
> - Display test case name in case of failure along with param index
> Changes v2->v3:
> - Modifictaion of generator macro and method
> Changes v1->v2:
> - Use of a generator method to access test case parameters
>
> include/kunit/test.h | 36 ++++++++++++++++++++++++++++++++++
> lib/kunit/test.c | 46 +++++++++++++++++++++++++++++++-------------
> 2 files changed, 69 insertions(+), 13 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index db1b0ae666c4..16616d3974f9 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -107,6 +107,7 @@ struct kunit;
> *
> * @run_case: the function representing the actual test case.
> * @name: the name of the test case.
> + * @generate_params: the generator function for parameterized tests.
> *
> * A test case is a function with the signature,
> * ``void (*)(struct kunit *)``
> @@ -141,6 +142,7 @@ struct kunit;
> struct kunit_case {
> void (*run_case)(struct kunit *test);
> const char *name;
> + const void* (*generate_params)(const void *prev);
>
> /* private: internal use only. */
> bool success;
> @@ -163,6 +165,22 @@ static inline char *kunit_status_to_string(bool status)
> */
> #define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name }
>
> +/**
> + * KUNIT_CASE_PARAM - A helper for creation a parameterized &struct kunit_case
> + *
> + * @test_name: a reference to a test case function.
> + * @gen_params: a reference to a parameter generator function.
> + *
> + * The generator function ``const void* gen_params(const void *prev)`` is used
> + * to lazily generate a series of arbitrarily typed values that fit into a
> + * void*. The argument @prev is the previously returned value, which should be
> + * used to derive the next value; @prev is set to NULL on the initial generator
> + * call. When no more values are available, the generator must return NULL.
> + */
> +#define KUNIT_CASE_PARAM(test_name, gen_params) \
> + { .run_case = test_name, .name = #test_name, \
> + .generate_params = gen_params }
> +
> /**
> * struct kunit_suite - describes a related collection of &struct kunit_case
> *
> @@ -208,6 +226,10 @@ struct kunit {
> const char *name; /* Read only after initialization! */
> char *log; /* Points at case log after initialization */
> struct kunit_try_catch try_catch;
> + /* param_value is the current parameter value for a test case. */
> + const void *param_value;
> + /* param_index stores the index of the parameter in parameterized tests. */
> + int param_index;
> /*
> * success starts as true, and may only be set to false during a
> * test case; thus, it is safe to update this across multiple
> @@ -1742,4 +1764,18 @@ do { \
> fmt, \
> ##__VA_ARGS__)
>
> +/**
> + * KUNIT_ARRAY_PARAM() - Define test parameter generator from an array.
> + * @name: prefix for the test parameter generator function.
> + * @array: array of test parameters.
> + *
> + * Define function @name_gen_params which uses @array to generate parameters.
> + */
> +#define KUNIT_ARRAY_PARAM(name, array) \
> + static const void *name##_gen_params(const void *prev) \
> + { \
> + typeof((array)[0]) * __next = prev ? ((typeof(__next)) prev) + 1 : (array); \
> + return __next - (array) < ARRAY_SIZE((array)) ? __next : NULL; \
> + }
> +
> #endif /* _KUNIT_TEST_H */
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 750704abe89a..329fee9e0634 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -325,29 +325,25 @@ static void kunit_catch_run_case(void *data)
> * occur in a test case and reports them as failures.
> */
> static void kunit_run_case_catch_errors(struct kunit_suite *suite,
> - struct kunit_case *test_case)
> + struct kunit_case *test_case,
> + struct kunit *test)
> {
> struct kunit_try_catch_context context;
> struct kunit_try_catch *try_catch;
> - struct kunit test;
>
> - kunit_init_test(&test, test_case->name, test_case->log);
> - try_catch = &test.try_catch;
> + kunit_init_test(test, test_case->name, test_case->log);
> + try_catch = &test->try_catch;
>
> kunit_try_catch_init(try_catch,
> - &test,
> + test,
> kunit_try_run_case,
> kunit_catch_run_case);
> - context.test = &test;
> + context.test = test;
> context.suite = suite;
> context.test_case = test_case;
> kunit_try_catch_run(try_catch, &context);
>
> - test_case->success = test.success;
> -
> - kunit_print_ok_not_ok(&test, true, test_case->success,
> - kunit_test_case_num(suite, test_case),
> - test_case->name);
> + test_case->success = test->success;
> }
>
> int kunit_run_tests(struct kunit_suite *suite)
> @@ -356,8 +352,32 @@ int kunit_run_tests(struct kunit_suite *suite)
>
> kunit_print_subtest_start(suite);
>
> - kunit_suite_for_each_test_case(suite, test_case)
> - kunit_run_case_catch_errors(suite, test_case);
> + kunit_suite_for_each_test_case(suite, test_case) {
> + struct kunit test = { .param_value = NULL, .param_index = 0 };
> + bool test_success = true;
> +
> + if (test_case->generate_params)
> + test.param_value = test_case->generate_params(NULL);
> +
> + do {
> + kunit_run_case_catch_errors(suite, test_case, &test);
> + test_success &= test_case->success;
> +
> + if (test_case->generate_params) {
> + kunit_log(KERN_INFO, &test,
> + KUNIT_SUBTEST_INDENT
> + "# %s: param-%d %s",
Would it make sense to have this imitate the TAP format a bit more?
So, have "# [ok|not ok] - [name]" as the format? [name] could be
something like "[test_case->name]:param-[index]" or similar.
If we keep it commented out and don't indent it further, it won't
formally be a nested test (though if we wanted to support those later,
it'd be easy to add), but I think it would be nicer to be consistent
here.
My other suggestion -- albeit one outside the scope of this initial
version -- would be to allow the "param-%d" name to be overridden
somehow by a test. For example, the ext4 inode test has names for all
its test cases: it'd be nice to be able to display those instead (even
if they're not formatted as identifiers as-is).
> + test_case->name, test.param_index,
> + kunit_status_to_string(test.success));
> + test.param_value = test_case->generate_params(test.param_value);
> + test.param_index++;
> + }
> + } while (test.param_value);
> +
> + kunit_print_ok_not_ok(&test, true, test_success,
> + kunit_test_case_num(suite, test_case),
> + test_case->name);
> + }
>
> kunit_print_subtest_end(suite);
>
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@...glegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20201106192154.51514-1-98.arpi%40gmail.com.
Powered by blists - more mailing lists