[3/5] kselftest: run tests by fixture
diff mbox series

Message ID 20200313031752.2332565-4-kuba@kernel.org
State New
Headers show
Series
  • kselftest: add fixture parameters
Related show

Commit Message

Jakub Kicinski March 13, 2020, 3:17 a.m. UTC
Now that all tests have a fixture object move from a global
list of tests to a list of tests per fixture.

Order of tests may change as we will now group and run test
fixture by fixture, rather than in declaration order.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/kselftest_harness.h | 42 ++++++++++++---------
 1 file changed, 25 insertions(+), 17 deletions(-)

Comments

Kees Cook March 13, 2020, 11:27 p.m. UTC | #1
On Thu, Mar 12, 2020 at 08:17:50PM -0700, Jakub Kicinski wrote:
> Now that all tests have a fixture object move from a global
> list of tests to a list of tests per fixture.
> 
> Order of tests may change as we will now group and run test
> fixture by fixture, rather than in declaration order.

I'm not convinced about this change. Declaration order is a pretty
intuitive result that I'd like to keep for the harness.

Can this change be avoided and still keep the final results of a
"mutable" fixture?

-Kees

> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  tools/testing/selftests/kselftest_harness.h | 42 ++++++++++++---------
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
> index a396afe4a579..7a3392941a5b 100644
> --- a/tools/testing/selftests/kselftest_harness.h
> +++ b/tools/testing/selftests/kselftest_harness.h
> @@ -637,8 +637,11 @@
>  } while (0); OPTIONAL_HANDLER(_assert)
>  
>  /* Contains all the information about a fixture */
> +struct __test_metadata;
> +
>  struct __fixture_metadata {
>  	const char *name;
> +	struct __test_metadata *tests;
>  	struct __fixture_metadata *prev, *next;
>  } _fixture_global __attribute__((unused)) = {
>  	.name = "global",
> @@ -684,7 +687,6 @@ struct __test_metadata {
>  };
>  
>  /* Storage for the (global) tests to be run. */
> -static struct __test_metadata *__test_list;
>  static unsigned int __test_count;
>  
>  /*
> @@ -698,24 +700,26 @@ static unsigned int __test_count;
>   */
>  static inline void __register_test(struct __test_metadata *t)
>  {
> +	struct __fixture_metadata *f = t->fixture;
> +
>  	__test_count++;
>  	/* Circular linked list where only prev is circular. */
> -	if (__test_list == NULL) {
> -		__test_list = t;
> +	if (f->tests == NULL) {
> +		f->tests = t;
>  		t->next = NULL;
>  		t->prev = t;
>  		return;
>  	}
>  	if (__constructor_order == _CONSTRUCTOR_ORDER_FORWARD) {
>  		t->next = NULL;
> -		t->prev = __test_list->prev;
> +		t->prev = f->tests->prev;
>  		t->prev->next = t;
> -		__test_list->prev = t;
> +		f->tests->prev = t;
>  	} else {
> -		t->next = __test_list;
> +		t->next = f->tests;
>  		t->next->prev = t;
>  		t->prev = t;
> -		__test_list = t;
> +		f->tests = t;
>  	}
>  }
>  
> @@ -729,14 +733,15 @@ static inline int __bail(int for_realz, bool no_print, __u8 step)
>  	return 0;
>  }
>  
> -void __run_test(struct __test_metadata *t)
> +void __run_test(struct __fixture_metadata *f,
> +		struct __test_metadata *t)
>  {
>  	pid_t child_pid;
>  	int status;
>  
>  	t->passed = 1;
>  	t->trigger = 0;
> -	printf("[ RUN      ] %s.%s\n", t->fixture->name, t->name);
> +	printf("[ RUN      ] %s.%s\n", f->name, t->name);
>  	alarm(t->timeout);
>  	child_pid = fork();
>  	if (child_pid < 0) {
> @@ -786,13 +791,14 @@ void __run_test(struct __test_metadata *t)
>  		}
>  	}
>  	printf("[     %4s ] %s.%s\n", (t->passed ? "OK" : "FAIL"),
> -	       t->fixture->name, t->name);
> +	       f->name, t->name);
>  	alarm(0);
>  }
>  
>  static int test_harness_run(int __attribute__((unused)) argc,
>  			    char __attribute__((unused)) **argv)
>  {
> +	struct __fixture_metadata *f;
>  	struct __test_metadata *t;
>  	int ret = 0;
>  	unsigned int count = 0;
> @@ -801,13 +807,15 @@ static int test_harness_run(int __attribute__((unused)) argc,
>  	/* TODO(wad) add optional arguments similar to gtest. */
>  	printf("[==========] Running %u tests from %u test cases.\n",
>  	       __test_count, __fixture_count + 1);
> -	for (t = __test_list; t; t = t->next) {
> -		count++;
> -		__run_test(t);
> -		if (t->passed)
> -			pass_count++;
> -		else
> -			ret = 1;
> +	for (f = __fixture_list; f; f = f->next) {
> +		for (t = f->tests; t; t = t->next) {
> +			count++;
> +			__run_test(f, t);
> +			if (t->passed)
> +				pass_count++;
> +			else
> +				ret = 1;
> +		}
>  	}
>  	printf("[==========] %u / %u tests passed.\n", pass_count, count);
>  	printf("[  %s  ]\n", (ret ? "FAILED" : "PASSED"));
> -- 
> 2.24.1
>
Jakub Kicinski March 13, 2020, 11:54 p.m. UTC | #2
On Fri, 13 Mar 2020 16:27:54 -0700 Kees Cook wrote:
> On Thu, Mar 12, 2020 at 08:17:50PM -0700, Jakub Kicinski wrote:
> > Now that all tests have a fixture object move from a global
> > list of tests to a list of tests per fixture.
> > 
> > Order of tests may change as we will now group and run test
> > fixture by fixture, rather than in declaration order.  
> 
> I'm not convinced about this change. Declaration order is a pretty
> intuitive result that I'd like to keep for the harness.
> 
> Can this change be avoided and still keep the final results of a
> "mutable" fixture?

Sure! I will abandon the reorg of the lists then, keep just the list of
tests, and have each test point to its fixture. Which then may contain
param sets.

Patch
diff mbox series

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index a396afe4a579..7a3392941a5b 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -637,8 +637,11 @@ 
 } while (0); OPTIONAL_HANDLER(_assert)
 
 /* Contains all the information about a fixture */
+struct __test_metadata;
+
 struct __fixture_metadata {
 	const char *name;
+	struct __test_metadata *tests;
 	struct __fixture_metadata *prev, *next;
 } _fixture_global __attribute__((unused)) = {
 	.name = "global",
@@ -684,7 +687,6 @@  struct __test_metadata {
 };
 
 /* Storage for the (global) tests to be run. */
-static struct __test_metadata *__test_list;
 static unsigned int __test_count;
 
 /*
@@ -698,24 +700,26 @@  static unsigned int __test_count;
  */
 static inline void __register_test(struct __test_metadata *t)
 {
+	struct __fixture_metadata *f = t->fixture;
+
 	__test_count++;
 	/* Circular linked list where only prev is circular. */
-	if (__test_list == NULL) {
-		__test_list = t;
+	if (f->tests == NULL) {
+		f->tests = t;
 		t->next = NULL;
 		t->prev = t;
 		return;
 	}
 	if (__constructor_order == _CONSTRUCTOR_ORDER_FORWARD) {
 		t->next = NULL;
-		t->prev = __test_list->prev;
+		t->prev = f->tests->prev;
 		t->prev->next = t;
-		__test_list->prev = t;
+		f->tests->prev = t;
 	} else {
-		t->next = __test_list;
+		t->next = f->tests;
 		t->next->prev = t;
 		t->prev = t;
-		__test_list = t;
+		f->tests = t;
 	}
 }
 
@@ -729,14 +733,15 @@  static inline int __bail(int for_realz, bool no_print, __u8 step)
 	return 0;
 }
 
-void __run_test(struct __test_metadata *t)
+void __run_test(struct __fixture_metadata *f,
+		struct __test_metadata *t)
 {
 	pid_t child_pid;
 	int status;
 
 	t->passed = 1;
 	t->trigger = 0;
-	printf("[ RUN      ] %s.%s\n", t->fixture->name, t->name);
+	printf("[ RUN      ] %s.%s\n", f->name, t->name);
 	alarm(t->timeout);
 	child_pid = fork();
 	if (child_pid < 0) {
@@ -786,13 +791,14 @@  void __run_test(struct __test_metadata *t)
 		}
 	}
 	printf("[     %4s ] %s.%s\n", (t->passed ? "OK" : "FAIL"),
-	       t->fixture->name, t->name);
+	       f->name, t->name);
 	alarm(0);
 }
 
 static int test_harness_run(int __attribute__((unused)) argc,
 			    char __attribute__((unused)) **argv)
 {
+	struct __fixture_metadata *f;
 	struct __test_metadata *t;
 	int ret = 0;
 	unsigned int count = 0;
@@ -801,13 +807,15 @@  static int test_harness_run(int __attribute__((unused)) argc,
 	/* TODO(wad) add optional arguments similar to gtest. */
 	printf("[==========] Running %u tests from %u test cases.\n",
 	       __test_count, __fixture_count + 1);
-	for (t = __test_list; t; t = t->next) {
-		count++;
-		__run_test(t);
-		if (t->passed)
-			pass_count++;
-		else
-			ret = 1;
+	for (f = __fixture_list; f; f = f->next) {
+		for (t = f->tests; t; t = t->next) {
+			count++;
+			__run_test(f, t);
+			if (t->passed)
+				pass_count++;
+			else
+				ret = 1;
+		}
 	}
 	printf("[==========] %u / %u tests passed.\n", pass_count, count);
 	printf("[  %s  ]\n", (ret ? "FAILED" : "PASSED"));