linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
* [Linux-kernel-mentees] [RFC] kunit: Support for Parameterized Testing
@ 2020-09-20  6:40 Arpitha Raghunandan
  2020-09-24  3:01 ` Brendan Higgins via Linux-kernel-mentees
  0 siblings, 1 reply; 2+ messages in thread
From: Arpitha Raghunandan @ 2020-09-20  6:40 UTC (permalink / raw)
  To: brendanhiggins; +Cc: Arpitha Raghunandan, linux-kernel-mentees, kunit-dev

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.

Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com>
---
 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(-)

diff --git a/fs/ext4/inode-test.c b/fs/ext4/inode-test.c
index d62d802c9c12..5ef242f2fef3 100644
--- a/fs/ext4/inode-test.c
+++ b/fs/ext4/inode-test.c
@@ -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),
 	{}
 };
 
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 59f3144f009a..23e4ff68c4b5 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -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);
 
diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c
index 69f902440a0e..b79e287ca19b 100644
--- a/lib/kunit/kunit-test.c
+++ b/lib/kunit/kunit-test.c
@@ -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;
 }
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index c36037200310..2e061bfe154d 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -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,
-- 
2.25.1

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [Linux-kernel-mentees] [RFC] kunit: Support for Parameterized Testing
  2020-09-20  6:40 [Linux-kernel-mentees] [RFC] kunit: Support for Parameterized Testing Arpitha Raghunandan
@ 2020-09-24  3:01 ` Brendan Higgins via Linux-kernel-mentees
  0 siblings, 0 replies; 2+ messages in thread
From: Brendan Higgins via Linux-kernel-mentees @ 2020-09-24  3:01 UTC (permalink / raw)
  To: Arpitha Raghunandan; +Cc: linux-kernel-mentees, KUnit Development

On Sat, Sep 19, 2020 at 11:40 PM Arpitha Raghunandan <98.arpi@gmail.com> wrote:
>
> 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.
>
> Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com>

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

> ---
>  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(-)
>
> diff --git a/fs/ext4/inode-test.c b/fs/ext4/inode-test.c

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

> index d62d802c9c12..5ef242f2fef3 100644
> --- a/fs/ext4/inode-test.c
> +++ b/fs/ext4/inode-test.c
> @@ -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?

>         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),
>         {}
>  };
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 59f3144f009a..23e4ff68c4b5 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -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);
>
> diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c
> index 69f902440a0e..b79e287ca19b 100644
> --- a/lib/kunit/kunit-test.c
> +++ b/lib/kunit/kunit-test.c
> @@ -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;
>  }
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index c36037200310..2e061bfe154d 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -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();

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

> +}
> +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,
> --
> 2.25.1
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-09-24  3:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-20  6:40 [Linux-kernel-mentees] [RFC] kunit: Support for Parameterized Testing Arpitha Raghunandan
2020-09-24  3:01 ` Brendan Higgins via Linux-kernel-mentees

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).