| Message ID | 20210831171926.3832806-1-dlatypov@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,1/2] kunit: drop assumption in kunit-log-test about current suite | expand |
On Tue, Aug 31, 2021 at 10:19 AM Daniel Latypov <dlatypov@google.com> wrote: > > Commit 1d71307a6f94 ("kunit: add unit test for filtering suites by > names") introduced the ability to filter which suites we run via glob. > > This change extends it so we can also filter individual test cases > inside of suites as well. > > This is quite useful when, e.g. > * trying to run just the tests cases you've just added or are working on > * trying to debug issues with test hermeticity > > Examples: > $ ./tools/testing/kunit/kunit.py run --kunitconfig=lib/kunit '*exec*.parse*' > ... > ============================================================ > ======== [PASSED] kunit_executor_test ======== > [PASSED] parse_filter_test > ============================================================ > Testing complete. 1 tests run. 0 failed. 0 crashed. > > $ ./tools/testing/kunit/kunit.py run --kunitconfig=lib/kunit '*.no_matching_tests' > ... > [ERROR] no tests run! > > Signed-off-by: Daniel Latypov <dlatypov@google.com> > Reviewed-by: David Gow <davidgow@google.com> Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index d69efcbed624..555601d17f79 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -415,12 +415,15 @@ static struct kunit_suite kunit_log_test_suite = { static void kunit_log_test(struct kunit *test) { - struct kunit_suite *suite = &kunit_log_test_suite; + struct kunit_suite suite; + + suite.log = kunit_kzalloc(test, KUNIT_LOG_SIZE, GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log); kunit_log(KERN_INFO, test, "put this in log."); kunit_log(KERN_INFO, test, "this too."); - kunit_log(KERN_INFO, suite, "add to suite log."); - kunit_log(KERN_INFO, suite, "along with this."); + kunit_log(KERN_INFO, &suite, "add to suite log."); + kunit_log(KERN_INFO, &suite, "along with this."); #ifdef CONFIG_KUNIT_DEBUGFS KUNIT_EXPECT_NOT_ERR_OR_NULL(test, @@ -428,12 +431,11 @@ static void kunit_log_test(struct kunit *test) KUNIT_EXPECT_NOT_ERR_OR_NULL(test, strstr(test->log, "this too.")); KUNIT_EXPECT_NOT_ERR_OR_NULL(test, - strstr(suite->log, "add to suite log.")); + strstr(suite.log, "add to suite log.")); KUNIT_EXPECT_NOT_ERR_OR_NULL(test, - strstr(suite->log, "along with this.")); + strstr(suite.log, "along with this.")); #else KUNIT_EXPECT_PTR_EQ(test, test->log, (char *)NULL); - KUNIT_EXPECT_PTR_EQ(test, suite->log, (char *)NULL); #endif }
This test assumes that the declared kunit_suite object is the exact one which is being executed, which KUnit will not guarantee [1]. Specifically, `suite->log` is not initialized until a suite object is executed. So if KUnit makes a copy of the suite and runs that instead, this test dereferences an invalid pointer and (hopefully) segfaults. N.B. since we no longer assume this, we can no longer verify that `suite->log` is *not* allocated during normal execution. An alternative to this patch that would allow us to test that would require exposing an API for the current test to get its current suite. Exposing that for one internal kunit test seems like overkill, and grants users more footguns (e.g. reusing a test case in multiple suites and changing behavior based on the suite name, dynamically modifying the setup/cleanup funcs, storing/reading stuff out of the suite->log, etc.). [1] In a subsequent patch, KUnit will allow running subsets of test cases within a suite by making a copy of the suite w/ the filtered test list. But there are other reasons KUnit might execute a copy, e.g. if it ever wants to support parallel execution of different suites, recovering from errors and restarting suites Signed-off-by: Daniel Latypov <dlatypov@google.com> --- lib/kunit/kunit-test.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) base-commit: 9c849ce86e0fa93a218614eac562ace44053d7ce