diff mbox series

[1/3] kunit/test: Add example test showing parameterized testing

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

Commit Message

Michal Wajdeczko April 11, 2023, 4 p.m. UTC
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(+)

Comments

Rae Moar April 12, 2023, 8:51 p.m. UTC | #1
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.
David Gow April 13, 2023, 6:27 a.m. UTC | #2
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.
David Gow April 13, 2023, 6:30 a.m. UTC | #3
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 mbox series

Patch

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),
 	{}
 };