| Message ID | 20231220151952.415232-2-benjamin@sipsolutions.net |
|---|---|
| State | New |
| Headers | show |
| Series | Add some more cfg80211 and mac80211 kunit tests | expand |
On Wed, 20 Dec 2023 at 23:20, <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> > --- I'm generally pretty happy with this, though note the checkpatch warning below. There was some discussion at plumbers about expanding the parameterised test APIs, so we may need to adjust the implementation of this down the line, but I don't think that'll happen for a while, so don't worry. With the warnings fixed, this is: Reviewed-by: David Gow <davidgow@google.com> I'm okay with this going in via the wireless tree if that's easier; certainly there are some conflicts with the later patches in this series and the kunit one. Cheers, -- David > Documentation/dev-tools/kunit/usage.rst | 12 ++++-------- > include/kunit/test.h | 19 +++++++++++++++++++ > 2 files changed, 23 insertions(+), 8 deletions(-) > > diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst > index c27e1646ecd9..b959e5befcbe 100644 > --- a/Documentation/dev-tools/kunit/usage.rst > +++ b/Documentation/dev-tools/kunit/usage.rst > @@ -566,13 +566,9 @@ By reusing the same ``cases`` array from above, we can write the test as a > }, > }; > > - // Need a helper function to generate a name for each test case. > - static void case_to_desc(const struct sha1_test_case *t, char *desc) > - { > - 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); > > // Looks no different from a normal test. > static void sha1_test(struct kunit *test) > @@ -588,7 +584,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 pass in the > - // function declared by KUNIT_ARRAY_PARAM. > + // function declared by KUNIT_ARRAY_PARAM or KUNIT_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 20ed9f9275c9..2dfa851e1f88 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -1514,6 +1514,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); \ checkpatch is complaining here: ERROR: need consistent spacing around '*' (ctx:WxV) #71: FILE: include/kunit/test.h:1528: + 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; \ > + } \ > + return NULL; \ > + } > + > // TODO(dlatypov@google.com): consider eventually migrating users to explicitly > // include resource.h themselves if they need it. > #include <kunit/resource.h> > -- > 2.43.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/20231220151952.415232-2-benjamin%40sipsolutions.net.
On Fri, 2023-12-22 at 18:02 +0800, David Gow wrote: > On Wed, 20 Dec 2023 at 23:20, <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> > > --- > > I'm generally pretty happy with this, though note the checkpatch > warning below. > > There was some discussion at plumbers about expanding the > parameterised test APIs, so we may need to adjust the implementation > of this down the line, but I don't think that'll happen for a while, > so don't worry. > > With the warnings fixed, this is: I think the checkpatch warning is a false positive. It seems to confuse the * as a multiplication due to typeof() looking like a function call rather than a variable declaration. Benjamin > Reviewed-by: David Gow <davidgow@google.com> > > I'm okay with this going in via the wireless tree if that's easier; > certainly there are some conflicts with the later patches in this > series and the kunit one. > > Cheers, > -- David > > > Documentation/dev-tools/kunit/usage.rst | 12 ++++-------- > > include/kunit/test.h | 19 +++++++++++++++++++ > > 2 files changed, 23 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/dev-tools/kunit/usage.rst > > b/Documentation/dev-tools/kunit/usage.rst > > index c27e1646ecd9..b959e5befcbe 100644 > > --- a/Documentation/dev-tools/kunit/usage.rst > > +++ b/Documentation/dev-tools/kunit/usage.rst > > @@ -566,13 +566,9 @@ By reusing the same ``cases`` array from > > above, we can write the test as a > > }, > > }; > > > > - // Need a helper function to generate a name for each test > > case. > > - static void case_to_desc(const struct sha1_test_case *t, > > char *desc) > > - { > > - 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); > > > > // Looks no different from a normal test. > > static void sha1_test(struct kunit *test) > > @@ -588,7 +584,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 pass > > in the > > - // function declared by KUNIT_ARRAY_PARAM. > > + // function declared by KUNIT_ARRAY_PARAM or > > KUNIT_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 20ed9f9275c9..2dfa851e1f88 100644 > > --- a/include/kunit/test.h > > +++ b/include/kunit/test.h > > @@ -1514,6 +1514,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); \ > > checkpatch is complaining here: > ERROR: need consistent spacing around '*' (ctx:WxV) > #71: FILE: include/kunit/test.h:1528: > > + 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; \ > > + > > } > > \ > > + return > > NULL; > > \ > > + } > > + > > // TODO(dlatypov@google.com): consider eventually migrating users > > to explicitly > > // include resource.h themselves if they need it. > > #include <kunit/resource.h> > > -- > > 2.43.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/20231220151952.415232-2-benjamin%40sipsolutions.net > > .
On Fri, 22 Dec 2023 at 23:09, Benjamin Berg <benjamin@sipsolutions.net> wrote: > > On Fri, 2023-12-22 at 18:02 +0800, David Gow wrote: > > On Wed, 20 Dec 2023 at 23:20, <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> > > > --- > > > > I'm generally pretty happy with this, though note the checkpatch > > warning below. > > > > There was some discussion at plumbers about expanding the > > parameterised test APIs, so we may need to adjust the implementation > > of this down the line, but I don't think that'll happen for a while, > > so don't worry. > > > > With the warnings fixed, this is: > > I think the checkpatch warning is a false positive. It seems to confuse > the * as a multiplication due to typeof() looking like a function call > rather than a variable declaration. > > Benjamin > Ah, yeah: this appears to be due to checkpatch not handling nested () within a typeof(). Reviewed-by: David Gow <davidgow@google.com> Cheers, -- David
diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst index c27e1646ecd9..b959e5befcbe 100644 --- a/Documentation/dev-tools/kunit/usage.rst +++ b/Documentation/dev-tools/kunit/usage.rst @@ -566,13 +566,9 @@ By reusing the same ``cases`` array from above, we can write the test as a }, }; - // Need a helper function to generate a name for each test case. - static void case_to_desc(const struct sha1_test_case *t, char *desc) - { - 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); // Looks no different from a normal test. static void sha1_test(struct kunit *test) @@ -588,7 +584,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 pass in the - // function declared by KUNIT_ARRAY_PARAM. + // function declared by KUNIT_ARRAY_PARAM or KUNIT_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 20ed9f9275c9..2dfa851e1f88 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -1514,6 +1514,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; \ + } \ + return NULL; \ + } + // TODO(dlatypov@google.com): consider eventually migrating users to explicitly // include resource.h themselves if they need it. #include <kunit/resource.h>