linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 1/2] kunit: Support for Parameterized Testing
@ 2020-11-06 18:26 Arpitha Raghunandan
  2020-11-06 18:28 ` [PATCH v5 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Arpitha Raghunandan
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Arpitha Raghunandan @ 2020-11-06 18:26 UTC (permalink / raw)
  To: brendanhiggins, skhan, elver, yzaikin, tytso, adilger.kernel
  Cc: Arpitha Raghunandan, linux-kselftest, kunit-dev, linux-kernel,
	linux-kernel-mentees, linux-ext4

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 v4->v5:
- Update kernel-doc comments.
- Use const void* for generator return and prev value types.
- Add kernel-doc comment for KUNIT_ARRAY_PARAM.
- Rework parameterized test case execution strategy: each parameter is executed
  as if it was its own test case, with its own test initialization and cleanup
  (init and exit are called, etc.). However, we cannot add new test cases per TAP
  protocol once we have already started execution. Instead, log the result of
  each parameter run as a diagnostic comment.
Changes v3->v4:
- Rename kunit variables
- Rename generator function helper macro
- Add documentation for generator approach
- Display test case name in case of failure along with param index
Changes v2->v3:
- Modifictaion of generator macro and method
Changes v1->v2:
- Use of a generator method to access test case parameters

 include/kunit/test.h | 36 ++++++++++++++++++++++++++++++++++
 lib/kunit/test.c     | 46 +++++++++++++++++++++++++++++++-------------
 2 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 9197da792336..ae5488a37e48 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -107,6 +107,7 @@ struct kunit;
  *
  * @run_case: the function representing the actual test case.
  * @name:     the name of the test case.
+ * @generate_params: the generator function for parameterized tests.
  *
  * A test case is a function with the signature,
  * ``void (*)(struct kunit *)``
@@ -141,6 +142,7 @@ struct kunit;
 struct kunit_case {
 	void (*run_case)(struct kunit *test);
 	const char *name;
+	const void* (*generate_params)(const void *prev);
 
 	/* private: internal use only. */
 	bool success;
@@ -163,6 +165,22 @@ static inline char *kunit_status_to_string(bool status)
  */
 #define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name }
 
+/**
+ * KUNIT_CASE_PARAM - A helper for creation a parameterized &struct kunit_case
+ *
+ * @test_name: a reference to a test case function.
+ * @gen_params: a reference to a parameter generator function.
+ *
+ * The generator function ``const void* gen_params(const void *prev)`` 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.
+ */
+#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 +226,10 @@ struct kunit {
 	const char *name; /* Read only after initialization! */
 	char *log; /* Points at case log after initialization */
 	struct kunit_try_catch try_catch;
+	/* param_value is the current parameter value for a test case. */
+	const void *param_value;
+	/* param_index stores the index of the parameter in parameterized tests. */
+	int param_index;
 	/*
 	 * 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 +1764,18 @@ do {									       \
 						fmt,			       \
 						##__VA_ARGS__)
 
+/**
+ * KUNIT_ARRAY_PARAM() - Define test parameter generator from an array.
+ * @name:  prefix for the test parameter generator function.
+ * @array: array of test parameters.
+ *
+ * Define function @name_gen_params which uses @array to generate parameters.
+ */
+#define KUNIT_ARRAY_PARAM(name, array)								\
+	static const void *name##_gen_params(const void *prev)					\
+	{											\
+		typeof((array)[0]) * __next = prev ? ((typeof(__next)) prev) + 1 : (array);	\
+		return __next - (array) < ARRAY_SIZE((array)) ? __next : NULL;			\
+	}
+
 #endif /* _KUNIT_TEST_H */
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 750704abe89a..b8b63aeda504 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -325,29 +325,25 @@ static void kunit_catch_run_case(void *data)
  * occur in a test case and reports them as failures.
  */
 static void kunit_run_case_catch_errors(struct kunit_suite *suite,
-					struct kunit_case *test_case)
+					struct kunit_case *test_case,
+					struct kunit *test)
 {
 	struct kunit_try_catch_context context;
 	struct kunit_try_catch *try_catch;
-	struct kunit test;
 
-	kunit_init_test(&test, test_case->name, test_case->log);
-	try_catch = &test.try_catch;
+	kunit_init_test(test, test_case->name, test_case->log);
+	try_catch = &test->try_catch;
 
 	kunit_try_catch_init(try_catch,
-			     &test,
+			     test,
 			     kunit_try_run_case,
 			     kunit_catch_run_case);
-	context.test = &test;
+	context.test = test;
 	context.suite = suite;
 	context.test_case = test_case;
 	kunit_try_catch_run(try_catch, &context);
 
-	test_case->success = test.success;
-
-	kunit_print_ok_not_ok(&test, true, test_case->success,
-			      kunit_test_case_num(suite, test_case),
-			      test_case->name);
+	test_case->success = test->success;
 }
 
 int kunit_run_tests(struct kunit_suite *suite)
@@ -356,8 +352,32 @@ int kunit_run_tests(struct kunit_suite *suite)
 
 	kunit_print_subtest_start(suite);
 
-	kunit_suite_for_each_test_case(suite, test_case)
-		kunit_run_case_catch_errors(suite, test_case);
+	kunit_suite_for_each_test_case(suite, test_case) {
+		struct kunit test = { .param_value = NULL, .param_index = 0 };
+		bool test_success = true;
+
+		if (test_case->generate_params)
+			test.param_value = test_case->generate_params(NULL);
+
+		do {
+			kunit_run_case_catch_errors(suite, test_case, &test);
+			test_success &= test_case->success;
+
+			if (test_case->generate_params) {
+				kunit_log(KERN_INFO, &test,
+					KUNIT_SUBTEST_INDENT
+					"# %s: param-%d %s",
+					test_case->name, test.param_index,
+					kunit_status_to_string(test.success));
+				test.param_value = test_case->generate_params(test.param_value);
+				test.param_index++;
+			}
+		} while (test.param_value);
+
+		kunit_print_ok_not_ok(&test, true, test_success,
+				      kunit_test_case_num(suite, test_case),
+				      test_case->name);
+	}
 
 	kunit_print_subtest_end(suite);
 
-- 
2.25.1


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

* [PATCH v5 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature
  2020-11-06 18:26 [PATCH v5 1/2] kunit: Support for Parameterized Testing Arpitha Raghunandan
@ 2020-11-06 18:28 ` Arpitha Raghunandan
  2020-11-06 18:38   ` Marco Elver
  2020-11-06 18:37 ` [PATCH v5 1/2] kunit: Support for Parameterized Testing Marco Elver
  2020-11-06 18:45 ` Marco Elver
  2 siblings, 1 reply; 7+ messages in thread
From: Arpitha Raghunandan @ 2020-11-06 18:28 UTC (permalink / raw)
  To: brendanhiggins, skhan, elver, yzaikin, tytso, adilger.kernel
  Cc: Arpitha Raghunandan, linux-kselftest, kunit-dev, linux-kernel,
	linux-kernel-mentees, linux-ext4

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

Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com>
---
Changes v4->v5:
- No change to this patch of the patch series
Changes v3->v4:
- Modification based on latest implementation of KUnit parameterized testing
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..ebf1b1af4f1d 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_ARRAY_PARAM(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_value);
+
+	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


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

* Re: [PATCH v5 1/2] kunit: Support for Parameterized Testing
  2020-11-06 18:26 [PATCH v5 1/2] kunit: Support for Parameterized Testing Arpitha Raghunandan
  2020-11-06 18:28 ` [PATCH v5 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Arpitha Raghunandan
@ 2020-11-06 18:37 ` Marco Elver
  2020-11-06 18:45 ` Marco Elver
  2 siblings, 0 replies; 7+ messages in thread
From: Marco Elver @ 2020-11-06 18:37 UTC (permalink / raw)
  To: Arpitha Raghunandan
  Cc: Brendan Higgins, skhan, Iurii Zaikin, Theodore Ts'o,
	Andreas Dilger, open list:KERNEL SELFTEST FRAMEWORK,
	KUnit Development, LKML, linux-kernel-mentees, linux-ext4

On Fri, 6 Nov 2020 at 19:28, 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>
> ---
[...]
>  include/kunit/test.h | 36 ++++++++++++++++++++++++++++++++++
>  lib/kunit/test.c     | 46 +++++++++++++++++++++++++++++++-------------
>  2 files changed, 69 insertions(+), 13 deletions(-)

I think this is ready. Thank you for your patience, and addressing my
comments! I'm obviously fine with v5, but I think my Ack or Review
won't count much because of Co-developed. :-)

Others: Please take another look.

Thanks,
-- Marco

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

* Re: [PATCH v5 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature
  2020-11-06 18:28 ` [PATCH v5 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Arpitha Raghunandan
@ 2020-11-06 18:38   ` Marco Elver
  0 siblings, 0 replies; 7+ messages in thread
From: Marco Elver @ 2020-11-06 18:38 UTC (permalink / raw)
  To: Arpitha Raghunandan
  Cc: Brendan Higgins, skhan, Iurii Zaikin, Theodore Ts'o,
	Andreas Dilger, open list:KERNEL SELFTEST FRAMEWORK,
	KUnit Development, LKML, linux-kernel-mentees, linux-ext4

On Fri, 6 Nov 2020 at 19:29, Arpitha Raghunandan <98.arpi@gmail.com> wrote:
> Modify fs/ext4/inode-test.c to use the parameterized testing
> feature of KUnit.
>
> Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com>
> ---
[...]
>  fs/ext4/inode-test.c | 314 ++++++++++++++++++++++---------------------
>  1 file changed, 158 insertions(+), 156 deletions(-)

Acked-by: Marco Elver <elver@google.com>

Thanks,
-- Marco

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

* Re: [PATCH v5 1/2] kunit: Support for Parameterized Testing
  2020-11-06 18:26 [PATCH v5 1/2] kunit: Support for Parameterized Testing Arpitha Raghunandan
  2020-11-06 18:28 ` [PATCH v5 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Arpitha Raghunandan
  2020-11-06 18:37 ` [PATCH v5 1/2] kunit: Support for Parameterized Testing Marco Elver
@ 2020-11-06 18:45 ` Marco Elver
  2020-11-06 19:00   ` Arpitha Raghunandan
  2 siblings, 1 reply; 7+ messages in thread
From: Marco Elver @ 2020-11-06 18:45 UTC (permalink / raw)
  To: Arpitha Raghunandan
  Cc: Brendan Higgins, skhan, Iurii Zaikin, Theodore Ts'o,
	Andreas Dilger, open list:KERNEL SELFTEST FRAMEWORK,
	KUnit Development, LKML, linux-kernel-mentees, linux-ext4

On Fri, 6 Nov 2020 at 19:28, 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>
[...]
> -       kunit_suite_for_each_test_case(suite, test_case)
> -               kunit_run_case_catch_errors(suite, test_case);
> +       kunit_suite_for_each_test_case(suite, test_case) {
> +               struct kunit test = { .param_value = NULL, .param_index = 0 };
> +               bool test_success = true;
> +
> +               if (test_case->generate_params)
> +                       test.param_value = test_case->generate_params(NULL);
> +
> +               do {
> +                       kunit_run_case_catch_errors(suite, test_case, &test);
> +                       test_success &= test_case->success;
> +
> +                       if (test_case->generate_params) {
> +                               kunit_log(KERN_INFO, &test,
> +                                       KUNIT_SUBTEST_INDENT
> +                                       "# %s: param-%d %s",
> +                                       test_case->name, test.param_index,
> +                                       kunit_status_to_string(test.success));

Sorry, I still found something. The patch I sent had this aligned with
the '(', whereas when I apply this patch it no longer is aligned. Why?

I see the rest of the file also aligns arguments with opening '(', so
I think your change is inconsistent.

Thanks,
-- Marco

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

* Re: [PATCH v5 1/2] kunit: Support for Parameterized Testing
  2020-11-06 18:45 ` Marco Elver
@ 2020-11-06 19:00   ` Arpitha Raghunandan
  2020-11-06 19:05     ` Marco Elver
  0 siblings, 1 reply; 7+ messages in thread
From: Arpitha Raghunandan @ 2020-11-06 19:00 UTC (permalink / raw)
  To: Marco Elver
  Cc: Brendan Higgins, skhan, Iurii Zaikin, Theodore Ts'o,
	Andreas Dilger, open list:KERNEL SELFTEST FRAMEWORK,
	KUnit Development, LKML, linux-kernel-mentees, linux-ext4

On 07/11/20 12:15 am, Marco Elver wrote:
> On Fri, 6 Nov 2020 at 19:28, 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>
> [...]
>> -       kunit_suite_for_each_test_case(suite, test_case)
>> -               kunit_run_case_catch_errors(suite, test_case);
>> +       kunit_suite_for_each_test_case(suite, test_case) {
>> +               struct kunit test = { .param_value = NULL, .param_index = 0 };
>> +               bool test_success = true;
>> +
>> +               if (test_case->generate_params)
>> +                       test.param_value = test_case->generate_params(NULL);
>> +
>> +               do {
>> +                       kunit_run_case_catch_errors(suite, test_case, &test);
>> +                       test_success &= test_case->success;
>> +
>> +                       if (test_case->generate_params) {
>> +                               kunit_log(KERN_INFO, &test,
>> +                                       KUNIT_SUBTEST_INDENT
>> +                                       "# %s: param-%d %s",
>> +                                       test_case->name, test.param_index,
>> +                                       kunit_status_to_string(test.success));
> 
> Sorry, I still found something. The patch I sent had this aligned with
> the '(', whereas when I apply this patch it no longer is aligned. Why?
> 
> I see the rest of the file also aligns arguments with opening '(', so
> I think your change is inconsistent.
> 

Ah those lines had spaces instead of tab and I think I messed up the alignment
fixing that. I will send another version fixing this.
Thanks!

> Thanks,
> -- Marco
> 


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

* Re: [PATCH v5 1/2] kunit: Support for Parameterized Testing
  2020-11-06 19:00   ` Arpitha Raghunandan
@ 2020-11-06 19:05     ` Marco Elver
  0 siblings, 0 replies; 7+ messages in thread
From: Marco Elver @ 2020-11-06 19:05 UTC (permalink / raw)
  To: Arpitha Raghunandan
  Cc: Brendan Higgins, skhan, Iurii Zaikin, Theodore Ts'o,
	Andreas Dilger, open list:KERNEL SELFTEST FRAMEWORK,
	KUnit Development, LKML, linux-kernel-mentees, linux-ext4

On Fri, 6 Nov 2020 at 20:00, Arpitha Raghunandan <98.arpi@gmail.com> wrote:
>
> On 07/11/20 12:15 am, Marco Elver wrote:
> > On Fri, 6 Nov 2020 at 19:28, 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>
> > [...]
> >> -       kunit_suite_for_each_test_case(suite, test_case)
> >> -               kunit_run_case_catch_errors(suite, test_case);
> >> +       kunit_suite_for_each_test_case(suite, test_case) {
> >> +               struct kunit test = { .param_value = NULL, .param_index = 0 };
> >> +               bool test_success = true;
> >> +
> >> +               if (test_case->generate_params)
> >> +                       test.param_value = test_case->generate_params(NULL);
> >> +
> >> +               do {
> >> +                       kunit_run_case_catch_errors(suite, test_case, &test);
> >> +                       test_success &= test_case->success;
> >> +
> >> +                       if (test_case->generate_params) {
> >> +                               kunit_log(KERN_INFO, &test,
> >> +                                       KUNIT_SUBTEST_INDENT
> >> +                                       "# %s: param-%d %s",
> >> +                                       test_case->name, test.param_index,
> >> +                                       kunit_status_to_string(test.success));
> >
> > Sorry, I still found something. The patch I sent had this aligned with
> > the '(', whereas when I apply this patch it no longer is aligned. Why?
> >
> > I see the rest of the file also aligns arguments with opening '(', so
> > I think your change is inconsistent.
> >
>
> Ah those lines had spaces instead of tab and I think I messed up the alignment
> fixing that. I will send another version fixing this.
> Thanks!

It was tabs then <8 spaces to align. checkpatch.pl certainly is happy with that.

> > Thanks,
> > -- Marco
> >
>

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

end of thread, other threads:[~2020-11-06 19:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-06 18:26 [PATCH v5 1/2] kunit: Support for Parameterized Testing Arpitha Raghunandan
2020-11-06 18:28 ` [PATCH v5 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Arpitha Raghunandan
2020-11-06 18:38   ` Marco Elver
2020-11-06 18:37 ` [PATCH v5 1/2] kunit: Support for Parameterized Testing Marco Elver
2020-11-06 18:45 ` Marco Elver
2020-11-06 19:00   ` Arpitha Raghunandan
2020-11-06 19:05     ` Marco Elver

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).