kunit: Support skipped tests
authorDavid Gow <davidgow@google.com>
Fri, 25 Jun 2021 06:58:12 +0000 (23:58 -0700)
committerShuah Khan <skhan@linuxfoundation.org>
Fri, 25 Jun 2021 17:31:03 +0000 (11:31 -0600)
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@google.com>
Tested-by: Marco Elver <elver@google.com>
Reviewed-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
include/kunit/test.h
lib/kunit/debugfs.c
lib/kunit/kunit-test.c
lib/kunit/test.c

index e79ac81f5fcab64d7b252f1c3c80e246d534f0d8..35b0aed9b739caa40bff0a9f82398814fbfe45c9 100644 (file)
@@ -97,6 +97,9 @@ struct kunit;
 /* Maximum size of parameter description string. */
 #define KUNIT_PARAM_DESC_SIZE 128
 
+/* Maximum size of a status comment. */
+#define KUNIT_STATUS_COMMENT_SIZE 256
+
 /*
  * TAP specifies subtest stream indentation of 4 spaces, 8 spaces for a
  * sub-subtest.  See the "Subtests" section in
@@ -105,6 +108,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 +163,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 +234,7 @@ struct kunit_suite {
        struct kunit_case *test_cases;
 
        /* private: internal use only */
+       char status_comment[KUNIT_STATUS_COMMENT_SIZE];
        struct dentry *debugfs;
        char *log;
 };
@@ -245,19 +268,21 @@ struct kunit {
         * be read after the test case finishes once all threads associated
         * with the test case have terminated.
         */
-       bool success; /* Read only after test_case finishes! */
        spinlock_t lock; /* Guards all mutable test state. */
+       enum kunit_status status; /* Read only after test_case finishes! */
        /*
         * Because resources is a list that may be updated multiple times (with
         * new resources) from any thread associated with a test case, we must
         * protect it with some type of lock.
         */
        struct list_head resources; /* Protected by lock. */
+
+       char status_comment[KUNIT_STATUS_COMMENT_SIZE];
 };
 
 static inline void kunit_set_failure(struct kunit *test)
 {
-       WRITE_ONCE(test->success, false);
+       WRITE_ONCE(test->status, KUNIT_FAILURE);
 }
 
 void kunit_init_test(struct kunit *test, const char *name, char *log);
@@ -348,7 +373,7 @@ static inline int kunit_run_all_tests(void)
 #define kunit_suite_for_each_test_case(suite, test_case)               \
        for (test_case = suite->test_cases; test_case->run_case; test_case++)
 
-bool kunit_suite_has_succeeded(struct kunit_suite *suite);
+enum kunit_status kunit_suite_has_succeeded(struct kunit_suite *suite);
 
 /*
  * Like kunit_alloc_resource() below, but returns the struct kunit_resource
@@ -640,6 +665,42 @@ void kunit_cleanup(struct kunit *test);
 
 void __printf(2, 3) kunit_log_append(char *log, const char *fmt, ...);
 
+/**
+ * kunit_mark_skipped() - Marks @test_or_suite as skipped
+ *
+ * @test_or_suite: The test context object.
+ * @fmt:  A printk() style format string.
+ *
+ * Marks the test as skipped. @fmt is given output as the test status
+ * comment, typically the reason the test was skipped.
+ *
+ * Test execution continues after kunit_mark_skipped() is called.
+ */
+#define kunit_mark_skipped(test_or_suite, fmt, ...)                    \
+       do {                                                            \
+               WRITE_ONCE((test_or_suite)->status, KUNIT_SKIPPED);     \
+               scnprintf((test_or_suite)->status_comment,              \
+                         KUNIT_STATUS_COMMENT_SIZE,                    \
+                         fmt, ##__VA_ARGS__);                          \
+       } while (0)
+
+/**
+ * kunit_skip() - Marks @test_or_suite as skipped
+ *
+ * @test_or_suite: The test context object.
+ * @fmt:  A printk() style format string.
+ *
+ * Skips the test. @fmt is given output as the test status
+ * comment, typically the reason the test was skipped.
+ *
+ * Test execution is halted after kunit_skip() is called.
+ */
+#define kunit_skip(test_or_suite, fmt, ...)                            \
+       do {                                                            \
+               kunit_mark_skipped((test_or_suite), fmt, ##__VA_ARGS__);\
+               kunit_try_catch_throw(&((test_or_suite)->try_catch));   \
+       } while (0)
+
 /*
  * printk and log to per-test or per-suite log buffer.  Logging only done
  * if CONFIG_KUNIT_DEBUGFS is 'y'; if it is 'n', no log is allocated/used.
index 9214c493d8b76135af6511d04530d5189b6b58c0..b71db0abc12bf1babc9eab18f19f60fe8b4ba827 100644 (file)
@@ -64,7 +64,7 @@ static int debugfs_print_results(struct seq_file *seq, void *v)
                debugfs_print_result(seq, suite, test_case);
 
        seq_printf(seq, "%s %d - %s\n",
-                  kunit_status_to_string(success), 1, suite->name);
+                  kunit_status_to_ok_not_ok(success), 1, suite->name);
        return 0;
 }
 
index 69f902440a0e95fe2b29c88d4011e030cab7dbac..d69efcbed624e336dd47ebde6beb7fcb8d69ed48 100644 (file)
@@ -437,7 +437,47 @@ static void kunit_log_test(struct kunit *test)
 #endif
 }
 
+static void kunit_status_set_failure_test(struct kunit *test)
+{
+       struct kunit fake;
+
+       kunit_init_test(&fake, "fake test", NULL);
+
+       KUNIT_EXPECT_EQ(test, fake.status, (enum kunit_status)KUNIT_SUCCESS);
+       kunit_set_failure(&fake);
+       KUNIT_EXPECT_EQ(test, fake.status, (enum kunit_status)KUNIT_FAILURE);
+}
+
+static void kunit_status_mark_skipped_test(struct kunit *test)
+{
+       struct kunit fake;
+
+       kunit_init_test(&fake, "fake test", NULL);
+
+       /* Before: Should be SUCCESS with no comment. */
+       KUNIT_EXPECT_EQ(test, fake.status, KUNIT_SUCCESS);
+       KUNIT_EXPECT_STREQ(test, fake.status_comment, "");
+
+       /* Mark the test as skipped. */
+       kunit_mark_skipped(&fake, "Accepts format string: %s", "YES");
+
+       /* After: Should be SKIPPED with our comment. */
+       KUNIT_EXPECT_EQ(test, fake.status, (enum kunit_status)KUNIT_SKIPPED);
+       KUNIT_EXPECT_STREQ(test, fake.status_comment, "Accepts format string: YES");
+}
+
+static struct kunit_case kunit_status_test_cases[] = {
+       KUNIT_CASE(kunit_status_set_failure_test),
+       KUNIT_CASE(kunit_status_mark_skipped_test),
+       {}
+};
+
+static struct kunit_suite kunit_status_test_suite = {
+       .name = "kunit_status",
+       .test_cases = kunit_status_test_cases,
+};
+
 kunit_test_suites(&kunit_try_catch_test_suite, &kunit_resource_test_suite,
-                 &kunit_log_test_suite);
+                 &kunit_log_test_suite, &kunit_status_test_suite);
 
 MODULE_LICENSE("GPL v2");
index 06f6cff2c0e74d010859b10627ee5d893bd92e3f..b3d0c8e4e339e066806b70ab3e1153445021c6e1 100644 (file)
@@ -98,12 +98,14 @@ static void kunit_print_subtest_start(struct kunit_suite *suite)
 
 static void kunit_print_ok_not_ok(void *test_or_suite,
                                  bool is_test,
-                                 bool is_ok,
+                                 enum kunit_status status,
                                  size_t test_number,
-                                 const char *description)
+                                 const char *description,
+                                 const char *directive)
 {
        struct kunit_suite *suite = is_test ? NULL : test_or_suite;
        struct kunit *test = is_test ? test_or_suite : NULL;
+       const char *directive_header = (status == KUNIT_SKIPPED) ? " # SKIP " : "";
 
        /*
         * We do not log the test suite results as doing so would
@@ -114,25 +116,31 @@ static void kunit_print_ok_not_ok(void *test_or_suite,
         * representation.
         */
        if (suite)
-               pr_info("%s %zd - %s\n",
-                       kunit_status_to_string(is_ok),
-                       test_number, description);
+               pr_info("%s %zd - %s%s%s\n",
+                       kunit_status_to_ok_not_ok(status),
+                       test_number, description, directive_header,
+                       (status == KUNIT_SKIPPED) ? directive : "");
        else
-               kunit_log(KERN_INFO, test, KUNIT_SUBTEST_INDENT "%s %zd - %s",
-                         kunit_status_to_string(is_ok),
-                         test_number, description);
+               kunit_log(KERN_INFO, test,
+                         KUNIT_SUBTEST_INDENT "%s %zd - %s%s%s",
+                         kunit_status_to_ok_not_ok(status),
+                         test_number, description, directive_header,
+                         (status == KUNIT_SKIPPED) ? directive : "");
 }
 
-bool kunit_suite_has_succeeded(struct kunit_suite *suite)
+enum kunit_status kunit_suite_has_succeeded(struct kunit_suite *suite)
 {
        const struct kunit_case *test_case;
+       enum kunit_status status = KUNIT_SKIPPED;
 
        kunit_suite_for_each_test_case(suite, test_case) {
-               if (!test_case->success)
-                       return false;
+               if (test_case->status == KUNIT_FAILURE)
+                       return KUNIT_FAILURE;
+               else if (test_case->status == KUNIT_SUCCESS)
+                       status = KUNIT_SUCCESS;
        }
 
-       return true;
+       return status;
 }
 EXPORT_SYMBOL_GPL(kunit_suite_has_succeeded);
 
@@ -143,7 +151,8 @@ static void kunit_print_subtest_end(struct kunit_suite *suite)
        kunit_print_ok_not_ok((void *)suite, false,
                              kunit_suite_has_succeeded(suite),
                              kunit_suite_counter++,
-                             suite->name);
+                             suite->name,
+                             suite->status_comment);
 }
 
 unsigned int kunit_test_case_num(struct kunit_suite *suite,
@@ -252,7 +261,8 @@ void kunit_init_test(struct kunit *test, const char *name, char *log)
        test->log = log;
        if (test->log)
                test->log[0] = '\0';
-       test->success = true;
+       test->status = KUNIT_SUCCESS;
+       test->status_comment[0] = '\0';
 }
 EXPORT_SYMBOL_GPL(kunit_init_test);
 
@@ -376,7 +386,11 @@ static void kunit_run_case_catch_errors(struct kunit_suite *suite,
        context.test_case = test_case;
        kunit_try_catch_run(try_catch, &context);
 
-       test_case->success &= test->success;
+       /* Propagate the parameter result to the test case. */
+       if (test->status == KUNIT_FAILURE)
+               test_case->status = KUNIT_FAILURE;
+       else if (test_case->status != KUNIT_FAILURE && test->status == KUNIT_SUCCESS)
+               test_case->status = KUNIT_SUCCESS;
 }
 
 int kunit_run_tests(struct kunit_suite *suite)
@@ -388,7 +402,7 @@ int kunit_run_tests(struct kunit_suite *suite)
 
        kunit_suite_for_each_test_case(suite, test_case) {
                struct kunit test = { .param_value = NULL, .param_index = 0 };
-               test_case->success = true;
+               test_case->status = KUNIT_SKIPPED;
 
                if (test_case->generate_params) {
                        /* Get initial param. */
@@ -409,7 +423,7 @@ int kunit_run_tests(struct kunit_suite *suite)
                                          KUNIT_SUBTEST_INDENT
                                          "# %s: %s %d - %s",
                                          test_case->name,
-                                         kunit_status_to_string(test.success),
+                                         kunit_status_to_ok_not_ok(test.status),
                                          test.param_index + 1, param_desc);
 
                                /* Get next param. */
@@ -419,9 +433,10 @@ int kunit_run_tests(struct kunit_suite *suite)
                        }
                } while (test.param_value);
 
-               kunit_print_ok_not_ok(&test, true, test_case->success,
+               kunit_print_ok_not_ok(&test, true, test_case->status,
                                      kunit_test_case_num(suite, test_case),
-                                     test_case->name);
+                                     test_case->name,
+                                     test.status_comment);
        }
 
        kunit_print_subtest_end(suite);
@@ -433,6 +448,7 @@ EXPORT_SYMBOL_GPL(kunit_run_tests);
 static void kunit_init_suite(struct kunit_suite *suite)
 {
        kunit_debugfs_create_suite(suite);
+       suite->status_comment[0] = '\0';
 }
 
 int __kunit_test_suites_init(struct kunit_suite * const * const suites)