On Wed, 6 Sept 2023 at 15:07, Berg, Benjamin benjamin.berg@intel.com wrote:
Hi,
On Wed, 2023-09-06 at 14:45 +0800, David Gow wrote:
On Mon, 4 Sept 2023 at 21:22, benjamin@sipsolutions.net wrote:
From: Benjamin Berg benjamin.berg@intel.com
The existing KUNIT_ARRAY_PARAM macro requires a separate function to get the description. However, in a lot of cases the description can just be copied directly from the array. Add a second macro that avoids having to write a static function just for a single strscpy.
Signed-off-by: Benjamin Berg benjamin.berg@intel.com
Looks good to me: this will be much more convenient. The actual implementation looks spot on, just a small comment about the documentation change.
It may make sense to write some tests and/or some follow-up patches to existing tests to use this macro, too. I'm just a little wary of introducing something totally unused. (I'm happy to do these myself if you don't have time, though.)
I agree. I am happy to submit one or more patches to change the existing users. The question would be how we pull such a change in. Should it be submitted separately for each subtree or can we pull them all in at the same time here?
Benjamin
It depends a little bit on the test being modified: if it rarely sees conflicting changes, we can pull it in via KUnit, if it's being actively modified a lot, it's best to send it through separately.
If you're not sure, just include them all in a series here, CC the test maintainers, and ask what tree they'd prefer it to go in via.
It all usually works out in the end, and worst-case, if we miss one or two tests, we can update them separately.
Cheers, -- David
Regardless, with the documentation fix, this is: Reviewed-by: David Gow davidgow@google.com
Cheers, -- David
Documentation/dev-tools/kunit/usage.rst | 7 ++++--- include/kunit/test.h | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst index c27e1646ecd9..fe8c28d66dfe 100644 --- a/Documentation/dev-tools/kunit/usage.rst +++ b/Documentation/dev-tools/kunit/usage.rst @@ -571,8 +571,9 @@ By reusing the same ``cases`` array from above, we can write the test as a { strcpy(desc, t->str); }
// Creates `sha1_gen_params()` to iterate over `cases`.KUNIT_ARRAY_PARAM(sha1, cases, case_to_desc);
// Creates `sha1_gen_params()` to iterate over `cases`while using
// the struct member `str` for the case description.KUNIT_ARRAY_PARAM_DESC(sha1, cases, str);I'd suggest either getting rid of the case_to_desc function totally here, or show both the manual KUNIT_ARRAY_PARAM() example, and then point out explicitly that KUNIT_ARRAY_PARAM_DESC() can replace it.
Otherwise we end up with a vestigial function which doesn't do anything and is confusing.
// Looks no different from a normal test. static void sha1_test(struct kunit *test)@@ -588,7 +589,7 @@ By reusing the same ``cases`` array from above, we can write the test as a }
// Instead of KUNIT_CASE, we use KUNIT_CASE_PARAM and passin the
// function declared by KUNIT_ARRAY_PARAM.
// function declared by KUNIT_ARRAY_PARAM orKUNIT_ARRAY_PARAM_DESC. static struct kunit_case sha1_test_cases[] = { KUNIT_CASE_PARAM(sha1_test, sha1_gen_params), {} diff --git a/include/kunit/test.h b/include/kunit/test.h index 68ff01aee244..f60d11e41855 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -1516,6 +1516,25 @@ do { \ return NULL; \ }
+/**
- KUNIT_ARRAY_PARAM_DESC() - Define test parameter generator from
an array.
- @name: prefix for the test parameter generator function.
- @array: array of test parameters.
- @desc_member: structure member from array element to use as
description
- Define function @name_gen_params which uses @array to generate
parameters.
- */
+#define KUNIT_ARRAY_PARAM_DESC(name, array, desc_member) \
static const void *name##_gen_params(const void *prev, char*desc) \
{ \
typeof((array)[0]) *__next = prev ?((typeof(__next)) prev) + 1 : (array); \
if (__next - (array) < ARRAY_SIZE((array))){ \
strscpy(desc, __next->desc_member,KUNIT_PARAM_DESC_SIZE); \
return__next; \
} \
returnNULL; \
}// TODO(dlatypov@google.com): consider eventually migrating users to explicitly // include resource.h themselves if they need it.
#include <kunit/resource.h>
2.41.0
-- 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@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230904132139.103140-1-benjamin... .
Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de http://www.intel.de Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928