Re: [PATCH 2/2] kunit: Allow to filter entries from zero terminated arrays

2023-09-30 Thread David Gow
On Wed, 27 Sept 2023 at 06:02, Michal Wajdeczko
 wrote:
>
> In some cases we may want to generate params based on existing
> zero terminated array, but with some entries filtered out.
> Extend macro KUNIT_ZERO_ARRAY_PARAM to accept filter function
> and provide example how to use it.
>
> $ ./tools/testing/kunit/kunit.py run \
> --kunitconfig ./lib/kunit/.kunitconfig *.example_params*
>
> [ ] Starting KUnit Kernel (1/1)...
> [ ] 
> [ ] = example  =
> [ ] === example_params_test  ===
> [ ] [SKIPPED] example value 3
> [ ] [PASSED] example value 2
> [ ] [PASSED] example value 1
> [ ] [SKIPPED] example value 0
> [ ] === [PASSED] example_params_test ===
> [ ] === example_params_test  ===
> [ ] [SKIPPED] example value 3
> [ ] [PASSED] example value 2
> [ ] [PASSED] example value 1
> [ ] === [PASSED] example_params_test ===
> [ ] === example_params_test  ===
> [ ] [PASSED] example value 2
> [ ] [PASSED] example value 1
> [ ] === [PASSED] example_params_test ===
> [ ] = [PASSED] example =
> [ ] 
> [ ] Testing complete. Ran 9 tests: passed: 6, skipped: 3
>
> Signed-off-by: Michal Wajdeczko 
> Cc: David Gow 
> Cc: Rae Moar 
> ---

I tend to agree with Rae here that this is:
a) starting to get quite specific, and might be better served by a
per-test generator function than a generic helper here, and
b) probably would need to be implemented for both  ZERO_ARRAY_PARAM
and the regular ARRAY_PARAM, for consistency's sake.

I'd probably err on the side of letting tests implement something like
this themselves, and then reconsider a helper macro once there are
several tests all using the same thing. If, for example, all the users
of this are just checking PCI ids, or something, it may make more
sense to just have a PCI-specific macro they can share.

Regardless, I'd love to see a real-world example of this being used,
which may make it seem less niche.

Thanks,
-- David

>  include/kunit/test.h   | 19 +--
>  lib/kunit/kunit-example-test.c |  9 +
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 280113ceb6a6..8a87d1ce37e0 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -1515,20 +1515,24 @@ do {  
>  \
> }
>
>  /**
> - * KUNIT_ZERO_ARRAY_PARAM() - Define test parameter generator from a zero 
> terminated array.
> + * KUNIT_FILTERED_ZERO_ARRAY_PARAM() - Define test parameter generator from 
> a zero terminated array.
>   * @name:  prefix for the test parameter generator function.
>   * @array: zero terminated array of test parameters.
>   * @get_desc: function to convert param to description; NULL to use default
> + * @filter: function to filter out unwanted params (like duplicates); can be 
> NULL
>   *
>   * Define function @name_gen_params which uses zero terminated @array to 
> generate parameters.
>   */
> -#define KUNIT_ZERO_ARRAY_PARAM(name, array, get_desc)
>   \
> +#define KUNIT_FILTERED_ZERO_ARRAY_PARAM(name, array, get_desc, filter)   
>   \
> static const void *name##_gen_params(const void *prev, char *desc)
>   \
> { 
>   \
> typeof((array)[0]) *__prev = prev;
>   \
> typeof(__prev) __next = __prev ? __prev + 1 : (array);
>   \
> void (*__get_desc)(typeof(__next), char *) = get_desc;
>   \
> +   bool (*__filter)(typeof(__prev), typeof(__next)) = filter;
>   \
> for (; memchr_inv(__next, 0, sizeof(*__next)); __prev = 
> __next++) { \
> +   if (__filter && !__filter(__prev, __next))
>   \
> +   continue; 
>   \
> if (__get_desc)   
>   \
> __get_desc(__next, desc); 
>   \
> return __next;
>   \
> @@ -1536,6 +1540,17 @@ do {   
>  \
> return NULL;  
>   \
> }
>
> 

Re: [PATCH 2/2] kunit: Allow to filter entries from zero terminated arrays

2023-09-29 Thread Rae Moar
On Tue, Sep 26, 2023 at 6:02 PM Michal Wajdeczko
 wrote:
>
> In some cases we may want to generate params based on existing
> zero terminated array, but with some entries filtered out.
> Extend macro KUNIT_ZERO_ARRAY_PARAM to accept filter function
> and provide example how to use it.

Hello!

I definitely understand the use case of wanting to filter params.
However, since this is a static filter, it seems this could be done in
the test file and rather than implemented as a new KUnit feature.

I would be interested to see David's thoughts on this. If we do decide
to implement this as a KUnit feature, I would also prefer if the
filtering ability is available for both the zero-terminated param
arrays and normal param arrays.

Otherwise I just have one comment below. Let me know what you think.

Thanks!
-Rae

>
> $ ./tools/testing/kunit/kunit.py run \
> --kunitconfig ./lib/kunit/.kunitconfig *.example_params*
>
> [ ] Starting KUnit Kernel (1/1)...
> [ ] 
> [ ] = example  =
> [ ] === example_params_test  ===
> [ ] [SKIPPED] example value 3
> [ ] [PASSED] example value 2
> [ ] [PASSED] example value 1
> [ ] [SKIPPED] example value 0
> [ ] === [PASSED] example_params_test ===
> [ ] === example_params_test  ===
> [ ] [SKIPPED] example value 3
> [ ] [PASSED] example value 2
> [ ] [PASSED] example value 1
> [ ] === [PASSED] example_params_test ===
> [ ] === example_params_test  ===
> [ ] [PASSED] example value 2
> [ ] [PASSED] example value 1
> [ ] === [PASSED] example_params_test ===
> [ ] = [PASSED] example =
> [ ] 
> [ ] Testing complete. Ran 9 tests: passed: 6, skipped: 3
>
> Signed-off-by: Michal Wajdeczko 
> Cc: David Gow 
> Cc: Rae Moar 
> ---
>  include/kunit/test.h   | 19 +--
>  lib/kunit/kunit-example-test.c |  9 +
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 280113ceb6a6..8a87d1ce37e0 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -1515,20 +1515,24 @@ do {  
>  \
> }
>
>  /**
> - * KUNIT_ZERO_ARRAY_PARAM() - Define test parameter generator from a zero 
> terminated array.
> + * KUNIT_FILTERED_ZERO_ARRAY_PARAM() - Define test parameter generator from 
> a zero terminated array.
>   * @name:  prefix for the test parameter generator function.
>   * @array: zero terminated array of test parameters.
>   * @get_desc: function to convert param to description; NULL to use default
> + * @filter: function to filter out unwanted params (like duplicates); can be 
> NULL
>   *
>   * Define function @name_gen_params which uses zero terminated @array to 
> generate parameters.
>   */
> -#define KUNIT_ZERO_ARRAY_PARAM(name, array, get_desc)
>   \
> +#define KUNIT_FILTERED_ZERO_ARRAY_PARAM(name, array, get_desc, filter)   
>   \
> static const void *name##_gen_params(const void *prev, char *desc)
>   \
> { 
>   \
> typeof((array)[0]) *__prev = prev;
>   \
> typeof(__prev) __next = __prev ? __prev + 1 : (array);
>   \
> void (*__get_desc)(typeof(__next), char *) = get_desc;
>   \
> +   bool (*__filter)(typeof(__prev), typeof(__next)) = filter;
>   \
> for (; memchr_inv(__next, 0, sizeof(*__next)); __prev = 
> __next++) { \
> +   if (__filter && !__filter(__prev, __next))
>   \
> +   continue; 
>   \
> if (__get_desc)   
>   \
> __get_desc(__next, desc); 
>   \
> return __next;
>   \
> @@ -1536,6 +1540,17 @@ do {   
>  \
> return NULL;  
>   \
> }
>
> +/**
> + * KUNIT_ZERO_ARRAY_PARAM() - Define test parameter generator from a zero 
> terminated array.
> + * @name:  prefix for the test parameter generator function.
> + * @array: zero terminated array of test