Message ID | 20230411160056.1586-2-michal.wajdeczko@intel.com (mailing list archive) |
---|---|
State | Accepted |
Commit | d273b72846d636a7a9072587b5c53e7c0aeb791b |
Delegated to: | Brendan Higgins |
Headers | show |
Series | kunit: Fix reporting of the skipped parameterized tests | expand |
On Tue, Apr 11, 2023 at 12:01 PM Michal Wajdeczko <michal.wajdeczko@intel.com> wrote: > > Use of parameterized testing is documented [1] but such use case > is not present in demo kunit test. Add small subtest for that. > > [1] https://kernel.org/doc/html/latest/dev-tools/kunit/usage.html#parameterized-testing > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: David Gow <davidgow@google.com> > --- Hello! This looks all pretty good to me! I only have one comment. In the KTAP output: KTAP version 1 # Subtest: example_params_test # example_params_test: initializing ok 1 example value 2 # example_params_test: initializing ok 2 example value 1 # example_params_test: initializing ok 3 example value 0 # SKIP unsupported param value # example_params_test: pass:2 fail:0 skip:1 total:3 ok 6 example_params_test The init method is causing the "# example_params_test: initializing" to print lines for each case. However, since they are not inline with the correct indentation and they don't include helpful test data, I would consider finding a way to remove these. We could consider removing these lines from the test suite as a whole. However, they are helpful in that they show how to use the init function. Maybe check if the test is a param test case in the init function itself? Let me know what you think. Thanks! Rae > lib/kunit/kunit-example-test.c | 34 ++++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c > index cd8b7e51d02b..775443f77763 100644 > --- a/lib/kunit/kunit-example-test.c > +++ b/lib/kunit/kunit-example-test.c > @@ -167,6 +167,39 @@ static void example_static_stub_test(struct kunit *test) > KUNIT_EXPECT_EQ(test, add_one(1), 2); > } > > +static const struct example_param { > + int value; > +} example_params_array[] = { > + { .value = 2, }, > + { .value = 1, }, > + { .value = 0, }, > +}; > + > +static void example_param_get_desc(const struct example_param *p, char *desc) > +{ > + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "example value %d", p->value); > +} > + > +KUNIT_ARRAY_PARAM(example, example_params_array, example_param_get_desc); > + > +/* > + * This test shows the use of params. > + */ > +static void example_params_test(struct kunit *test) > +{ > + const struct example_param *param = test->param_value; > + > + /* By design, param pointer will not be NULL */ > + KUNIT_ASSERT_NOT_NULL(test, param); > + > + /* Test can be skipped on unsupported param values */ > + if (!param->value) > + kunit_skip(test, "unsupported param value"); > + > + /* You can use param values for parameterized testing */ > + KUNIT_EXPECT_EQ(test, param->value % param->value, 0); > +} > + > /* > * Here we make a list of all the test cases we want to add to the test suite > * below. > @@ -183,6 +216,7 @@ static struct kunit_case example_test_cases[] = { > KUNIT_CASE(example_mark_skipped_test), > KUNIT_CASE(example_all_expect_macros_test), > KUNIT_CASE(example_static_stub_test), > + KUNIT_CASE_PARAM(example_params_test, example_gen_params), > {} > }; > > -- > 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@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230411160056.1586-2-michal.wajdeczko%40intel.com.
On Wed, 12 Apr 2023 at 00:01, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote: > > Use of parameterized testing is documented [1] but such use case > is not present in demo kunit test. Add small subtest for that. > > [1] https://kernel.org/doc/html/latest/dev-tools/kunit/usage.html#parameterized-testing > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: David Gow <davidgow@google.com> > --- Thanks: this is a long-overdue addition to the KUnit examples. Reviewed-by: David Gow <davidgow@google.com> Cheers, -- David > lib/kunit/kunit-example-test.c | 34 ++++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c > index cd8b7e51d02b..775443f77763 100644 > --- a/lib/kunit/kunit-example-test.c > +++ b/lib/kunit/kunit-example-test.c > @@ -167,6 +167,39 @@ static void example_static_stub_test(struct kunit *test) > KUNIT_EXPECT_EQ(test, add_one(1), 2); > } > > +static const struct example_param { > + int value; > +} example_params_array[] = { > + { .value = 2, }, > + { .value = 1, }, > + { .value = 0, }, > +}; > + > +static void example_param_get_desc(const struct example_param *p, char *desc) > +{ > + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "example value %d", p->value); > +} > + > +KUNIT_ARRAY_PARAM(example, example_params_array, example_param_get_desc); > + > +/* > + * This test shows the use of params. > + */ > +static void example_params_test(struct kunit *test) > +{ > + const struct example_param *param = test->param_value; > + > + /* By design, param pointer will not be NULL */ > + KUNIT_ASSERT_NOT_NULL(test, param); > + > + /* Test can be skipped on unsupported param values */ > + if (!param->value) > + kunit_skip(test, "unsupported param value"); > + > + /* You can use param values for parameterized testing */ > + KUNIT_EXPECT_EQ(test, param->value % param->value, 0); > +} > + > /* > * Here we make a list of all the test cases we want to add to the test suite > * below. > @@ -183,6 +216,7 @@ static struct kunit_case example_test_cases[] = { > KUNIT_CASE(example_mark_skipped_test), > KUNIT_CASE(example_all_expect_macros_test), > KUNIT_CASE(example_static_stub_test), > + KUNIT_CASE_PARAM(example_params_test, example_gen_params), > {} > }; > > -- > 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@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230411160056.1586-2-michal.wajdeczko%40intel.com.
On Thu, 13 Apr 2023 at 04:51, Rae Moar <rmoar@google.com> wrote: > > On Tue, Apr 11, 2023 at 12:01 PM Michal Wajdeczko > <michal.wajdeczko@intel.com> wrote: > > > > Use of parameterized testing is documented [1] but such use case > > is not present in demo kunit test. Add small subtest for that. > > > > [1] https://kernel.org/doc/html/latest/dev-tools/kunit/usage.html#parameterized-testing > > > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Cc: David Gow <davidgow@google.com> > > --- > > Hello! > > This looks all pretty good to me! I only have one comment. In the KTAP output: > > KTAP version 1 > # Subtest: example_params_test > # example_params_test: initializing > ok 1 example value 2 > # example_params_test: initializing > ok 2 example value 1 > # example_params_test: initializing > ok 3 example value 0 # SKIP unsupported param value > # example_params_test: pass:2 fail:0 skip:1 total:3 > ok 6 example_params_test > > The init method is causing the "# example_params_test: initializing" > to print lines for each case. However, since they are not inline with > the correct indentation and they don't include helpful test data, I > would consider finding a way to remove these. I think this is probably a problem for the kunit_log() infrastructure, rather than init functions in general. I'm not worried about them in relation to this particular test. > > We could consider removing these lines from the test suite as a whole. > However, they are helpful in that they show how to use the init > function. Maybe check if the test is a param test case in the init > function itself? Let me know what you think. > I'd rather keep these as-is: the idea is to have a very simple example of an init function, and complicating further by checking which test is running is needless complexity, IMO. -- David
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c index cd8b7e51d02b..775443f77763 100644 --- a/lib/kunit/kunit-example-test.c +++ b/lib/kunit/kunit-example-test.c @@ -167,6 +167,39 @@ static void example_static_stub_test(struct kunit *test) KUNIT_EXPECT_EQ(test, add_one(1), 2); } +static const struct example_param { + int value; +} example_params_array[] = { + { .value = 2, }, + { .value = 1, }, + { .value = 0, }, +}; + +static void example_param_get_desc(const struct example_param *p, char *desc) +{ + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "example value %d", p->value); +} + +KUNIT_ARRAY_PARAM(example, example_params_array, example_param_get_desc); + +/* + * This test shows the use of params. + */ +static void example_params_test(struct kunit *test) +{ + const struct example_param *param = test->param_value; + + /* By design, param pointer will not be NULL */ + KUNIT_ASSERT_NOT_NULL(test, param); + + /* Test can be skipped on unsupported param values */ + if (!param->value) + kunit_skip(test, "unsupported param value"); + + /* You can use param values for parameterized testing */ + KUNIT_EXPECT_EQ(test, param->value % param->value, 0); +} + /* * Here we make a list of all the test cases we want to add to the test suite * below. @@ -183,6 +216,7 @@ static struct kunit_case example_test_cases[] = { KUNIT_CASE(example_mark_skipped_test), KUNIT_CASE(example_all_expect_macros_test), KUNIT_CASE(example_static_stub_test), + KUNIT_CASE_PARAM(example_params_test, example_gen_params), {} };
Use of parameterized testing is documented [1] but such use case is not present in demo kunit test. Add small subtest for that. [1] https://kernel.org/doc/html/latest/dev-tools/kunit/usage.html#parameterized-testing Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: David Gow <davidgow@google.com> --- lib/kunit/kunit-example-test.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)