Skip to contentSkip to conversation

kunit: Support for Parameterized Testing

RFC Superseded Arpitha Raghunandan 2 participants 2 messages Sep 20, 2020 – Sep 24, 2020

Subsystems (2)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

RFC 1/1 kunit: Support for Parameterized Testing

This patch has a basic implementation for adding support for parameterized testing in KUnit. It is not complete. Currently there is only support for integer parameters for test cases. I will generalize this to support other data types as well. I have tested this by making some small changes to the ext4/inode-test.c test. The main logic of this test hasn't been changed. However, I have created an integer array of inputs. The test only shows that these values can be accessed through the kunit struct as shown by the dmesg output:

# inode_test_xtimestamp_decoding:
Parameterized = 1
# inode_test_xtimestamp_decoding:
a[0][0] = 1 and a[0][1] = 2 and a[1][0] = 3 and a[1][1] = 4

While this does not affect the test, it shows that this initial approach for parameterized testing works.
Please let me know any suggestions for this approach and how it can be improved.

---
 fs/ext4/inode-test.c   | 22 +++++++++++++++++++++-
 include/kunit/test.h   | 11 ++++++++++-
 lib/kunit/kunit-test.c |  4 ++--
 lib/kunit/test.c       | 15 +++++++++++++--
 4 files changed, 46 insertions(+), 6 deletions(-)
fs/ext4/inode-test.c +21 -1
index d62d802c9c12..5ef242f2fef3 100644
@@ -235,6 +235,11 @@ static void inode_test_xtimestamp_decoding(struct kunit *test)
 	struct timespec64 timestamp;
 	int i;
 
+	kunit_info(test, "\nParameterized = %d\n", test->parameterized);
+	kunit_info(test, "\na[0][0] = %d and a[0][1] = %d and a[1][0] = %d and a[1][1] = %d\n",
+						test->param_values[0][0], test->param_values[0][1],
+						test->param_values[1][0], test->param_values[1][1]);
+
 	for (i = 0; i < ARRAY_SIZE(test_data); ++i) {
 		timestamp.tv_sec = get_32bit_time(&test_data[i]);
 		ext4_decode_extra_time(&timestamp,
@@ -259,8 +264,23 @@ static void inode_test_xtimestamp_decoding(struct kunit *test)
 	}
 }
 
+int **getParams(void)
+{
+	int *x = (int *)kmalloc(sizeof(int) * 2, GFP_KERNEL);
+	int *y = (int *)kmalloc(sizeof(int) * 2, GFP_KERNEL);
+	int **a = (int **)kmalloc(sizeof(x) * 2, GFP_KERNEL);
+
+	x[0] = 1;
+	x[1] = 2;
+	y[0] = 3;
+	y[1] = 4;
+	a[0] = x;
+	a[1] = y;
+	return a;
+}
+
 static struct kunit_case ext4_inode_test_cases[] = {
-	KUNIT_CASE(inode_test_xtimestamp_decoding),
+	KUNIT_CASE_PARAM(inode_test_xtimestamp_decoding, getParams),
 	{}
 };
 
include/kunit/test.h +10 -1
index 59f3144f009a..23e4ff68c4b5 100644
@@ -140,6 +140,8 @@ struct kunit;
 struct kunit_case {
 	void (*run_case)(struct kunit *test);
 	const char *name;
+	bool parameterized;
+	int** (*get_params)(void);
 
 	/* private: internal use only. */
 	bool success;
@@ -162,6 +164,10 @@ 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) { .run_case = test_name,		\
+								.name = #test_name, .parameterized = true,	\
+								.get_params = getparams }
+
 /**
  * struct kunit_suite - describes a related collection of &struct kunit_case
  *
@@ -206,6 +212,8 @@ struct kunit {
 	/* private: internal use only. */
 	const char *name; /* Read only after initialization! */
 	char *log; /* Points at case log after initialization */
+	bool parameterized;
+	int **param_values;
 	struct kunit_try_catch try_catch;
 	/*
 	 * success starts as true, and may only be set to false during a
@@ -224,7 +232,8 @@ struct kunit {
 	struct list_head resources; /* Protected by lock. */
 };
 
-void kunit_init_test(struct kunit *test, const char *name, char *log);
+void kunit_init_test(struct kunit *test, const char *name, char *log, bool parameterized);
+void kunit_init_param_test(struct kunit *test, struct kunit_case *test_case);
 
 int kunit_run_tests(struct kunit_suite *suite);
 
lib/kunit/kunit-test.c +2 -2
index 69f902440a0e..b79e287ca19b 100644
@@ -134,7 +134,7 @@ static void kunit_resource_test_init_resources(struct kunit *test)
 {
 	struct kunit_test_resource_context *ctx = test->priv;
 
-	kunit_init_test(&ctx->test, "testing_test_init_test", NULL);
+	kunit_init_test(&ctx->test, "testing_test_init_test", NULL, false);
 
 	KUNIT_EXPECT_TRUE(test, list_empty(&ctx->test.resources));
 }
@@ -370,7 +370,7 @@ static int kunit_resource_test_init(struct kunit *test)
 
 	test->priv = ctx;
 
-	kunit_init_test(&ctx->test, "test_test_context", NULL);
+	kunit_init_test(&ctx->test, "test_test_context", NULL, false);
 
 	return 0;
 }
lib/kunit/test.c +13 -2
index c36037200310..2e061bfe154d 100644
@@ -224,7 +224,7 @@ void kunit_do_assertion(struct kunit *test,
 }
 EXPORT_SYMBOL_GPL(kunit_do_assertion);
 
-void kunit_init_test(struct kunit *test, const char *name, char *log)
+void kunit_init_test(struct kunit *test, const char *name, char *log, bool parameterized)
 {
 	spin_lock_init(&test->lock);
 	INIT_LIST_HEAD(&test->resources);
@@ -232,10 +232,19 @@ void kunit_init_test(struct kunit *test, const char *name, char *log)
 	test->log = log;
 	if (test->log)
 		test->log[0] = '\0';
+	test->parameterized = parameterized;
 	test->success = true;
 }
 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->param_values = test_case->get_params();
+}
+EXPORT_SYMBOL_GPL(kunit_init_param_test);
+
 /*
  * Initializes and runs test case. Does not clean up or do post validations.
  */
@@ -342,7 +351,9 @@ static void kunit_run_case_catch_errors(struct kunit_suite *suite,
 	struct kunit_try_catch *try_catch;
 	struct kunit test;
 
-	kunit_init_test(&test, test_case->name, test_case->log);
+	kunit_init_test(&test, test_case->name, test_case->log, test_case->parameterized);
+	if (test_case->parameterized)
+		kunit_init_param_test(&test, test_case);
 	try_catch = &test.try_catch;
 
 	kunit_try_catch_init(try_catch,
Signature · 6 lines
2.25.1

_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
#1 lore raw
Quoted 15 lines · Arpitha Raghunandan

This patch has a basic implementation for adding support for parameterized testing in KUnit. It is not complete. Currently there is only support for integer parameters for test cases. I will generalize this to support other data types as well. I have tested this by making some small changes to the ext4/inode-test.c test. The main logic of this test hasn't been changed. However, I have created an integer array of inputs. The test only shows that these values can be accessed through the kunit struct as shown by the dmesg output:

# inode_test_xtimestamp_decoding:
Parameterized = 1
# inode_test_xtimestamp_decoding:
a[0][0] = 1 and a[0][1] = 2 and a[1][0] = 3 and a[1][1] = 4

Quoted 5 lines

While this does not affect the test, it shows that this initial approach for parameterized testing works.
Please let me know any suggestions for this approach and how it can be improved.

All in all, looks like a good start, but I think we might need a slightly more concrete example.

fs/ext4/inode-test.c

Quoted 8 lines
---
 fs/ext4/inode-test.c   | 22 +++++++++++++++++++++-
 include/kunit/test.h   | 11 ++++++++++-
 lib/kunit/kunit-test.c |  4 ++--
 lib/kunit/test.c       | 15 +++++++++++++--
 4 files changed, 46 insertions(+), 6 deletions(-)
fs/ext4/inode-test.c +0 -0

This is maintained by non-KUnit people; this should probably be in another commit.

fs/ext4/inode-test.c static void inode_test_xtimestamp_decoding(struct kunit *test) · line 242

Quoted 12 lines

index d62d802c9c12..5ef242f2fef3 100644

fs/ext4/inode-test.c +5 -0
@@ -235,6 +235,11 @@ static void inode_test_xtimestamp_decoding(struct kunit *test)
        struct timespec64 timestamp;
        int i;
 
+       kunit_info(test, "\nParameterized = %d\n", test->parameterized);
+       kunit_info(test, "\na[0][0] = %d and a[0][1] = %d and a[1][0] = %d and a[1][1] = %d\n",
+                                               test->param_values[0][0], test->param_values[0][1],
+                                               test->param_values[1][0], test->param_values[1][1]);
+

These look like debug statements. Did you forget to remove them?

Quoted 110 lines
fs/ext4/inode-test.c +16 -1
        for (i = 0; i < ARRAY_SIZE(test_data); ++i) {
                timestamp.tv_sec = get_32bit_time(&test_data[i]);
                ext4_decode_extra_time(&timestamp,
@@ -259,8 +264,23 @@ static void inode_test_xtimestamp_decoding(struct kunit *test)
        }
 }
 
+int **getParams(void)
+{
+       int *x = (int *)kmalloc(sizeof(int) * 2, GFP_KERNEL);
+       int *y = (int *)kmalloc(sizeof(int) * 2, GFP_KERNEL);
+       int **a = (int **)kmalloc(sizeof(x) * 2, GFP_KERNEL);
+
+       x[0] = 1;
+       x[1] = 2;
+       y[0] = 3;
+       y[1] = 4;
+       a[0] = x;
+       a[1] = y;
+       return a;
+}
+
 static struct kunit_case ext4_inode_test_cases[] = {
-       KUNIT_CASE(inode_test_xtimestamp_decoding),
+       KUNIT_CASE_PARAM(inode_test_xtimestamp_decoding, getParams),
        {}
 };
 
include/kunit/test.h +10 -1
index 59f3144f009a..23e4ff68c4b5 100644
@@ -140,6 +140,8 @@ struct kunit;
 struct kunit_case {
        void (*run_case)(struct kunit *test);
        const char *name;
+       bool parameterized;
+       int** (*get_params)(void);
 
        /* private: internal use only. */
        bool success;
@@ -162,6 +164,10 @@ 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) { .run_case = test_name,                \
+                                                               .name = #test_name, .parameterized = true,      \
+                                                               .get_params = getparams }
+
 /**
  * struct kunit_suite - describes a related collection of &struct kunit_case
  *
@@ -206,6 +212,8 @@ struct kunit {
        /* private: internal use only. */
        const char *name; /* Read only after initialization! */
        char *log; /* Points at case log after initialization */
+       bool parameterized;
+       int **param_values;
        struct kunit_try_catch try_catch;
        /*
         * success starts as true, and may only be set to false during a
@@ -224,7 +232,8 @@ struct kunit {
        struct list_head resources; /* Protected by lock. */
 };
 
-void kunit_init_test(struct kunit *test, const char *name, char *log);
+void kunit_init_test(struct kunit *test, const char *name, char *log, bool parameterized);
+void kunit_init_param_test(struct kunit *test, struct kunit_case *test_case);
 
 int kunit_run_tests(struct kunit_suite *suite);
 
lib/kunit/kunit-test.c +2 -2
index 69f902440a0e..b79e287ca19b 100644
@@ -134,7 +134,7 @@ static void kunit_resource_test_init_resources(struct kunit *test)
 {
        struct kunit_test_resource_context *ctx = test->priv;
 
-       kunit_init_test(&ctx->test, "testing_test_init_test", NULL);
+       kunit_init_test(&ctx->test, "testing_test_init_test", NULL, false);
 
        KUNIT_EXPECT_TRUE(test, list_empty(&ctx->test.resources));
 }
@@ -370,7 +370,7 @@ static int kunit_resource_test_init(struct kunit *test)
 
        test->priv = ctx;
 
-       kunit_init_test(&ctx->test, "test_test_context", NULL);
+       kunit_init_test(&ctx->test, "test_test_context", NULL, false);
 
        return 0;
 }
lib/kunit/test.c +1 -1
index c36037200310..2e061bfe154d 100644
@@ -224,7 +224,7 @@ void kunit_do_assertion(struct kunit *test,
 }
 EXPORT_SYMBOL_GPL(kunit_do_assertion);
 
-void kunit_init_test(struct kunit *test, const char *name, char *log)
+void kunit_init_test(struct kunit *test, const char *name, char *log, bool parameterized)
 {
        spin_lock_init(&test->lock);
        INIT_LIST_HEAD(&test->resources);
@@ -232,10 +232,19 @@ void kunit_init_test(struct kunit *test, const char *name, char *log)
        test->log = log;
        if (test->log)
                test->log[0] = '\0';

lib/kunit/test.c void kunit_init_test(struct kunit *test, const char *name, char *log) · line 244

Quoted 10 lines
lib/kunit/test.c +6 -0
+       test->parameterized = parameterized;
        test->success = true;
 }
 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->param_values = test_case->get_params();

I don't see where you are using this other than in those debug statements; can we see something more concrete?

Quoted 18 lines
lib/kunit/test.c +6 -2
+}
+EXPORT_SYMBOL_GPL(kunit_init_param_test);
+
 /*
  * Initializes and runs test case. Does not clean up or do post validations.
  */
@@ -342,7 +351,9 @@ static void kunit_run_case_catch_errors(struct kunit_suite *suite,
        struct kunit_try_catch *try_catch;
        struct kunit test;
 
-       kunit_init_test(&test, test_case->name, test_case->log);
+       kunit_init_test(&test, test_case->name, test_case->log, test_case->parameterized);
+       if (test_case->parameterized)
+               kunit_init_param_test(&test, test_case);
        try_catch = &test.try_catch;
 
        kunit_try_catch_init(try_catch,
--
Quoted 2 lines

2.25.1

_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees