On Fri, May 28, 2021 at 12:59 AM David Gow <davidgow@xxxxxxxxxx> wrote: > > The kunit_mark_skipped() macro marks the current test as "skipped", with > the provided reason. The kunit_skip() macro will mark the test as > skipped, and abort the test. > > The TAP specification supports this "SKIP directive" as a comment after > the "ok" / "not ok" for a test. See the "Directives" section of the TAP > spec for details: > https://testanything.org/tap-specification.html#directives > > The 'success' field for KUnit tests is replaced with a kunit_status > enum, which can be SUCCESS, FAILURE, or SKIPPED, combined with a > 'status_comment' containing information on why a test was skipped. > > A new 'kunit_status' test suite is added to test this. > > Signed-off-by: David Gow <davidgow@xxxxxxxxxx> > Tested-by: Marco Elver <elver@xxxxxxxxxx> > Reviewed-by: Daniel Latypov <dlatypov@xxxxxxxxxx> One fairly minor nit below. Other than that, looks great! Reviewed-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx> > --- [...] > diff --git a/include/kunit/test.h b/include/kunit/test.h > index b68c61348121..1401c620ac5e 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -105,6 +105,18 @@ struct kunit; > #define KUNIT_SUBTEST_INDENT " " > #define KUNIT_SUBSUBTEST_INDENT " " > > +/** > + * enum kunit_status - Type of result for a test or test suite > + * @KUNIT_SUCCESS: Denotes the test suite has not failed nor been skipped > + * @KUNIT_FAILURE: Denotes the test has failed. > + * @KUNIT_SKIPPED: Denotes the test has been skipped. > + */ > +enum kunit_status { > + KUNIT_SUCCESS, > + KUNIT_FAILURE, > + KUNIT_SKIPPED, > +}; > + > /** > * struct kunit_case - represents an individual test case. > * > @@ -148,13 +160,20 @@ struct kunit_case { > const void* (*generate_params)(const void *prev, char *desc); > > /* private: internal use only. */ > - bool success; > + enum kunit_status status; > char *log; > }; > > -static inline char *kunit_status_to_string(bool status) > +static inline char *kunit_status_to_ok_not_ok(enum kunit_status status) > { > - return status ? "ok" : "not ok"; > + switch (status) { > + case KUNIT_SKIPPED: > + case KUNIT_SUCCESS: > + return "ok"; > + case KUNIT_FAILURE: > + return "not ok"; > + } > + return "invalid"; > } > > /** > @@ -212,6 +231,7 @@ struct kunit_suite { > struct kunit_case *test_cases; > > /* private: internal use only */ > + char status_comment[256]; nit: How about we make the 256 a constant since you use it in a number of places? If not, at least when you reference the struct, you might want to use ARRAY_SIZE(...). > struct dentry *debugfs; > char *log; > }; [...]
