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

Signed-off-by: Arpitha Raghunandan 98.arpi@gmail.com

Changes v2->v3:

• Marked hardcoded test data const
• Modification based on latest implementation of KUnit parameterized testing

Changes v1->v2:

• Modification based on latest implementation of KUnit parameterized testing

fs/ext4/inode-test.c | 314 ++++++++++++++++++++++--------------------- 1 file changed, 158 insertions(+), 156 deletions(-)

diff --git a/fs/ext4/inode-test.c b/fs/ext4/inode-test.c index d62d802c9c12..3a449623b775 100644 --- a/fs/ext4/inode-test.c +++ b/fs/ext4/inode-test.c @@ -80,6 +80,139 @@ struct timestamp_expectation { bool lower_bound; };

+static const struct timestamp_expectation test_data[] = {

•   {
  

•           .test_case_name = LOWER_BOUND_NEG_NO_EXTRA_BITS_CASE,
  

•           .msb_set = true,
  

•           .lower_bound = true,
  

•           .extra_bits = 0,
  

•           .expected = {.tv_sec = -0x80000000LL, .tv_nsec = 0L},
  

•   },
  

•   {
  

•           .test_case_name = UPPER_BOUND_NEG_NO_EXTRA_BITS_CASE,
  

•           .msb_set = true,
  

•           .lower_bound = false,
  

•           .extra_bits = 0,
  

•           .expected = {.tv_sec = -1LL, .tv_nsec = 0L},
  

•   },
  

•   {
  

•           .test_case_name = LOWER_BOUND_NONNEG_NO_EXTRA_BITS_CASE,
  

•           .msb_set = false,
  

•           .lower_bound = true,
  

•           .extra_bits = 0,
  

•           .expected = {0LL, 0L},
  

•   },
  

•   {
  

•           .test_case_name = UPPER_BOUND_NONNEG_NO_EXTRA_BITS_CASE,
  

•           .msb_set = false,
  

•           .lower_bound = false,
  

•           .extra_bits = 0,
  

•           .expected = {.tv_sec = 0x7fffffffLL, .tv_nsec = 0L},
  

•   },
  

•   {
  

•           .test_case_name = LOWER_BOUND_NEG_LO_1_CASE,
  

•           .msb_set = true,
  

•           .lower_bound = true,
  

•           .extra_bits = 1,
  

•           .expected = {.tv_sec = 0x80000000LL, .tv_nsec = 0L},
  

•   },
  

•   {
  

•           .test_case_name = UPPER_BOUND_NEG_LO_1_CASE,
  

•           .msb_set = true,
  

•           .lower_bound = false,
  

•           .extra_bits = 1,
  

•           .expected = {.tv_sec = 0xffffffffLL, .tv_nsec = 0L},
  

•   },
  

•   {
  

•           .test_case_name = LOWER_BOUND_NONNEG_LO_1_CASE,
  

•           .msb_set = false,
  

•           .lower_bound = true,
  

•           .extra_bits = 1,
  

•           .expected = {.tv_sec = 0x100000000LL, .tv_nsec = 0L},
  

•   },
  

•   {
  

•           .test_case_name = UPPER_BOUND_NONNEG_LO_1_CASE,
  

•           .msb_set = false,
  

•           .lower_bound = false,
  

•           .extra_bits = 1,
  

•           .expected = {.tv_sec = 0x17fffffffLL, .tv_nsec = 0L},
  

•   },
  

•   {
  

•           .test_case_name = LOWER_BOUND_NEG_HI_1_CASE,
  

•           .msb_set = true,
  

•           .lower_bound = true,
  

•           .extra_bits =  2,
  

•           .expected = {.tv_sec = 0x180000000LL, .tv_nsec = 0L},
  

•   },
  

•   {
  

•           .test_case_name = UPPER_BOUND_NEG_HI_1_CASE,
  

•           .msb_set = true,
  

•           .lower_bound = false,
  

•           .extra_bits = 2,
  

•           .expected = {.tv_sec = 0x1ffffffffLL, .tv_nsec = 0L},
  

•   },
  

•   {
  

•           .test_case_name = LOWER_BOUND_NONNEG_HI_1_CASE,
  

•           .msb_set = false,
  

•           .lower_bound = true,
  

•           .extra_bits = 2,
  

•           .expected = {.tv_sec = 0x200000000LL, .tv_nsec = 0L},
  

•   },
  

•   {
  

•           .test_case_name = UPPER_BOUND_NONNEG_HI_1_CASE,
  

•           .msb_set = false,
  

•           .lower_bound = false,
  

•           .extra_bits = 2,
  

•           .expected = {.tv_sec = 0x27fffffffLL, .tv_nsec = 0L},
  

•   },
  

•   {
  

•           .test_case_name = UPPER_BOUND_NONNEG_HI_1_NS_1_CASE,
  

•           .msb_set = false,
  

•           .lower_bound = false,
  

•           .extra_bits = 6,
  

•           .expected = {.tv_sec = 0x27fffffffLL, .tv_nsec = 1L},
  

•   },
  

•   {
  

•           .test_case_name = LOWER_BOUND_NONNEG_HI_1_NS_MAX_CASE,
  

•           .msb_set = false,
  

•           .lower_bound = true,
  

•           .extra_bits = 0xFFFFFFFF,
  

•           .expected = {.tv_sec = 0x300000000LL,
  

•                        .tv_nsec = MAX_NANOSECONDS},
  

•   },
  

•   {
  

•           .test_case_name = LOWER_BOUND_NONNEG_EXTRA_BITS_1_CASE,
  

•           .msb_set = false,
  

•           .lower_bound = true,
  

•           .extra_bits = 3,
  

•           .expected = {.tv_sec = 0x300000000LL, .tv_nsec = 0L},
  

•   },
  

•   {
  

•           .test_case_name = UPPER_BOUND_NONNEG_EXTRA_BITS_1_CASE,
  

•           .msb_set = false,
  

•           .lower_bound = false,
  

•           .extra_bits = 3,
  

•           .expected = {.tv_sec = 0x37fffffffLL, .tv_nsec = 0L},
  

•   }
  

+};

+KUNIT_PARAM_GENERATOR(ext4_inode, test_data);

static time64_t get_32bit_time(const struct timestamp_expectation * const test) { if (test->msb_set) { @@ -101,166 +234,35 @@ 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[] = {
  

•           {
  

•                   .test_case_name = LOWER_BOUND_NEG_NO_EXTRA_BITS_CASE,
  

•                   .msb_set = true,
  

•                   .lower_bound = true,
  

•                   .extra_bits = 0,
  

•                   .expected = {.tv_sec = -0x80000000LL, .tv_nsec = 0L},
  

•           },
  

•           {
  

•                   .test_case_name = UPPER_BOUND_NEG_NO_EXTRA_BITS_CASE,
  

•                   .msb_set = true,
  

•                   .lower_bound = false,
  

•                   .extra_bits = 0,
  

•                   .expected = {.tv_sec = -1LL, .tv_nsec = 0L},
  

•           },
  

•           {
  

•                   .test_case_name = LOWER_BOUND_NONNEG_NO_EXTRA_BITS_CASE,
  

•                   .msb_set = false,
  

•                   .lower_bound = true,
  

•                   .extra_bits = 0,
  

•                   .expected = {0LL, 0L},
  

•           },
  

•           {
  

•                   .test_case_name = UPPER_BOUND_NONNEG_NO_EXTRA_BITS_CASE,
  

•                   .msb_set = false,
  

•                   .lower_bound = false,
  

•                   .extra_bits = 0,
  

•                   .expected = {.tv_sec = 0x7fffffffLL, .tv_nsec = 0L},
  

•           },
  

•           {
  

•                   .test_case_name = LOWER_BOUND_NEG_LO_1_CASE,
  

•                   .msb_set = true,
  

•                   .lower_bound = true,
  

•                   .extra_bits = 1,
  

•                   .expected = {.tv_sec = 0x80000000LL, .tv_nsec = 0L},
  

•           },
  

•           {
  

•                   .test_case_name = UPPER_BOUND_NEG_LO_1_CASE,
  

•                   .msb_set = true,
  

•                   .lower_bound = false,
  

•                   .extra_bits = 1,
  

•                   .expected = {.tv_sec = 0xffffffffLL, .tv_nsec = 0L},
  

•           },
  

•           {
  

•                   .test_case_name = LOWER_BOUND_NONNEG_LO_1_CASE,
  

•                   .msb_set = false,
  

•                   .lower_bound = true,
  

•                   .extra_bits = 1,
  

•                   .expected = {.tv_sec = 0x100000000LL, .tv_nsec = 0L},
  

•           },
  

•           {
  

•                   .test_case_name = UPPER_BOUND_NONNEG_LO_1_CASE,
  

•                   .msb_set = false,
  

•                   .lower_bound = false,
  

•                   .extra_bits = 1,
  

•                   .expected = {.tv_sec = 0x17fffffffLL, .tv_nsec = 0L},
  

•           },
  

•           {
  

•                   .test_case_name = LOWER_BOUND_NEG_HI_1_CASE,
  

•                   .msb_set = true,
  

•                   .lower_bound = true,
  

•                   .extra_bits =  2,
  

•                   .expected = {.tv_sec = 0x180000000LL, .tv_nsec = 0L},
  

•           },
  

•           {
  

•                   .test_case_name = UPPER_BOUND_NEG_HI_1_CASE,
  

•                   .msb_set = true,
  

•                   .lower_bound = false,
  

•                   .extra_bits = 2,
  

•                   .expected = {.tv_sec = 0x1ffffffffLL, .tv_nsec = 0L},
  

•           },
  

•           {
  

•                   .test_case_name = LOWER_BOUND_NONNEG_HI_1_CASE,
  

•                   .msb_set = false,
  

•                   .lower_bound = true,
  

•                   .extra_bits = 2,
  

•                   .expected = {.tv_sec = 0x200000000LL, .tv_nsec = 0L},
  

•           },
  

•           {
  

•                   .test_case_name = UPPER_BOUND_NONNEG_HI_1_CASE,
  

•                   .msb_set = false,
  

•                   .lower_bound = false,
  

•                   .extra_bits = 2,
  

•                   .expected = {.tv_sec = 0x27fffffffLL, .tv_nsec = 0L},
  

•           },
  

•           {
  

•                   .test_case_name = UPPER_BOUND_NONNEG_HI_1_NS_1_CASE,
  

•                   .msb_set = false,
  

•                   .lower_bound = false,
  

•                   .extra_bits = 6,
  

•                   .expected = {.tv_sec = 0x27fffffffLL, .tv_nsec = 1L},
  

•           },
  

•           {
  

•                   .test_case_name = LOWER_BOUND_NONNEG_HI_1_NS_MAX_CASE,
  

•                   .msb_set = false,
  

•                   .lower_bound = true,
  

•                   .extra_bits = 0xFFFFFFFF,
  

•                   .expected = {.tv_sec = 0x300000000LL,
  

•                                .tv_nsec = MAX_NANOSECONDS},
  

•           },
  

•           {
  

•                   .test_case_name = LOWER_BOUND_NONNEG_EXTRA_BITS_1_CASE,
  

•                   .msb_set = false,
  

•                   .lower_bound = true,
  

•                   .extra_bits = 3,
  

•                   .expected = {.tv_sec = 0x300000000LL, .tv_nsec = 0L},
  

•           },
  

•           {
  

•                   .test_case_name = UPPER_BOUND_NONNEG_EXTRA_BITS_1_CASE,
  

•                   .msb_set = false,
  

•                   .lower_bound = false,
  

•                   .extra_bits = 3,
  

•                   .expected = {.tv_sec = 0x37fffffffLL, .tv_nsec = 0L},
  

•           }
  

•   };
  

•   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);
  

•   }
  

•   struct timestamp_expectation *test_param =
  

•                   (struct timestamp_expectation *)(test->param_values);
  

•   timestamp.tv_sec = get_32bit_time(test_param);
  

•   ext4_decode_extra_time(&timestamp,
  

•                          cpu_to_le32(test_param->extra_bits));
  

•   KUNIT_EXPECT_EQ_MSG(test,
  

•                       test_param->expected.tv_sec,
  

•                       timestamp.tv_sec,
  

•                       CASE_NAME_FORMAT,
  

•                       test_param->test_case_name,
  

•                       test_param->msb_set,
  

•                       test_param->lower_bound,
  

•                       test_param->extra_bits);
  

•   KUNIT_EXPECT_EQ_MSG(test,
  

•                       test_param->expected.tv_nsec,
  

•                       timestamp.tv_nsec,
  

•                       CASE_NAME_FORMAT,
  

•                       test_param->test_case_name,
  

•                       test_param->msb_set,
  

•                       test_param->lower_bound,
  

•                       test_param->extra_bits);
  

}

static struct kunit_case ext4_inode_test_cases[] = {

•   KUNIT_CASE(inode_test_xtimestamp_decoding),
  

•   KUNIT_CASE_PARAM(inode_test_xtimestamp_decoding, ext4_inode_gen_params),
    {}
  

};

-- 2.25.1

Reviewed-by: Iurii Zaikin yzaikin@google.com

On 27/10/20 4:44 am, Marco Elver wrote:

On Mon, 26 Oct 2020 at 19:36, Arpitha Raghunandan 98.arpi@gmail.com wrote:

...

Implementation of support for parameterized testing in KUnit. This approach requires the creation of a test case using the KUNIT_CASE_PARAM macro that accepts a generator function as input. This generator function should return the next parameter given the previous parameter in parameterized tests. It also provides a macro to generate common-case generators.

Signed-off-by: Arpitha Raghunandan 98.arpi@gmail.com Co-developed-by: Marco Elver elver@google.com Signed-off-by: Marco Elver elver@google.com

---

Changes v2->v3:

• Modifictaion of generator macro and method

Great to see it worked as expected!

...

Changes v1->v2:

• Use of a generator method to access test case parameters

include/kunit/test.h | 32 ++++++++++++++++++++++++++++++++ lib/kunit/test.c | 20 +++++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h index a423fffefea0..16bf9f334e2c 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -142,6 +142,12 @@ struct kunit_case { void (*run_case)(struct kunit *test); const char *name;

•   /*
  

•    * Pointer to test parameter generator function.
  

•    * Used only for parameterized tests.
  

What I meant was to give a description of the protocol, so that if somebody wanted, they could (without reading the implementation) implement their own custom generator without the helper macro.

E.g. something like: "The generator function is used to lazily generate a series of arbitrarily typed values that fit into a void*. The argument @prev is the previously returned value, which should be used to derive the next value; @prev is set to NULL on the initial generator call. When no more values are available, the generator must return NULL."

Oh okay. I am not sure if this is the best place to add documentation for this.

...

•    */
  

•   void* (*generate_params)(void *prev);
  

•   /* private: internal use only. */
    bool success;
    char *log;
  

@@ -162,6 +168,9 @@ static inline char *kunit_status_to_string(bool status)

• &struct kunit_case for an example on how to use it.

*/ #define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name } +#define KUNIT_CASE_PARAM(test_name, gen_params) \

•           { .run_case = test_name, .name = #test_name,    \
  

•             .generate_params = gen_params }
  
  

/**

• struct kunit_suite - describes a related collection of &struct kunit_case

@@ -208,6 +217,15 @@ struct kunit { const char *name; /* Read only after initialization! */ char *log; /* Points at case log after initialization */ struct kunit_try_catch try_catch;

•   /* param_values points to test case parameters in parameterized tests */
  

•   void *param_values;
  

•   /*
  

•    * current_param stores the index of the parameter in
  

•    * the array of parameters in parameterized tests.
  

•    * current_param + 1 is printed to indicate the parameter
  

•    * that causes the test to fail in case of test failure.
  

•    */
  

•   int current_param;
    /*
     * success starts as true, and may only be set to false during a
     * test case; thus, it is safe to update this across multiple
  

@@ -1742,4 +1760,18 @@ do { \ fmt, \ ##__VA_ARGS__)

+/**

• • KUNIT_PARAM_GENERATOR() - Helper method for test parameter generators

• •                      required in parameterized tests.
    

This is only for arrays, which is why I suggested KUNIT_ARRAY_PARAM() as the name.

A generator can very well be implemented without an array, so this macro name is confusing. In future somebody might want to provide a macro that takes a start + end value (and maybe a step value) to generate a series of values. That generator could be named KUNIT_RANGE_PARAM(name, start, end, step) and gives us a generator that is also named name##_gen_params. (If you want to try implementing that macro, I'd suggest doing it as a separate patch.)

And I don't think we need to put "GENERATOR" into the name of these macros, because the generators are now the fundamental method with which to get parameterized tests. We don't need to state the obvious, in favor of some brevity.

Okay, makes sense. I will change it to KUNIT_ARRAY_PARAM() for the next version.

...

• • @name: prefix of the name for the test parameter generator function.
• • @prev: a pointer to the previous test parameter, NULL for first parameter.
• • @array: a user-supplied pointer to an array of test parameters.
• */

+#define KUNIT_PARAM_GENERATOR(name, array) \

•   static void *name##_gen_params(void *prev)                                              \
  

•   {                                                                                       \
  

•           typeof((array)[0]) * __next = prev ? ((typeof(__next)) prev) + 1 : (array);     \
  

•           return __next - (array) < ARRAY_SIZE((array)) ? __next : NULL;                  \
  

•   }
  

#endif /* _KUNIT_TEST_H */

Thanks, -- Marco

Thanks!

On 27/10/20 1:14 pm, Marco Elver wrote:

On Tue, 27 Oct 2020 at 06:14, Arpitha Raghunandan 98.arpi@gmail.com wrote: [...]

...

...

...

diff --git a/include/kunit/test.h b/include/kunit/test.h index a423fffefea0..16bf9f334e2c 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -142,6 +142,12 @@ struct kunit_case { void (*run_case)(struct kunit *test); const char *name;

•   /*
  

•    * Pointer to test parameter generator function.
  

•    * Used only for parameterized tests.
  

What I meant was to give a description of the protocol, so that if somebody wanted, they could (without reading the implementation) implement their own custom generator without the helper macro.

E.g. something like: "The generator function is used to lazily generate a series of arbitrarily typed values that fit into a void*. The argument @prev is the previously returned value, which should be used to derive the next value; @prev is set to NULL on the initial generator call. When no more values are available, the generator must return NULL."

Oh okay. I am not sure if this is the best place to add documentation for this.

I think it doesn't hurt to add, but have a look at the comment above this struct, which is already a kernel-doc comment. It probably makes sense to move the comment there to describe the new variable.

Alright, I will move the comment there.

Thanks, -- Marco

Thanks.