kunit: Support for Parameterized Testing

Modifies fs/ext4/inode-test.c to use the parameterized testing feature of KUnit.

Signed-off-by Arpitha Raghunandan <[email protected]>

---
 fs/ext4/inode-test.c | 69 +++++++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 27 deletions(-)

fs/ext4/inode-test.c +2 -0

index d62d802c9c12..e262fef505b3 100644
@@ -72,6 +72,8 @@
 #define UPPER_BOUND_NONNEG_EXTRA_BITS_1_CASE\
        "2446-05-10 Upper bound of 32bit >=0 timestamp. All extra sec bits on"
 
+#define NUMBER_OF_TESTCASES 16
+
 struct timestamp_expectation {
        const char *test_case_name;
        struct timespec64 expected;

fs/ext4/inode-test.c +18 -0

+               (struct timestamp_expectation *)get_test_case_parameters(test);
+
+       timestamp.tv_sec = get_32bit_time(test_case);
+       ext4_decode_extra_time(&timestamp,
+                              cpu_to_le32(test_case->extra_bits));
+
+       KUNIT_EXPECT_EQ_MSG(test,
+                           test_case->expected.tv_sec,
+                           timestamp.tv_sec,
+                           CASE_NAME_FORMAT,
+                           test_case->test_case_name,
+                           test_case->msb_set,
+                           test_case->lower_bound,
+                           test_case->extra_bits);
+       KUNIT_EXPECT_EQ_MSG(test,
+                           test_case->expected.tv_nsec,
+                           timestamp.tv_nsec,
+                           CASE_NAME_FORMAT,

fs/ext4/inode-test.c +7 -27

                {
                        .test_case_name = LOWER_BOUND_NEG_NO_EXTRA_BITS_CASE,
                        .msb_set = true,

@@ -232,35 +266,16 @@ static void inode_test_xtimestamp_decoding(struct kunit *test)
                }
        };
 
-       struct timespec64 timestamp;
-       int i;
-
-       for (i = 0; i < ARRAY_SIZE(test_data); ++i) {
-               timestamp.tv_sec = get_32bit_time(&test_data[i]);
-               ext4_decode_extra_time(&timestamp,
-                                      cpu_to_le32(test_data[i].extra_bits));
-
-               KUNIT_EXPECT_EQ_MSG(test,
-                                   test_data[i].expected.tv_sec,
-                                   timestamp.tv_sec,
-                                   CASE_NAME_FORMAT,
-                                   test_data[i].test_case_name,
-                                   test_data[i].msb_set,
-                                   test_data[i].lower_bound,
-                                   test_data[i].extra_bits);
-               KUNIT_EXPECT_EQ_MSG(test,
-                                   test_data[i].expected.tv_nsec,
-                                   timestamp.tv_nsec,
-                                   CASE_NAME_FORMAT,
-                                   test_data[i].test_case_name,
-                                   test_data[i].msb_set,
-                                   test_data[i].lower_bound,
-                                   test_data[i].extra_bits);
-       }
+       memcpy(test_data, test_data_init,
+               sizeof(struct timestamp_expectation) * ARRAY_SIZE(test_data_init));
+
+       return test_data;
 }
 
 static struct kunit_case ext4_inode_test_cases[] = {
-       KUNIT_CASE(inode_test_xtimestamp_decoding),
+       KUNIT_CASE_PARAM(inode_test_xtimestamp_decoding,
+                       get_test_parameters, NUMBER_OF_TESTCASES,
+                       sizeof(struct timestamp_expectation)),
        {}
 };
 
--

2.25.1

Arpitha, please add Iurii on future revisions. He authored this test. Okay!

Quoted 23 lines · Arpitha Raghunandan

Modifies fs/ext4/inode-test.c to use the parameterized testing feature of KUnit.

Signed-off-by Arpitha Raghunandan <[email protected]>

---
 fs/ext4/inode-test.c | 69 +++++++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 27 deletions(-)

fs/ext4/inode-test.c +2 -0

index d62d802c9c12..e262fef505b3 100644
@@ -72,6 +72,8 @@
 #define UPPER_BOUND_NONNEG_EXTRA_BITS_1_CASE\
        "2446-05-10 Upper bound of 32bit >=0 timestamp. All extra sec bits on"
 
+#define NUMBER_OF_TESTCASES 16
+
 struct timestamp_expectation {
        const char *test_case_name;
        struct timespec64 expected;

fs/ext4/inode-test.c static time64_t get_32bit_time(const struct timestamp_expectation * const test) · line 108

Quoted 7 lines

fs/ext4/inode-test.c +3 -1

@@ -101,7 +103,39 @@ static time64_t get_32bit_time(const struct timestamp_expectation * const test)
  */
 static void inode_test_xtimestamp_decoding(struct kunit *test)
 {
-       const struct timestamp_expectation test_data[] = {
+       struct timespec64 timestamp;
+
+       struct timestamp_expectation *test_case =

I think test_data or test_parameters makes more sense.

Quoted 18 lines

fs/ext4/inode-test.c +18 -0

+               (struct timestamp_expectation *)get_test_case_parameters(test);
+
+       timestamp.tv_sec = get_32bit_time(test_case);
+       ext4_decode_extra_time(&timestamp,
+                              cpu_to_le32(test_case->extra_bits));
+
+       KUNIT_EXPECT_EQ_MSG(test,
+                           test_case->expected.tv_sec,
+                           timestamp.tv_sec,
+                           CASE_NAME_FORMAT,
+                           test_case->test_case_name,
+                           test_case->msb_set,
+                           test_case->lower_bound,
+                           test_case->extra_bits);
+       KUNIT_EXPECT_EQ_MSG(test,
+                           test_case->expected.tv_nsec,
+                           timestamp.tv_nsec,
+                           CASE_NAME_FORMAT,

fs/ext4/inode-test.c static time64_t get_32bit_time(const struct timestamp_expectation * const test) · line 136

Quoted 10 lines

fs/ext4/inode-test.c +10 -0

+                           test_case->test_case_name,
+                           test_case->msb_set,
+                           test_case->lower_bound,
+                           test_case->extra_bits);
+}
+
+struct timestamp_expectation *get_test_parameters(void)
+{
+       struct timestamp_expectation *test_data = (struct timestamp_expectation *)
+               kmalloc(sizeof(struct timestamp_expectation) * NUMBER_OF_TESTCASES, GFP_KERNEL);

I don't see this get freed anywhere; you can get around it with kunit_kmalloc. However, I suspect you won't need this at all given my next comment...

fs/ext4/inode-test.c static time64_t get_32bit_time(const struct timestamp_expectation * const test) · line 138

Quoted 2 lines

fs/ext4/inode-test.c +2 -0

+
+       const struct timestamp_expectation test_data_init[] = {

Can't you just make the scope of this array global or static and then just return a pointer to an element in the array?

Quoted 46 lines

fs/ext4/inode-test.c +7 -26

                {
                        .test_case_name = LOWER_BOUND_NEG_NO_EXTRA_BITS_CASE,
                        .msb_set = true,

@@ -232,35 +266,16 @@ static void inode_test_xtimestamp_decoding(struct kunit *test)
                }
        };
 
-       struct timespec64 timestamp;
-       int i;
-
-       for (i = 0; i < ARRAY_SIZE(test_data); ++i) {
-               timestamp.tv_sec = get_32bit_time(&test_data[i]);
-               ext4_decode_extra_time(&timestamp,
-                                      cpu_to_le32(test_data[i].extra_bits));
-
-               KUNIT_EXPECT_EQ_MSG(test,
-                                   test_data[i].expected.tv_sec,
-                                   timestamp.tv_sec,
-                                   CASE_NAME_FORMAT,
-                                   test_data[i].test_case_name,
-                                   test_data[i].msb_set,
-                                   test_data[i].lower_bound,
-                                   test_data[i].extra_bits);
-               KUNIT_EXPECT_EQ_MSG(test,
-                                   test_data[i].expected.tv_nsec,
-                                   timestamp.tv_nsec,
-                                   CASE_NAME_FORMAT,
-                                   test_data[i].test_case_name,
-                                   test_data[i].msb_set,
-                                   test_data[i].lower_bound,
-                                   test_data[i].extra_bits);
-       }
+       memcpy(test_data, test_data_init,
+               sizeof(struct timestamp_expectation) * ARRAY_SIZE(test_data_init));
+
+       return test_data;
 }
 
 static struct kunit_case ext4_inode_test_cases[] = {
-       KUNIT_CASE(inode_test_xtimestamp_decoding),
+       KUNIT_CASE_PARAM(inode_test_xtimestamp_decoding,
+                       get_test_parameters, NUMBER_OF_TESTCASES,
+                       sizeof(struct timestamp_expectation)),
        {}
 };
 

Quoted 3 lines

--
2.25.1

Quoted 1 line

Quoted 1 line

+#define NUMBER_OF_TESTCASES 16

Quoted 1 line

Quoted 1 line

+struct timestamp_expectation *get_test_parameters(void)

Implementation of support for parameterized testing in KUnit.

Signed-off-by Arpitha Raghunandan <[email protected]>

---
Changes v2->v3:
- Support for test to run iterating through all parameters added within the KUnit framework
- The index of the parameter causing test failure is displayed

Changes v1->v2:
- Parameterized input stored in a void* array
- An iterator method to access the different parameters

 include/kunit/test.h | 29 +++++++++++++++++++++++++++++
 lib/kunit/test.c     | 44 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 1 deletion(-)

include/kunit/test.h +29 -0

index 59f3144f009a..8e9325b29058 100644
@@ -140,10 +140,14 @@ struct kunit;
 struct kunit_case {
        void (*run_case)(struct kunit *test);
        const char *name;
+       void* (*get_params)(void);
+       int max_parameters_count;
+       int parameter_size;
 
        /* private: internal use only. */
        bool success;
        char *log;
+       bool parameterized;
 };
 
 static inline char *kunit_status_to_string(bool status)

@@ -162,6 +166,11 @@ static inline char *kunit_status_to_string(bool status)
  */
 #define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name }
 
+#define KUNIT_CASE_PARAM(test_name, getparams, count, size) {  .run_case = test_name,                                          \
+                                                               .name = #test_name, .parameterized = true,                      \
+                                                               .get_params = (void* (*)(void))getparams,                       \
+                                                               .max_parameters_count = count, .parameter_size = size }
+
 /**
  * struct kunit_suite - describes a related collection of &struct kunit_case
  *

@@ -206,6 +215,23 @@ struct kunit {
        /* private: internal use only. */
        const char *name; /* Read only after initialization! */
        char *log; /* Points at case log after initialization */
+       bool parameterized; /* True for parameterized tests */
+       /* param_values stores the test parameters
+        * for parameterized tests.
+        */
+       void *param_values;
+       /* max_parameters_count indicates maximum number of
+        * parameters for parameterized tests.
+        */
+       int max_parameters_count;
+       /* iterator_count is used by the iterator method
+        * for parameterized tests.
+        */
+       int iterator_count;
+       /* parameter_size indicates size of a single test case
+        * for parameterized tests.
+        */
+       int parameter_size;
        struct kunit_try_catch try_catch;
        /*
         * success starts as true, and may only be set to false during a

@@ -225,6 +251,7 @@ struct kunit {
 };
 
 void kunit_init_test(struct kunit *test, const char *name, char *log);
+void kunit_init_param_test(struct kunit *test, struct kunit_case *test_case);
 
 int kunit_run_tests(struct kunit_suite *suite);
 

@@ -237,6 +264,8 @@ int __kunit_test_suites_init(struct kunit_suite **suites);
 
 void __kunit_test_suites_exit(struct kunit_suite **suites);
 
+void *get_test_case_parameters(struct kunit *test);
+
 /**
  * kunit_test_suites() - used to register one or more &struct kunit_suite
  *                      with KUnit.

lib/kunit/test.c +43 -2

index c36037200310..ae012e65368e 100644
@@ -142,6 +142,11 @@ unsigned int kunit_test_case_num(struct kunit_suite *suite,
 }
 EXPORT_SYMBOL_GPL(kunit_test_case_num);
 
+static void kunit_print_failed_param(struct kunit *test)
+{
+       kunit_err(test, "\n\tTest failed at parameter: %d\n", test->iterator_count);
+}
+
 static void kunit_print_string_stream(struct kunit *test,
                                      struct string_stream *stream)
 {

@@ -182,6 +187,9 @@ static void kunit_fail(struct kunit *test, struct kunit_assert *assert)
 
        assert->format(assert, stream);
 
+       if (test->parameterized)
+               kunit_print_failed_param(test);
+
        kunit_print_string_stream(test, stream);
 
        WARN_ON(string_stream_destroy(stream));

@@ -236,6 +244,18 @@ void kunit_init_test(struct kunit *test, const char *name, char *log)
 }
 EXPORT_SYMBOL_GPL(kunit_init_test);
 
+void kunit_init_param_test(struct kunit *test, struct kunit_case *test_case)
+{
+       spin_lock_init(&test->lock);
+       INIT_LIST_HEAD(&test->resources);
+       test->parameterized = true;
+       test->param_values = (void *)(test_case->get_params());
+       test->max_parameters_count = test_case->max_parameters_count;
+       test->parameter_size = test_case->parameter_size;
+       test->iterator_count = 0;
+}
+EXPORT_SYMBOL_GPL(kunit_init_param_test);
+
 /*
  * Initializes and runs test case. Does not clean up or do post validations.
  */

@@ -254,7 +274,14 @@ static void kunit_run_case_internal(struct kunit *test,
                }
        }
 
-       test_case->run_case(test);
+       if (!test->parameterized)
+               test_case->run_case(test);
+       else {
+               int i;
+
+               for (i = 0; i < test->max_parameters_count; i++)
+                       test_case->run_case(test);
+       }
 }
 
 static void kunit_case_internal_cleanup(struct kunit *test)

@@ -343,6 +370,8 @@ static void kunit_run_case_catch_errors(struct kunit_suite *suite,
        struct kunit test;
 
        kunit_init_test(&test, test_case->name, test_case->log);
+       if (test_case->parameterized)
+               kunit_init_param_test(&test, test_case);
        try_catch = &test.try_catch;
 
        kunit_try_catch_init(try_catch,

@@ -407,6 +436,19 @@ void __kunit_test_suites_exit(struct kunit_suite **suites)
 }
 EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
 
+/*
+ * Iterator method for the parameterized test cases
+ */
+void *get_test_case_parameters(struct kunit *test)
+{
+       int index = test->iterator_count * test->parameter_size;
+
+       if (test->iterator_count != test->max_parameters_count)
+               test->iterator_count++;
+       return (test->param_values + index);
+}
+EXPORT_SYMBOL_GPL(get_test_case_parameters);
+
 /*
  * Used for static resources and when a kunit_resource * has been created by
  * kunit_alloc_resource().  When an init function is supplied, @data is passed
--

2.25.1

On Sat, Oct 3, 2020 at 7:50 AM Arpitha Raghunandan <[email protected]> wrote:
[...]

Quoted 4 lines

(quoted diff) +0 -0

@@ -254,7 +274,14 @@ static void kunit_run_case_internal(struct kunit *test,
                }
        }
 

lib/kunit/test.c static void kunit_run_case_internal(struct kunit *test, · line 279

Quoted 4 lines

lib/kunit/test.c +3 -1

-       test_case->run_case(test);
+       if (!test->parameterized)
+               test_case->run_case(test);
+       else {

Sorry, I just caught this. When you put either the if or else block in braces; the other should go in braces as well.

Quoted 7 lines

lib/kunit/test.c +5 -0

+               int i;
+
+               for (i = 0; i < test->max_parameters_count; i++)
+                       test_case->run_case(test);
+       }
 }
 

[...]