On Wed, Apr 27, 2022 at 2:19 AM Daniel Latypov <dlatypov@xxxxxxxxxx> wrote: > > KUnit has support for setup/cleanup logic for each test case in a suite. > But it lacks the ability to specify setup/cleanup for the entire suite > itself. > > This can be used to do setup that is too expensive or cumbersome to do > for each test. > Or it can be used to do simpler things like log debug information after > the suite completes. > It's a fairly common feature, so the lack of it is noticeable. > > Some examples in other frameworks and languages: > * https://docs.python.org/3/library/unittest.html#setupclass-and-teardownclass > * https://google.github.io/googletest/reference/testing.html#Test::SetUpTestSuite > > Meta: > This is very similar to this patch here: https://lore.kernel.org/linux-kselftest/20210805043503.20252-3-bvanassche@xxxxxxx/ > The changes from that patch: > * pass in `struct kunit *` so users can do stuff like > `kunit_info(suite, "debug message")` > * makes sure the init failure is bubbled up as a failure > * updates kunit-example-test.c to use a suite init > * Updates kunit/usage.rst to mention the new support > * some minor cosmetic things > * use `suite_{init,exit}` instead of `{init/exit}_suite` > * make suite init error message more consistent w/ test init > * etc. > > Signed-off-by: Daniel Latypov <dlatypov@xxxxxxxxxx> > --- Thanks for picking this up again: it's definitely something which has been obviously missing for a while. One comment below, but I don't mind if you'd prefer to leave things as-is. Reviewed-by: David Gow <davidgow@xxxxxxxxxx> > Documentation/dev-tools/kunit/usage.rst | 19 +++++++++++-------- > include/kunit/test.h | 4 ++++ > lib/kunit/kunit-example-test.c | 14 ++++++++++++++ > lib/kunit/test.c | 23 ++++++++++++++++++++--- > 4 files changed, 49 insertions(+), 11 deletions(-) > > diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst > index 1c83e7d60a8a..d62a04255c2e 100644 > --- a/Documentation/dev-tools/kunit/usage.rst > +++ b/Documentation/dev-tools/kunit/usage.rst > @@ -125,8 +125,8 @@ We need many test cases covering all the unit's behaviors. It is common to have > many similar tests. In order to reduce duplication in these closely related > tests, most unit testing frameworks (including KUnit) provide the concept of a > *test suite*. A test suite is a collection of test cases for a unit of code > -with a setup function that gets invoked before every test case and then a tear > -down function that gets invoked after every test case completes. For example: > +with optional setup and teardown functions that run before/after the whole > +suite and/or every test case. For example: > > .. code-block:: c > > @@ -141,16 +141,19 @@ down function that gets invoked after every test case completes. For example: > .name = "example", > .init = example_test_init, > .exit = example_test_exit, > + .suite_init = example_suite_init, > + .suite_exit = example_suite_exit, > .test_cases = example_test_cases, > }; > kunit_test_suite(example_test_suite); > > -In the above example, the test suite ``example_test_suite`` would run the test > -cases ``example_test_foo``, ``example_test_bar``, and ``example_test_baz``. Each > -would have ``example_test_init`` called immediately before it and > -``example_test_exit`` called immediately after it. > -``kunit_test_suite(example_test_suite)`` registers the test suite with the > -KUnit test framework. > +In the above example, the test suite ``example_test_suite`` would first run > +``example_suite_init``, then run the test cases ``example_test_foo``, > +``example_test_bar``, and ``example_test_baz``. Each would have > +``example_test_init`` called immediately before it and ``example_test_exit`` > +called immediately after it. Finally, ``example_suite_exit`` would be called > +after everything else. ``kunit_test_suite(example_test_suite)`` registers the > +test suite with the KUnit test framework. > > .. note:: > A test case will only run if it is associated with a test suite. > diff --git a/include/kunit/test.h b/include/kunit/test.h > index 97cd76461886..5d288f3d8f68 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -153,6 +153,8 @@ static inline char *kunit_status_to_ok_not_ok(enum kunit_status status) > * struct kunit_suite - describes a related collection of &struct kunit_case > * > * @name: the name of the test. Purely informational. > + * @suite_init: called once per test suite before the test cases. > + * @suite_exit: called once per test suite after all test cases. > * @init: called before every test case. > * @exit: called after every test case. > * @test_cases: a null terminated array of test cases. > @@ -167,6 +169,8 @@ static inline char *kunit_status_to_ok_not_ok(enum kunit_status status) > */ > struct kunit_suite { > const char name[256]; > + int (*suite_init)(struct kunit_suite *suite); > + void (*suite_exit)(struct kunit_suite *suite); > int (*init)(struct kunit *test); > void (*exit)(struct kunit *test); > struct kunit_case *test_cases; > diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c > index 91b1df7f59ed..f8fe582c9e36 100644 > --- a/lib/kunit/kunit-example-test.c > +++ b/lib/kunit/kunit-example-test.c > @@ -40,6 +40,17 @@ static int example_test_init(struct kunit *test) > return 0; > } > > +/* > + * This is run once before all test cases in the suite. > + * See the comment on example_test_suite for more information. > + */ > +static int example_test_init_suite(struct kunit_suite *suite) > +{ > + kunit_info(suite, "initializing suite\n"); > + > + return 0; > +} > + > /* > * This test should always be skipped. > */ > @@ -142,17 +153,20 @@ static struct kunit_case example_test_cases[] = { > * may be specified which runs after every test case and can be used to for > * cleanup. For clarity, running tests in a test suite would behave as follows: > * > + * suite.suite_init(suite); > * suite.init(test); > * suite.test_case[0](test); > * suite.exit(test); > * suite.init(test); > * suite.test_case[1](test); > * suite.exit(test); > + * suite.suite_exit(suite); > * ...; > */ > static struct kunit_suite example_test_suite = { > .name = "example", > .init = example_test_init, > + .suite_init = example_test_init_suite, > .test_cases = example_test_cases, > }; > > diff --git a/lib/kunit/test.c b/lib/kunit/test.c > index 64ee6a9d8003..b66e395c795a 100644 > --- a/lib/kunit/test.c > +++ b/lib/kunit/test.c > @@ -192,10 +192,13 @@ EXPORT_SYMBOL_GPL(kunit_suite_has_succeeded); > > static size_t kunit_suite_counter = 1; > > -static void kunit_print_suite_end(struct kunit_suite *suite) > +static void kunit_print_suite_end(struct kunit_suite *suite, int init_err) A part of me feels that it'd be nicer to have the init_err be part of struct kunit_suite, and have kunit_suite_has_succeeded() take it into account. It could go either way, though -- WDYT? > { > + enum kunit_status status = > + init_err ? KUNIT_FAILURE : kunit_suite_has_succeeded(suite); > + > kunit_print_ok_not_ok((void *)suite, false, > - kunit_suite_has_succeeded(suite), > + status, > kunit_suite_counter++, > suite->name, > suite->status_comment); > @@ -497,6 +500,16 @@ int kunit_run_tests(struct kunit_suite *suite) > struct kunit_case *test_case; > struct kunit_result_stats suite_stats = { 0 }; > struct kunit_result_stats total_stats = { 0 }; > + int suite_init_err = 0; > + > + if (suite->suite_init) { > + suite_init_err = suite->suite_init(suite); > + if (suite_init_err != 0) { > + kunit_err(suite, KUNIT_SUBTEST_INDENT > + "# failed to initialize (%d)", suite_init_err); > + goto suite_end; > + } > + } > > kunit_print_suite_start(suite); > > @@ -551,8 +564,12 @@ int kunit_run_tests(struct kunit_suite *suite) > kunit_accumulate_stats(&total_stats, param_stats); > } > > + if (suite->suite_exit) > + suite->suite_exit(suite); > + > kunit_print_suite_stats(suite, suite_stats, total_stats); > - kunit_print_suite_end(suite); > +suite_end: > + kunit_print_suite_end(suite, suite_init_err); > > return 0; > } > -- > 2.36.0.rc2.479.g8af0fa9b8e-goog >
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
