[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f25c881d-03f9-e246-d8d4-e985d9662d04@gmail.com>
Date: Tue, 27 Oct 2020 10:44:05 +0530
From: Arpitha Raghunandan <98.arpi@...il.com>
To: Marco Elver <elver@...gle.com>
Cc: Brendan Higgins <brendanhiggins@...gle.com>,
skhan@...uxfoundation.org, 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>,
LKML <linux-kernel@...r.kernel.org>,
linux-kernel-mentees@...ts.linuxfoundation.org,
linux-ext4@...r.kernel.org
Subject: Re: [PATCH v3 1/2] kunit: Support for Parameterized Testing
On 27/10/20 4:44 am, Marco Elver wrote:
> On Mon, 26 Oct 2020 at 19:36, 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>
>> ---
>> Changes v2->v3:
>> - Modifictaion of generator macro and method
>
> Great to see it worked as expected!
>
>> Changes v1->v2:
>> - Use of a generator method to access test case parameters
>>
>> include/kunit/test.h | 32 ++++++++++++++++++++++++++++++++
>> lib/kunit/test.c | 20 +++++++++++++++++++-
>> 2 files changed, 51 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/kunit/test.h b/include/kunit/test.h
>> index a423fffefea0..16bf9f334e2c 100644
>> --- a/include/kunit/test.h
>> +++ b/include/kunit/test.h
>> @@ -142,6 +142,12 @@ struct kunit_case {
>> void (*run_case)(struct kunit *test);
>> const char *name;
>>
>> + /*
>> + * Pointer to test parameter generator function.
>> + * Used only for parameterized tests.
>
> What I meant was to give a description of the protocol, so that if
> somebody wanted, they could (without reading the implementation)
> implement their own custom generator without the helper macro.
>
> E.g. something like: "The generator function 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."
>
Oh okay. I am not sure if this is the best place to add documentation for this.
>> + */
>> + void* (*generate_params)(void *prev);
>> +
>> /* private: internal use only. */
>> bool success;
>> char *log;
>> @@ -162,6 +168,9 @@ static inline char *kunit_status_to_string(bool status)
>> * &struct kunit_case for an example on how to use it.
>> */
>> #define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name }
>> +#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 +217,15 @@ struct kunit {
>> const char *name; /* Read only after initialization! */
>> char *log; /* Points at case log after initialization */
>> struct kunit_try_catch try_catch;
>> + /* param_values points to test case parameters in parameterized tests */
>> + void *param_values;
>> + /*
>> + * current_param stores the index of the parameter in
>> + * the array of parameters in parameterized tests.
>> + * current_param + 1 is printed to indicate the parameter
>> + * that causes the test to fail in case of test failure.
>> + */
>> + int current_param;
>> /*
>> * 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 +1760,18 @@ do { \
>> fmt, \
>> ##__VA_ARGS__)
>>
>> +/**
>> + * KUNIT_PARAM_GENERATOR() - Helper method for test parameter generators
>> + * required in parameterized tests.
>
> This is only for arrays, which is why I suggested KUNIT_ARRAY_PARAM()
> as the name.
>
> A generator can very well be implemented without an array, so this
> macro name is confusing. In future somebody might want to provide a
> macro that takes a start + end value (and maybe a step value) to
> generate a series of values. That generator could be named
> KUNIT_RANGE_PARAM(name, start, end, step) and gives us a generator
> that is also named name##_gen_params. (If you want to try implementing
> that macro, I'd suggest doing it as a separate patch.)
>
> And I don't think we need to put "GENERATOR" into the name of these
> macros, because the generators are now the fundamental method with
> which to get parameterized tests. We don't need to state the obvious,
> in favor of some brevity.
>
Okay, makes sense. I will change it to KUNIT_ARRAY_PARAM() for the next version.
>> + * @name: prefix of the name for the test parameter generator function.
>> + * @prev: a pointer to the previous test parameter, NULL for first parameter.
>> + * @array: a user-supplied pointer to an array of test parameters.
>> + */
>> +#define KUNIT_PARAM_GENERATOR(name, array) \
>> + static void *name##_gen_params(void *prev) \
>> + { \
>> + typeof((array)[0]) * __next = prev ? ((typeof(__next)) prev) + 1 : (array); \
>> + return __next - (array) < ARRAY_SIZE((array)) ? __next : NULL; \
>> + }
>> +
>> #endif /* _KUNIT_TEST_H */
>
> Thanks,
> -- Marco
>
Thanks!
Powered by blists - more mailing lists