linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 1/2] kunit: Support for Parameterized Testing
@ 2020-11-16  5:40 Arpitha Raghunandan
  2020-11-16  5:41 ` [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Arpitha Raghunandan
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Arpitha Raghunandan @ 2020-11-16  5:40 UTC (permalink / raw)
  To: brendanhiggins, skhan, elver, yzaikin, tytso, adilger.kernel,
	Tim.Bird, davidgow
  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 based on arrays. Generators may also
optionally provide a human-readable description of parameters, which is
displayed where available.

Note, currently the result of each parameter run is displayed in
diagnostic lines, and only the overall test case output summarizes
TAP-compliant success or failure of all parameter runs. In future, when
supported by kunit-tool, these can be turned into subsubtest outputs.

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 v8->v9:
- No change to this patch of the patch series

Changes v7->v8:
- Increase KUNIT_PARAM_DESC_SIZE to 128
- Format pointer style appropriately 

Changes v6->v7:
- Clarify commit message.
- Introduce ability to optionally generate descriptions for parameters;
  if no description is provided, we'll still print 'param-N'.
- Change diagnostic line format to:
        # <test-case-name>: <ok|not ok> N - [<param description>]

Changes v5->v6:
- Fix alignment to maintain consistency

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
Changes v6->v7:
- Clarify commit message.
- Introduce ability to optionally generate descriptions for parameters;
  if no description is provided, we'll still print 'param-N'.
- Change diagnostic line format to:
        # <test-case-name>: <ok|not ok> N - [<param description>]
- Before execution of parameterized test case, count number of
  parameters and display number of parameters. Currently also as a
  diagnostic line, but this may be used in future to generate a subsubtest
  plan. A requirement of this change is that generators must generate a
  deterministic number of parameters.

Changes v5->v6:
- Fix alignment to maintain consistency

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 | 51 ++++++++++++++++++++++++++++++++++++++
 lib/kunit/test.c     | 59 ++++++++++++++++++++++++++++++++++----------
 2 files changed, 97 insertions(+), 13 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index db1b0ae666c4..27b42a008c7a 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -94,6 +94,9 @@ struct kunit;
 /* Size of log associated with test. */
 #define KUNIT_LOG_SIZE	512
 
+/* Maximum size of parameter description string. */
+#define KUNIT_PARAM_DESC_SIZE 128
+
 /*
  * TAP specifies subtest stream indentation of 4 spaces, 8 spaces for a
  * sub-subtest.  See the "Subtests" section in
@@ -107,6 +110,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 +145,7 @@ struct kunit;
 struct kunit_case {
 	void (*run_case)(struct kunit *test);
 	const char *name;
+	const void* (*generate_params)(const void *prev, char *desc);
 
 	/* private: internal use only. */
 	bool success;
@@ -163,6 +168,27 @@ 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, char *desc)
+ *
+ * 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.
+ * Optionally write a string into @desc (size of KUNIT_PARAM_DESC_SIZE)
+ * describing the parameter.
+ */
+#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 +234,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 +1772,25 @@ 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.
+ * @get_desc: function to convert param to description; NULL to use default
+ *
+ * Define function @name_gen_params which uses @array to generate parameters.
+ */
+#define KUNIT_ARRAY_PARAM(name, array, get_desc)						\
+	static const void *name##_gen_params(const void *prev, char *desc)			\
+	{											\
+		typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 : (array);	\
+		if (__next - (array) < ARRAY_SIZE((array))) {					\
+			void (*__get_desc)(typeof(__next), char *) = get_desc;			\
+			if (__get_desc)								\
+				__get_desc(__next, desc);					\
+			return __next;								\
+		}										\
+		return NULL;									\
+	}
+
 #endif /* _KUNIT_TEST_H */
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 750704abe89a..ec9494e914ef 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -325,39 +325,72 @@ 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)
 {
+	char param_desc[KUNIT_PARAM_DESC_SIZE];
 	struct kunit_case *test_case;
 
 	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) {
+			/* Get initial param. */
+			param_desc[0] = '\0';
+			test.param_value = test_case->generate_params(NULL, param_desc);
+		}
+
+		do {
+			kunit_run_case_catch_errors(suite, test_case, &test);
+			test_success &= test_case->success;
+
+			if (test_case->generate_params) {
+				if (param_desc[0] == '\0') {
+					snprintf(param_desc, sizeof(param_desc),
+						 "param-%d", test.param_index);
+				}
+
+				kunit_log(KERN_INFO, &test,
+					  KUNIT_SUBTEST_INDENT
+					  "# %s: %s %d - %s",
+					  test_case->name,
+					  kunit_status_to_string(test.success),
+					  test.param_index + 1, param_desc);
+
+				/* Get next param. */
+				param_desc[0] = '\0';
+				test.param_value = test_case->generate_params(test.param_value, param_desc);
+				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] 17+ messages in thread

* [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature
  2020-11-16  5:40 [PATCH v9 1/2] kunit: Support for Parameterized Testing Arpitha Raghunandan
@ 2020-11-16  5:41 ` Arpitha Raghunandan
  2020-11-16  8:51   ` Marco Elver
                     ` (3 more replies)
  2020-11-16  8:53 ` [PATCH v9 1/2] kunit: Support for Parameterized Testing Marco Elver
  2020-11-17  7:20 ` David Gow
  2 siblings, 4 replies; 17+ messages in thread
From: Arpitha Raghunandan @ 2020-11-16  5:41 UTC (permalink / raw)
  To: brendanhiggins, skhan, elver, yzaikin, tytso, adilger.kernel,
	Tim.Bird, davidgow
  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>
Signed-off-by: Marco Elver <elver@google.com>
---
Changes v8->v9:
- Replace strncpy() with strscpy() in timestamp_expectation_to_desc()
Changes v7->v8:
- Replace strcpy() with strncpy() in timestamp_expectation_to_desc()
Changes v6->v7:
- Introduce timestamp_expectation_to_desc() to convert param to
  description.
Changes v5->v6:
- No change to this patch of the patch series
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 | 320 ++++++++++++++++++++++---------------------
 1 file changed, 164 insertions(+), 156 deletions(-)

diff --git a/fs/ext4/inode-test.c b/fs/ext4/inode-test.c
index d62d802c9c12..7935ea6cf92c 100644
--- a/fs/ext4/inode-test.c
+++ b/fs/ext4/inode-test.c
@@ -80,6 +80,145 @@ 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},
+	}
+};
+
+static void timestamp_expectation_to_desc(const struct timestamp_expectation *t,
+					  char *desc)
+{
+	strscpy(desc, t->test_case_name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(ext4_inode, test_data, timestamp_expectation_to_desc);
+
 static time64_t get_32bit_time(const struct timestamp_expectation * const test)
 {
 	if (test->msb_set) {
@@ -101,166 +240,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] 17+ messages in thread

* Re: [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature
  2020-11-16  5:41 ` [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Arpitha Raghunandan
@ 2020-11-16  8:51   ` Marco Elver
  2020-11-17  7:22   ` David Gow
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: Marco Elver @ 2020-11-16  8:51 UTC (permalink / raw)
  To: Arpitha Raghunandan
  Cc: Brendan Higgins, Shuah Khan, Iurii Zaikin, Theodore Ts'o,
	Andreas Dilger, Tim Bird, David Gow,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development, LKML,
	linux-kernel-mentees, linux-ext4

On Mon, 16 Nov 2020 at 06:42, 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>
> Signed-off-by: Marco Elver <elver@google.com>

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

Thank you!

> ---
> Changes v8->v9:
> - Replace strncpy() with strscpy() in timestamp_expectation_to_desc()
> Changes v7->v8:
> - Replace strcpy() with strncpy() in timestamp_expectation_to_desc()
> Changes v6->v7:
> - Introduce timestamp_expectation_to_desc() to convert param to
>   description.
> Changes v5->v6:
> - No change to this patch of the patch series
> 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 | 320 ++++++++++++++++++++++---------------------
>  1 file changed, 164 insertions(+), 156 deletions(-)
>
> diff --git a/fs/ext4/inode-test.c b/fs/ext4/inode-test.c
> index d62d802c9c12..7935ea6cf92c 100644
> --- a/fs/ext4/inode-test.c
> +++ b/fs/ext4/inode-test.c
> @@ -80,6 +80,145 @@ 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},
> +       }
> +};
> +
> +static void timestamp_expectation_to_desc(const struct timestamp_expectation *t,
> +                                         char *desc)
> +{
> +       strscpy(desc, t->test_case_name, KUNIT_PARAM_DESC_SIZE);
> +}
> +
> +KUNIT_ARRAY_PARAM(ext4_inode, test_data, timestamp_expectation_to_desc);
> +
>  static time64_t get_32bit_time(const struct timestamp_expectation * const test)
>  {
>         if (test->msb_set) {
> @@ -101,166 +240,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	[flat|nested] 17+ messages in thread

* Re: [PATCH v9 1/2] kunit: Support for Parameterized Testing
  2020-11-16  5:40 [PATCH v9 1/2] kunit: Support for Parameterized Testing Arpitha Raghunandan
  2020-11-16  5:41 ` [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Arpitha Raghunandan
@ 2020-11-16  8:53 ` Marco Elver
  2020-11-17  7:20 ` David Gow
  2 siblings, 0 replies; 17+ messages in thread
From: Marco Elver @ 2020-11-16  8:53 UTC (permalink / raw)
  To: Arpitha Raghunandan
  Cc: Brendan Higgins, Shuah Khan, Iurii Zaikin, Theodore Ts'o,
	Andreas Dilger, Tim Bird, David Gow,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development, LKML,
	linux-kernel-mentees, linux-ext4

On Mon, 16 Nov 2020 at 06:41, 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 based on arrays. Generators may also
> optionally provide a human-readable description of parameters, which is
> displayed where available.
>
> Note, currently the result of each parameter run is displayed in
> diagnostic lines, and only the overall test case output summarizes
> TAP-compliant success or failure of all parameter runs. In future, when
> supported by kunit-tool, these can be turned into subsubtest outputs.
>
> 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>

This and patch 2/2 look good to me.

Thank you!

-- Marco

> ---
> Changes v8->v9:
> - No change to this patch of the patch series
>
> Changes v7->v8:
> - Increase KUNIT_PARAM_DESC_SIZE to 128
> - Format pointer style appropriately
>
> Changes v6->v7:
> - Clarify commit message.
> - Introduce ability to optionally generate descriptions for parameters;
>   if no description is provided, we'll still print 'param-N'.
> - Change diagnostic line format to:
>         # <test-case-name>: <ok|not ok> N - [<param description>]
>
> Changes v5->v6:
> - Fix alignment to maintain consistency
>
> 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
> Changes v6->v7:
> - Clarify commit message.
> - Introduce ability to optionally generate descriptions for parameters;
>   if no description is provided, we'll still print 'param-N'.
> - Change diagnostic line format to:
>         # <test-case-name>: <ok|not ok> N - [<param description>]
> - Before execution of parameterized test case, count number of
>   parameters and display number of parameters. Currently also as a
>   diagnostic line, but this may be used in future to generate a subsubtest
>   plan. A requirement of this change is that generators must generate a
>   deterministic number of parameters.
>
> Changes v5->v6:
> - Fix alignment to maintain consistency
>
> 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 | 51 ++++++++++++++++++++++++++++++++++++++
>  lib/kunit/test.c     | 59 ++++++++++++++++++++++++++++++++++----------
>  2 files changed, 97 insertions(+), 13 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index db1b0ae666c4..27b42a008c7a 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -94,6 +94,9 @@ struct kunit;
>  /* Size of log associated with test. */
>  #define KUNIT_LOG_SIZE 512
>
> +/* Maximum size of parameter description string. */
> +#define KUNIT_PARAM_DESC_SIZE 128
> +
>  /*
>   * TAP specifies subtest stream indentation of 4 spaces, 8 spaces for a
>   * sub-subtest.  See the "Subtests" section in
> @@ -107,6 +110,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 +145,7 @@ struct kunit;
>  struct kunit_case {
>         void (*run_case)(struct kunit *test);
>         const char *name;
> +       const void* (*generate_params)(const void *prev, char *desc);
>
>         /* private: internal use only. */
>         bool success;
> @@ -163,6 +168,27 @@ 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, char *desc)
> + *
> + * 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.
> + * Optionally write a string into @desc (size of KUNIT_PARAM_DESC_SIZE)
> + * describing the parameter.
> + */
> +#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 +234,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 +1772,25 @@ 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.
> + * @get_desc: function to convert param to description; NULL to use default
> + *
> + * Define function @name_gen_params which uses @array to generate parameters.
> + */
> +#define KUNIT_ARRAY_PARAM(name, array, get_desc)                                               \
> +       static const void *name##_gen_params(const void *prev, char *desc)                      \
> +       {                                                                                       \
> +               typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 : (array);      \
> +               if (__next - (array) < ARRAY_SIZE((array))) {                                   \
> +                       void (*__get_desc)(typeof(__next), char *) = get_desc;                  \
> +                       if (__get_desc)                                                         \
> +                               __get_desc(__next, desc);                                       \
> +                       return __next;                                                          \
> +               }                                                                               \
> +               return NULL;                                                                    \
> +       }
> +
>  #endif /* _KUNIT_TEST_H */
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 750704abe89a..ec9494e914ef 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -325,39 +325,72 @@ 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)
>  {
> +       char param_desc[KUNIT_PARAM_DESC_SIZE];
>         struct kunit_case *test_case;
>
>         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) {
> +                       /* Get initial param. */
> +                       param_desc[0] = '\0';
> +                       test.param_value = test_case->generate_params(NULL, param_desc);
> +               }
> +
> +               do {
> +                       kunit_run_case_catch_errors(suite, test_case, &test);
> +                       test_success &= test_case->success;
> +
> +                       if (test_case->generate_params) {
> +                               if (param_desc[0] == '\0') {
> +                                       snprintf(param_desc, sizeof(param_desc),
> +                                                "param-%d", test.param_index);
> +                               }
> +
> +                               kunit_log(KERN_INFO, &test,
> +                                         KUNIT_SUBTEST_INDENT
> +                                         "# %s: %s %d - %s",
> +                                         test_case->name,
> +                                         kunit_status_to_string(test.success),
> +                                         test.param_index + 1, param_desc);
> +
> +                               /* Get next param. */
> +                               param_desc[0] = '\0';
> +                               test.param_value = test_case->generate_params(test.param_value, param_desc);
> +                               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	[flat|nested] 17+ messages in thread

* Re: [PATCH v9 1/2] kunit: Support for Parameterized Testing
  2020-11-16  5:40 [PATCH v9 1/2] kunit: Support for Parameterized Testing Arpitha Raghunandan
  2020-11-16  5:41 ` [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Arpitha Raghunandan
  2020-11-16  8:53 ` [PATCH v9 1/2] kunit: Support for Parameterized Testing Marco Elver
@ 2020-11-17  7:20 ` David Gow
  2020-11-23 13:08   ` Marco Elver
  2 siblings, 1 reply; 17+ messages in thread
From: David Gow @ 2020-11-17  7:20 UTC (permalink / raw)
  To: Arpitha Raghunandan
  Cc: Brendan Higgins, Shuah Khan, Marco Elver, Iurii Zaikin,
	Theodore Ts'o, Andreas Dilger, Bird, Tim,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List, linux-kernel-mentees, linux-ext4

On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also
> optionally provide a human-readable description of parameters, which is
> displayed where available.
>
> Note, currently the result of each parameter run is displayed in
> diagnostic lines, and only the overall test case output summarizes
> TAP-compliant success or failure of all parameter runs. In future, when
> supported by kunit-tool, these can be turned into subsubtest outputs.
>
> 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>
> ---
[Resending this because my email client re-defaulted to HTML! Aarrgh!]

This looks good to me! I tested it in UML and x86-64 w/ KASAN, and
both worked fine.

Reviewed-by: David Gow <davidgow@google.com>
Tested-by: David Gow <davidgow@google.com>

Thanks for sticking with this!

-- David

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

* Re: [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature
  2020-11-16  5:41 ` [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Arpitha Raghunandan
  2020-11-16  8:51   ` Marco Elver
@ 2020-11-17  7:22   ` David Gow
  2020-12-01 20:30   ` Iurii Zaikin
  2020-12-02 16:07   ` Theodore Y. Ts'o
  3 siblings, 0 replies; 17+ messages in thread
From: David Gow @ 2020-11-17  7:22 UTC (permalink / raw)
  To: Arpitha Raghunandan
  Cc: Brendan Higgins, Shuah Khan, Marco Elver, Iurii Zaikin,
	Theodore Ts'o, Andreas Dilger, Bird, Tim,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List, linux-kernel-mentees, linux-ext4

On Mon, Nov 16, 2020 at 1:42 PM 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>
> Signed-off-by: Marco Elver <elver@google.com>
> ---
[Resending this because the HTML-email demon struck again! Sorry for the mess!]


Thanks: this is working well over here.
The only (minor) issue I've noticed is that the diagnostic output is
too big for the default log buffer if debugfs output is used, causing
some of the messages to be dropped from the debugfs results file. But
that's clearly not an issue with this patch, and the actual combined
result is still present (and the complete results should show up in
dmesg anyway).

Tested-by: David Gow <davidgow@google.com>
Reviewed-by: David Gow <davidgow@google.com>

Thanks!
-- David

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

* Re: [PATCH v9 1/2] kunit: Support for Parameterized Testing
  2020-11-17  7:20 ` David Gow
@ 2020-11-23 13:08   ` Marco Elver
  2020-11-24  7:25     ` David Gow
  0 siblings, 1 reply; 17+ messages in thread
From: Marco Elver @ 2020-11-23 13:08 UTC (permalink / raw)
  To: David Gow
  Cc: Arpitha Raghunandan, Brendan Higgins, Shuah Khan, Iurii Zaikin,
	Theodore Ts'o, Andreas Dilger, Bird, Tim,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List, linux-kernel-mentees, linux-ext4

On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote:
> On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also
> > optionally provide a human-readable description of parameters, which is
> > displayed where available.
> >
> > Note, currently the result of each parameter run is displayed in
> > diagnostic lines, and only the overall test case output summarizes
> > TAP-compliant success or failure of all parameter runs. In future, when
> > supported by kunit-tool, these can be turned into subsubtest outputs.
> >
> > 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>
> > ---
> [Resending this because my email client re-defaulted to HTML! Aarrgh!]
>
> This looks good to me! I tested it in UML and x86-64 w/ KASAN, and
> both worked fine.
>
> Reviewed-by: David Gow <davidgow@google.com>
> Tested-by: David Gow <davidgow@google.com>

Thank you!

> Thanks for sticking with this!

Will these patches be landing in 5.11 or 5.12?

> -- David

Thanks,
-- Marco

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

* Re: [PATCH v9 1/2] kunit: Support for Parameterized Testing
  2020-11-23 13:08   ` Marco Elver
@ 2020-11-24  7:25     ` David Gow
  2020-11-24  8:02       ` Marco Elver
  2020-11-30 22:22       ` Brendan Higgins
  0 siblings, 2 replies; 17+ messages in thread
From: David Gow @ 2020-11-24  7:25 UTC (permalink / raw)
  To: Marco Elver, Brendan Higgins, Theodore Ts'o, Shuah Khan
  Cc: Arpitha Raghunandan, Iurii Zaikin, Andreas Dilger, Bird, Tim,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List, linux-kernel-mentees, linux-ext4

On Mon, Nov 23, 2020 at 9:08 PM Marco Elver <elver@google.com> wrote:
>
> On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote:
> > On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also
> > > optionally provide a human-readable description of parameters, which is
> > > displayed where available.
> > >
> > > Note, currently the result of each parameter run is displayed in
> > > diagnostic lines, and only the overall test case output summarizes
> > > TAP-compliant success or failure of all parameter runs. In future, when
> > > supported by kunit-tool, these can be turned into subsubtest outputs.
> > >
> > > 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>
> > > ---
> > [Resending this because my email client re-defaulted to HTML! Aarrgh!]
> >
> > This looks good to me! I tested it in UML and x86-64 w/ KASAN, and
> > both worked fine.
> >
> > Reviewed-by: David Gow <davidgow@google.com>
> > Tested-by: David Gow <davidgow@google.com>
>
> Thank you!
>
> > Thanks for sticking with this!
>
> Will these patches be landing in 5.11 or 5.12?
>

I can't think of any reason not to have these in 5.11. We haven't
started staging things in the kselftest/kunit branch for 5.11 yet,
though.

Patch 2 will probably need to be acked by Ted for ext4 first.

Brendan, Shuah: can you make sure this doesn't get lost in patchwork?

Cheers,
-- David

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

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

* Re: [PATCH v9 1/2] kunit: Support for Parameterized Testing
  2020-11-24  7:25     ` David Gow
@ 2020-11-24  8:02       ` Marco Elver
  2020-11-30 22:22       ` Brendan Higgins
  1 sibling, 0 replies; 17+ messages in thread
From: Marco Elver @ 2020-11-24  8:02 UTC (permalink / raw)
  To: David Gow
  Cc: Brendan Higgins, Theodore Ts'o, Shuah Khan,
	Arpitha Raghunandan, Iurii Zaikin, Andreas Dilger, Bird, Tim,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List, linux-kernel-mentees, linux-ext4

On Tue, 24 Nov 2020 at 08:25, David Gow <davidgow@google.com> wrote:
>
> On Mon, Nov 23, 2020 at 9:08 PM Marco Elver <elver@google.com> wrote:
> >
> > On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote:
> > > On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also
> > > > optionally provide a human-readable description of parameters, which is
> > > > displayed where available.
> > > >
> > > > Note, currently the result of each parameter run is displayed in
> > > > diagnostic lines, and only the overall test case output summarizes
> > > > TAP-compliant success or failure of all parameter runs. In future, when
> > > > supported by kunit-tool, these can be turned into subsubtest outputs.
> > > >
> > > > 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>
> > > > ---
> > > [Resending this because my email client re-defaulted to HTML! Aarrgh!]
> > >
> > > This looks good to me! I tested it in UML and x86-64 w/ KASAN, and
> > > both worked fine.
> > >
> > > Reviewed-by: David Gow <davidgow@google.com>
> > > Tested-by: David Gow <davidgow@google.com>
> >
> > Thank you!
> >
> > > Thanks for sticking with this!
> >
> > Will these patches be landing in 5.11 or 5.12?
> >
>
> I can't think of any reason not to have these in 5.11. We haven't
> started staging things in the kselftest/kunit branch for 5.11 yet,
> though.
>
> Patch 2 will probably need to be acked by Ted for ext4 first.

Patch 2 had already had 1 Reviewed-by on v3 that got lost. The core
bits of that test haven't changed since then, but I can't tell if it
needs a re-review.

https://lkml.kernel.org/r/CAAXuY3o9Xe-atK0Mja6qXLncUhmmVf4pR7hsANsqaoUX71RXVg@mail.gmail.com

Thanks,
-- Marco

> Brendan, Shuah: can you make sure this doesn't get lost in patchwork?
>
> Cheers,
> -- David
>
> > > -- David
> >
> > Thanks,
> > -- Marco

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

* Re: [PATCH v9 1/2] kunit: Support for Parameterized Testing
  2020-11-24  7:25     ` David Gow
  2020-11-24  8:02       ` Marco Elver
@ 2020-11-30 22:22       ` Brendan Higgins
  2020-12-01 22:28         ` Shuah Khan
  2020-12-02 16:02         ` Theodore Y. Ts'o
  1 sibling, 2 replies; 17+ messages in thread
From: Brendan Higgins @ 2020-11-30 22:22 UTC (permalink / raw)
  To: David Gow
  Cc: Marco Elver, Theodore Ts'o, Shuah Khan, Arpitha Raghunandan,
	Iurii Zaikin, Andreas Dilger, Bird, Tim,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List, linux-kernel-mentees, linux-ext4

On Mon, Nov 23, 2020 at 11:25 PM David Gow <davidgow@google.com> wrote:
>
> On Mon, Nov 23, 2020 at 9:08 PM Marco Elver <elver@google.com> wrote:
> >
> > On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote:
> > > On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also
> > > > optionally provide a human-readable description of parameters, which is
> > > > displayed where available.
> > > >
> > > > Note, currently the result of each parameter run is displayed in
> > > > diagnostic lines, and only the overall test case output summarizes
> > > > TAP-compliant success or failure of all parameter runs. In future, when
> > > > supported by kunit-tool, these can be turned into subsubtest outputs.
> > > >
> > > > 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>
> > > > ---
> > > [Resending this because my email client re-defaulted to HTML! Aarrgh!]
> > >
> > > This looks good to me! I tested it in UML and x86-64 w/ KASAN, and
> > > both worked fine.
> > >
> > > Reviewed-by: David Gow <davidgow@google.com>
> > > Tested-by: David Gow <davidgow@google.com>
> >
> > Thank you!
> >
> > > Thanks for sticking with this!
> >
> > Will these patches be landing in 5.11 or 5.12?
> >
>
> I can't think of any reason not to have these in 5.11. We haven't
> started staging things in the kselftest/kunit branch for 5.11 yet,
> though.
>
> Patch 2 will probably need to be acked by Ted for ext4 first.
>
> Brendan, Shuah: can you make sure this doesn't get lost in patchwork?

Looks good to me. I would definitely like to pick this up. But yeah,
in order to pick up 2/2 we will need an ack from either Ted or Iurii.

Ted seems to be busy right now, so I think I will just ask Shuah to go
ahead and pick this patch up by itself and we or Ted can pick up patch
2/2 later.

Cheers

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

* Re: [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature
  2020-11-16  5:41 ` [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Arpitha Raghunandan
  2020-11-16  8:51   ` Marco Elver
  2020-11-17  7:22   ` David Gow
@ 2020-12-01 20:30   ` Iurii Zaikin
  2020-12-02 16:07   ` Theodore Y. Ts'o
  3 siblings, 0 replies; 17+ messages in thread
From: Iurii Zaikin @ 2020-12-01 20:30 UTC (permalink / raw)
  To: Arpitha Raghunandan
  Cc: Brendan Higgins, Shuah Khan, Marco Elver, Theodore Ts'o,
	Andreas Dilger, Bird, Timothy, David Gow,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List, linux-kernel-mentees,
	Ext4 Developers List

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

Reviewed-by: Iurii Zaikin <yzaikin@google.com>

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

* Re: [PATCH v9 1/2] kunit: Support for Parameterized Testing
  2020-11-30 22:22       ` Brendan Higgins
@ 2020-12-01 22:28         ` Shuah Khan
  2020-12-01 23:31           ` Marco Elver
  2020-12-02 16:02         ` Theodore Y. Ts'o
  1 sibling, 1 reply; 17+ messages in thread
From: Shuah Khan @ 2020-12-01 22:28 UTC (permalink / raw)
  To: Brendan Higgins, David Gow, Arpitha Raghunandan
  Cc: Marco Elver, Theodore Ts'o, Iurii Zaikin, Andreas Dilger,
	Bird, Tim, open list:KERNEL SELFTEST FRAMEWORK,
	KUnit Development, Linux Kernel Mailing List,
	linux-kernel-mentees, linux-ext4, skhan

On 11/30/20 3:22 PM, Brendan Higgins wrote:
> On Mon, Nov 23, 2020 at 11:25 PM David Gow <davidgow@google.com> wrote:
>>
>> On Mon, Nov 23, 2020 at 9:08 PM Marco Elver <elver@google.com> wrote:
>>>
>>> On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote:
>>>> On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also
>>>>> optionally provide a human-readable description of parameters, which is
>>>>> displayed where available.
>>>>>
>>>>> Note, currently the result of each parameter run is displayed in
>>>>> diagnostic lines, and only the overall test case output summarizes
>>>>> TAP-compliant success or failure of all parameter runs. In future, when
>>>>> supported by kunit-tool, these can be turned into subsubtest outputs.
>>>>>
>>>>> 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>
>>>>> ---
>>>> [Resending this because my email client re-defaulted to HTML! Aarrgh!]
>>>>
>>>> This looks good to me! I tested it in UML and x86-64 w/ KASAN, and
>>>> both worked fine.
>>>>
>>>> Reviewed-by: David Gow <davidgow@google.com>
>>>> Tested-by: David Gow <davidgow@google.com>
>>>
>>> Thank you!
>>>
>>>> Thanks for sticking with this!
>>>
>>> Will these patches be landing in 5.11 or 5.12?
>>>
>>
>> I can't think of any reason not to have these in 5.11. We haven't
>> started staging things in the kselftest/kunit branch for 5.11 yet,
>> though.
>>
>> Patch 2 will probably need to be acked by Ted for ext4 first.
>>
>> Brendan, Shuah: can you make sure this doesn't get lost in patchwork?
> 
> Looks good to me. I would definitely like to pick this up. But yeah,
> in order to pick up 2/2 we will need an ack from either Ted or Iurii.
> 
> Ted seems to be busy right now, so I think I will just ask Shuah to go
> ahead and pick this patch up by itself and we or Ted can pick up patch
> 2/2 later.
> 
> Cheers
> 

I am seeing

ERROR: need consistent spacing around '*' (ctx:WxV)
#272: FILE: include/kunit/test.h:1786:
+		typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 : 
(array);	\
  		                   ^

Can you look into this and send v10?

thanks,
-- Shuah

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

* Re: [PATCH v9 1/2] kunit: Support for Parameterized Testing
  2020-12-01 22:28         ` Shuah Khan
@ 2020-12-01 23:31           ` Marco Elver
  2020-12-02 22:58             ` Shuah Khan
  0 siblings, 1 reply; 17+ messages in thread
From: Marco Elver @ 2020-12-01 23:31 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Brendan Higgins, David Gow, Arpitha Raghunandan,
	Theodore Ts'o, Iurii Zaikin, Andreas Dilger, Bird, Tim,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List, linux-kernel-mentees, linux-ext4

On Tue, 1 Dec 2020 at 23:28, Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 11/30/20 3:22 PM, Brendan Higgins wrote:
> > On Mon, Nov 23, 2020 at 11:25 PM David Gow <davidgow@google.com> wrote:
> >>
> >> On Mon, Nov 23, 2020 at 9:08 PM Marco Elver <elver@google.com> wrote:
> >>>
> >>> On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote:
> >>>> On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also
> >>>>> optionally provide a human-readable description of parameters, which is
> >>>>> displayed where available.
> >>>>>
> >>>>> Note, currently the result of each parameter run is displayed in
> >>>>> diagnostic lines, and only the overall test case output summarizes
> >>>>> TAP-compliant success or failure of all parameter runs. In future, when
> >>>>> supported by kunit-tool, these can be turned into subsubtest outputs.
> >>>>>
> >>>>> 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>
> >>>>> ---
> >>>> [Resending this because my email client re-defaulted to HTML! Aarrgh!]
> >>>>
> >>>> This looks good to me! I tested it in UML and x86-64 w/ KASAN, and
> >>>> both worked fine.
> >>>>
> >>>> Reviewed-by: David Gow <davidgow@google.com>
> >>>> Tested-by: David Gow <davidgow@google.com>
> >>>
> >>> Thank you!
> >>>
> >>>> Thanks for sticking with this!
> >>>
> >>> Will these patches be landing in 5.11 or 5.12?
> >>>
> >>
> >> I can't think of any reason not to have these in 5.11. We haven't
> >> started staging things in the kselftest/kunit branch for 5.11 yet,
> >> though.
> >>
> >> Patch 2 will probably need to be acked by Ted for ext4 first.
> >>
> >> Brendan, Shuah: can you make sure this doesn't get lost in patchwork?
> >
> > Looks good to me. I would definitely like to pick this up. But yeah,
> > in order to pick up 2/2 we will need an ack from either Ted or Iurii.
> >
> > Ted seems to be busy right now, so I think I will just ask Shuah to go
> > ahead and pick this patch up by itself and we or Ted can pick up patch
> > 2/2 later.
> >
> > Cheers
> >
>
> I am seeing
>
> ERROR: need consistent spacing around '*' (ctx:WxV)
> #272: FILE: include/kunit/test.h:1786:
> +               typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 :
> (array);        \
>                                    ^
>
> Can you look into this and send v10?

This is a false positive. I pointed this out here before:
https://lkml.kernel.org/r/CANpmjNNhpe6TYt0KmBCCR-Wfz1Bxd8qnhiwegwnDQsxRAWmUMg@mail.gmail.com

checkpatch.pl thinks this is a multiplication, but this is a pointer,
so the spacing here is correct.

Thanks,
-- Marco

> thanks,
> -- Shuah

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

* Re: [PATCH v9 1/2] kunit: Support for Parameterized Testing
  2020-11-30 22:22       ` Brendan Higgins
  2020-12-01 22:28         ` Shuah Khan
@ 2020-12-02 16:02         ` Theodore Y. Ts'o
  1 sibling, 0 replies; 17+ messages in thread
From: Theodore Y. Ts'o @ 2020-12-02 16:02 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: David Gow, Marco Elver, Shuah Khan, Arpitha Raghunandan,
	Iurii Zaikin, Andreas Dilger, Bird, Tim,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List, linux-kernel-mentees, linux-ext4

On Mon, Nov 30, 2020 at 02:22:22PM -0800, 'Brendan Higgins' via KUnit Development wrote:
> 
> Looks good to me. I would definitely like to pick this up. But yeah,
> in order to pick up 2/2 we will need an ack from either Ted or Iurii.
> 
> Ted seems to be busy right now, so I think I will just ask Shuah to go
> ahead and pick this patch up by itself and we or Ted can pick up patch
> 2/2 later.

I have been paying attention to this patch series, but I had presumed
that this was much more of a kunit change than an ext4 change, and the
critical bits was a review of the kunit infrastructure.  I certainly
have no objection to changing the ext4 test to use the new
parameterized testing, and if you'd like me to give a quick review,
I'll take a quick look.  I assume, Brendan, that you've already tried
doing a compile and run test of the patch series, so I'm not going to
do that?

						- Ted

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

* Re: [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature
  2020-11-16  5:41 ` [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Arpitha Raghunandan
                     ` (2 preceding siblings ...)
  2020-12-01 20:30   ` Iurii Zaikin
@ 2020-12-02 16:07   ` Theodore Y. Ts'o
  2020-12-02 22:59     ` Shuah Khan
  3 siblings, 1 reply; 17+ messages in thread
From: Theodore Y. Ts'o @ 2020-12-02 16:07 UTC (permalink / raw)
  To: Arpitha Raghunandan
  Cc: brendanhiggins, skhan, elver, yzaikin, adilger.kernel, Tim.Bird,
	davidgow, linux-kselftest, kunit-dev, linux-kernel,
	linux-kernel-mentees, linux-ext4

On Mon, Nov 16, 2020 at 11:11:50AM +0530, Arpitha Raghunandan wrote:
> Modify fs/ext4/inode-test.c to use the parameterized testing
> feature of KUnit.
> 
> Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com>
> Signed-off-by: Marco Elver <elver@google.com>

Acked-by: Theodore Ts'o <tytso@mit.edu>


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

* Re: [PATCH v9 1/2] kunit: Support for Parameterized Testing
  2020-12-01 23:31           ` Marco Elver
@ 2020-12-02 22:58             ` Shuah Khan
  0 siblings, 0 replies; 17+ messages in thread
From: Shuah Khan @ 2020-12-02 22:58 UTC (permalink / raw)
  To: Marco Elver
  Cc: Brendan Higgins, David Gow, Arpitha Raghunandan,
	Theodore Ts'o, Iurii Zaikin, Andreas Dilger, Bird, Tim,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List, linux-kernel-mentees, linux-ext4,
	Shuah Khan

On 12/1/20 4:31 PM, Marco Elver wrote:
> On Tue, 1 Dec 2020 at 23:28, Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 11/30/20 3:22 PM, Brendan Higgins wrote:
>>> On Mon, Nov 23, 2020 at 11:25 PM David Gow <davidgow@google.com> wrote:
>>>>
>>>> On Mon, Nov 23, 2020 at 9:08 PM Marco Elver <elver@google.com> wrote:
>>>>>
>>>>> On Tue, 17 Nov 2020 at 08:21, David Gow <davidgow@google.com> wrote:
>>>>>> On Mon, Nov 16, 2020 at 1:41 PM 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 based on arrays. Generators may also
>>>>>>> optionally provide a human-readable description of parameters, which is
>>>>>>> displayed where available.
>>>>>>>
>>>>>>> Note, currently the result of each parameter run is displayed in
>>>>>>> diagnostic lines, and only the overall test case output summarizes
>>>>>>> TAP-compliant success or failure of all parameter runs. In future, when
>>>>>>> supported by kunit-tool, these can be turned into subsubtest outputs.
>>>>>>>
>>>>>>> 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>
>>>>>>> ---
>>>>>> [Resending this because my email client re-defaulted to HTML! Aarrgh!]
>>>>>>
>>>>>> This looks good to me! I tested it in UML and x86-64 w/ KASAN, and
>>>>>> both worked fine.
>>>>>>
>>>>>> Reviewed-by: David Gow <davidgow@google.com>
>>>>>> Tested-by: David Gow <davidgow@google.com>
>>>>>
>>>>> Thank you!
>>>>>
>>>>>> Thanks for sticking with this!
>>>>>
>>>>> Will these patches be landing in 5.11 or 5.12?
>>>>>
>>>>
>>>> I can't think of any reason not to have these in 5.11. We haven't
>>>> started staging things in the kselftest/kunit branch for 5.11 yet,
>>>> though.
>>>>
>>>> Patch 2 will probably need to be acked by Ted for ext4 first.
>>>>
>>>> Brendan, Shuah: can you make sure this doesn't get lost in patchwork?
>>>
>>> Looks good to me. I would definitely like to pick this up. But yeah,
>>> in order to pick up 2/2 we will need an ack from either Ted or Iurii.
>>>
>>> Ted seems to be busy right now, so I think I will just ask Shuah to go
>>> ahead and pick this patch up by itself and we or Ted can pick up patch
>>> 2/2 later.
>>>
>>> Cheers
>>>
>>
>> I am seeing
>>
>> ERROR: need consistent spacing around '*' (ctx:WxV)
>> #272: FILE: include/kunit/test.h:1786:
>> +               typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 :
>> (array);        \
>>                                     ^
>>
>> Can you look into this and send v10?
> 
> This is a false positive. I pointed this out here before:
> https://lkml.kernel.org/r/CANpmjNNhpe6TYt0KmBCCR-Wfz1Bxd8qnhiwegwnDQsxRAWmUMg@mail.gmail.com
> 
> checkpatch.pl thinks this is a multiplication, but this is a pointer,
> so the spacing here is correct.
> 

Thank you for confirming. I will apply this.

thanks,
-- Shuah

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

* Re: [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature
  2020-12-02 16:07   ` Theodore Y. Ts'o
@ 2020-12-02 22:59     ` Shuah Khan
  0 siblings, 0 replies; 17+ messages in thread
From: Shuah Khan @ 2020-12-02 22:59 UTC (permalink / raw)
  To: Theodore Y. Ts'o, Arpitha Raghunandan
  Cc: brendanhiggins, elver, yzaikin, adilger.kernel, Tim.Bird,
	davidgow, linux-kselftest, kunit-dev, linux-kernel,
	linux-kernel-mentees, linux-ext4, Shuah Khan

On 12/2/20 9:07 AM, Theodore Y. Ts'o wrote:
> On Mon, Nov 16, 2020 at 11:11:50AM +0530, Arpitha Raghunandan wrote:
>> Modify fs/ext4/inode-test.c to use the parameterized testing
>> feature of KUnit.
>>
>> Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com>
>> Signed-off-by: Marco Elver <elver@google.com>
> 
> Acked-by: Theodore Ts'o <tytso@mit.edu>
> 

Thanks Ted.

-- Shuah

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

end of thread, other threads:[~2020-12-02 22:59 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-16  5:40 [PATCH v9 1/2] kunit: Support for Parameterized Testing Arpitha Raghunandan
2020-11-16  5:41 ` [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature Arpitha Raghunandan
2020-11-16  8:51   ` Marco Elver
2020-11-17  7:22   ` David Gow
2020-12-01 20:30   ` Iurii Zaikin
2020-12-02 16:07   ` Theodore Y. Ts'o
2020-12-02 22:59     ` Shuah Khan
2020-11-16  8:53 ` [PATCH v9 1/2] kunit: Support for Parameterized Testing Marco Elver
2020-11-17  7:20 ` David Gow
2020-11-23 13:08   ` Marco Elver
2020-11-24  7:25     ` David Gow
2020-11-24  8:02       ` Marco Elver
2020-11-30 22:22       ` Brendan Higgins
2020-12-01 22:28         ` Shuah Khan
2020-12-01 23:31           ` Marco Elver
2020-12-02 22:58             ` Shuah Khan
2020-12-02 16:02         ` Theodore Y. Ts'o

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