From: Brendan Higgins via Linux-kernel-mentees <linux-kernel-mentees@lists.linuxfoundation.org>
To: Arpitha Raghunandan <98.arpi@gmail.com>
Cc: linux-kernel-mentees@lists.linuxfoundation.org,
KUnit Development <kunit-dev@googlegroups.com>
Subject: Re: [Linux-kernel-mentees] [RFC] kunit: Support for Parameterized Testing
Date: Wed, 23 Sep 2020 20:01:39 -0700 [thread overview]
Message-ID: <CAFd5g44Ba4aZaudgEO-0g5PBWm5_wz39f25qHrszqNVJTOxy6w@mail.gmail.com> (raw)
In-Reply-To: <20200920064001.83359-1-98.arpi@gmail.com>
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(×tamp,
> @@ -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
prev parent reply other threads:[~2020-09-24 3:02 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFd5g44Ba4aZaudgEO-0g5PBWm5_wz39f25qHrszqNVJTOxy6w@mail.gmail.com \
--to=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=98.arpi@gmail.com \
--cc=brendanhiggins@google.com \
--cc=kunit-dev@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).