If we run parameterized test that uses test->priv to prepare some custom data, then value of test->priv will leak to the next param iteration and may be unexpected.
Cc: David Gow davidgow@google.com Cc: Rae Moar rmoar@google.com
Michal Wajdeczko (2): kunit: Add example for using test->priv kunit: Reset test->priv after each param iteration
lib/kunit/kunit-example-test.c | 15 +++++++++++++++ lib/kunit/test.c | 1 + 2 files changed, 16 insertions(+)
In a test->priv field the user can store arbitrary data. Add example how to use this feature in the test code.
Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com Cc: David Gow davidgow@google.com Cc: Rae Moar rmoar@google.com --- lib/kunit/kunit-example-test.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c index 6bb5c2ef6696..d7dfcf209ece 100644 --- a/lib/kunit/kunit-example-test.c +++ b/lib/kunit/kunit-example-test.c @@ -221,6 +221,20 @@ static void example_params_test(struct kunit *test) KUNIT_EXPECT_EQ(test, param->value % param->value, 0); }
+/* + * This test shows the use of test->priv. + */ +static void example_priv_test(struct kunit *test) +{ + /* unless setup in suite->init(), test->priv is NULL */ + KUNIT_ASSERT_NULL(test, test->priv); + + /* but can be used to pass arbitrary data to other functions */ + test->priv = kunit_kzalloc(test, 1, GFP_KERNEL); + KUNIT_EXPECT_NOT_NULL(test, test->priv); + KUNIT_ASSERT_PTR_EQ(test, test->priv, kunit_get_current_test()->priv); +} + /* * This test should always pass. Can be used to practice filtering attributes. */ @@ -245,6 +259,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(example_priv_test), KUNIT_CASE_PARAM(example_params_test, example_gen_params), KUNIT_CASE_SLOW(example_slow_test), {}
On Fri, 15 Dec 2023 at 23:13, Michal Wajdeczko michal.wajdeczko@intel.com wrote:
In a test->priv field the user can store arbitrary data. Add example how to use this feature in the test code.
Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com Cc: David Gow davidgow@google.com Cc: Rae Moar rmoar@google.com
Looks good to me.
Reviewed-by: David Gow davidgow@google.com
Cheers, -- David
lib/kunit/kunit-example-test.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c index 6bb5c2ef6696..d7dfcf209ece 100644 --- a/lib/kunit/kunit-example-test.c +++ b/lib/kunit/kunit-example-test.c @@ -221,6 +221,20 @@ static void example_params_test(struct kunit *test) KUNIT_EXPECT_EQ(test, param->value % param->value, 0); }
+/*
- This test shows the use of test->priv.
- */
+static void example_priv_test(struct kunit *test) +{
/* unless setup in suite->init(), test->priv is NULL */
KUNIT_ASSERT_NULL(test, test->priv);
/* but can be used to pass arbitrary data to other functions */
test->priv = kunit_kzalloc(test, 1, GFP_KERNEL);
KUNIT_EXPECT_NOT_NULL(test, test->priv);
KUNIT_ASSERT_PTR_EQ(test, test->priv, kunit_get_current_test()->priv);
+}
/*
- This test should always pass. Can be used to practice filtering attributes.
*/ @@ -245,6 +259,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(example_priv_test), KUNIT_CASE_PARAM(example_params_test, example_gen_params), KUNIT_CASE_SLOW(example_slow_test), {}
-- 2.25.1
If we run parameterized test that uses test->priv to prepare some custom data, then value of test->priv will leak to the next param iteration and may be unexpected. This could be easily seen if we promote example_priv_test to parameterized test as then only first test iteration will be successful:
$ ./tools/testing/kunit/kunit.py run \ --kunitconfig ./lib/kunit/.kunitconfig *.example_priv*
[ ] Starting KUnit Kernel (1/1)... [ ] ============================================================ [ ] =================== example (1 subtest) ==================== [ ] ==================== example_priv_test ==================== [ ] [PASSED] example value 3 [ ] # example_priv_test: initializing [ ] # example_priv_test: ASSERTION FAILED at lib/kunit/kunit-example-test.c:230 [ ] Expected test->priv == ((void *)0), but [ ] test->priv == 0000000060dfe290 [ ] ((void *)0) == 0000000000000000 [ ] # example_priv_test: cleaning up [ ] [FAILED] example value 2 [ ] # example_priv_test: initializing [ ] # example_priv_test: ASSERTION FAILED at lib/kunit/kunit-example-test.c:230 [ ] Expected test->priv == ((void *)0), but [ ] test->priv == 0000000060dfe290 [ ] ((void *)0) == 0000000000000000 [ ] # example_priv_test: cleaning up [ ] [FAILED] example value 1 [ ] # example_priv_test: initializing [ ] # example_priv_test: ASSERTION FAILED at lib/kunit/kunit-example-test.c:230 [ ] Expected test->priv == ((void *)0), but [ ] test->priv == 0000000060dfe290 [ ] ((void *)0) == 0000000000000000 [ ] # example_priv_test: cleaning up [ ] [FAILED] example value 0 [ ] # example_priv_test: initializing [ ] # example_priv_test: cleaning up [ ] # example_priv_test: pass:1 fail:3 skip:0 total:4 [ ] ================ [FAILED] example_priv_test ================ [ ] # example: initializing suite [ ] # module: kunit_example_test [ ] # example: exiting suite [ ] # Totals: pass:1 fail:3 skip:0 total:4 [ ] ===================== [FAILED] example =====================
Fix that by resetting test->priv after each param iteration, in similar way what we did for the test->status.
Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com Cc: David Gow davidgow@google.com Cc: Rae Moar rmoar@google.com --- lib/kunit/test.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 0308865194bb..7255ca297399 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -622,6 +622,7 @@ int kunit_run_tests(struct kunit_suite *suite) test.param_index++; test.status = KUNIT_SUCCESS; test.status_comment[0] = '\0'; + test.priv = NULL; } }
On Fri, 15 Dec 2023 at 23:13, Michal Wajdeczko michal.wajdeczko@intel.com wrote:
If we run parameterized test that uses test->priv to prepare some custom data, then value of test->priv will leak to the next param iteration and may be unexpected. This could be easily seen if we promote example_priv_test to parameterized test as then only first test iteration will be successful:
$ ./tools/testing/kunit/kunit.py run \ --kunitconfig ./lib/kunit/.kunitconfig *.example_priv*
[ ] Starting KUnit Kernel (1/1)... [ ] ============================================================ [ ] =================== example (1 subtest) ==================== [ ] ==================== example_priv_test ==================== [ ] [PASSED] example value 3 [ ] # example_priv_test: initializing [ ] # example_priv_test: ASSERTION FAILED at lib/kunit/kunit-example-test.c:230 [ ] Expected test->priv == ((void *)0), but [ ] test->priv == 0000000060dfe290 [ ] ((void *)0) == 0000000000000000 [ ] # example_priv_test: cleaning up [ ] [FAILED] example value 2 [ ] # example_priv_test: initializing [ ] # example_priv_test: ASSERTION FAILED at lib/kunit/kunit-example-test.c:230 [ ] Expected test->priv == ((void *)0), but [ ] test->priv == 0000000060dfe290 [ ] ((void *)0) == 0000000000000000 [ ] # example_priv_test: cleaning up [ ] [FAILED] example value 1 [ ] # example_priv_test: initializing [ ] # example_priv_test: ASSERTION FAILED at lib/kunit/kunit-example-test.c:230 [ ] Expected test->priv == ((void *)0), but [ ] test->priv == 0000000060dfe290 [ ] ((void *)0) == 0000000000000000 [ ] # example_priv_test: cleaning up [ ] [FAILED] example value 0 [ ] # example_priv_test: initializing [ ] # example_priv_test: cleaning up [ ] # example_priv_test: pass:1 fail:3 skip:0 total:4 [ ] ================ [FAILED] example_priv_test ================ [ ] # example: initializing suite [ ] # module: kunit_example_test [ ] # example: exiting suite [ ] # Totals: pass:1 fail:3 skip:0 total:4 [ ] ===================== [FAILED] example =====================
Fix that by resetting test->priv after each param iteration, in similar way what we did for the test->status.
Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com Cc: David Gow davidgow@google.com Cc: Rae Moar rmoar@google.com
Looks good to me. I'd vaguely assumed we'd treat test.priv as having an undefined value on test start, but thinking about it, I like making it explicitly NULL better.
Reviewed-by: David Gow davidgow@google.com
Cheers, -- David
linux-kselftest-mirror@lists.linaro.org