selftests: Add support for argc and argv.
diff mbox series

Message ID 20200304085204.48118-1-kuniyu@amazon.co.jp
State New
Headers show
Series
  • selftests: Add support for argc and argv.
Related show

Commit Message

Kuniyuki Iwashima March 4, 2020, 8:52 a.m. UTC
Currently tests are often written in C and shell script. In many cases, the
script passes some arguments to the C program. However, the helper
functions do not support arguments, so many tests are written without
helper functions.

This patch allows us to handle argc and argv in each tests and makes it
easier to write tests flexibly with helper functions.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp>
---
 tools/testing/selftests/kselftest_harness.h | 30 ++++++++++++---------
 1 file changed, 18 insertions(+), 12 deletions(-)

Comments

Kees Cook March 4, 2020, 6:17 p.m. UTC | #1
On Wed, Mar 04, 2020 at 05:52:04PM +0900, Kuniyuki Iwashima wrote:
> Currently tests are often written in C and shell script. In many cases, the
> script passes some arguments to the C program. However, the helper
> functions do not support arguments, so many tests are written without
> helper functions.
> 
> This patch allows us to handle argc and argv in each tests and makes it
> easier to write tests flexibly with helper functions.
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp>

Interesting! Do you have an example that uses this? I wonder if it might
make sense instead to allow extending the struct __test_metadata with
test-specific options so that individual tests don't have to re-parse
argv every time (the main test running could instead do it once and set
variables in struct __test_metadata (or somewhere else).

-Kees

> ---
>  tools/testing/selftests/kselftest_harness.h | 30 ++++++++++++---------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
> index 5336b26506ab..75bee67b87fa 100644
> --- a/tools/testing/selftests/kselftest_harness.h
> +++ b/tools/testing/selftests/kselftest_harness.h
> @@ -167,7 +167,8 @@
>  #define TEST_SIGNAL(test_name, signal) __TEST_IMPL(test_name, signal)
>  
>  #define __TEST_IMPL(test_name, _signal) \
> -	static void test_name(struct __test_metadata *_metadata); \
> +	static void test_name(struct __test_metadata *_metadata, \
> +		int argc, char **argv); \
>  	static struct __test_metadata _##test_name##_object = \
>  		{ .name = "global." #test_name, \
>  		  .fn = &test_name, .termsig = _signal, \
> @@ -177,7 +178,9 @@
>  		__register_test(&_##test_name##_object); \
>  	} \
>  	static void test_name( \
> -		struct __test_metadata __attribute__((unused)) *_metadata)
> +		struct __test_metadata __attribute__((unused)) *_metadata, \
> +		int __attribute__((unused)) argc, \
> +		char __attribute__((unused)) **argv)
>  
>  /**
>   * FIXTURE_DATA(datatype_name) - Wraps the struct name so we have one less
> @@ -293,9 +296,11 @@
>  #define __TEST_F_IMPL(fixture_name, test_name, signal, tmout) \
>  	static void fixture_name##_##test_name( \
>  		struct __test_metadata *_metadata, \
> -		FIXTURE_DATA(fixture_name) *self); \
> +		FIXTURE_DATA(fixture_name) *self, \
> +		int argc, char **argv); \
>  	static inline void wrapper_##fixture_name##_##test_name( \
> -		struct __test_metadata *_metadata) \
> +		struct __test_metadata *_metadata, \
> +		int argc, char **argv) \
>  	{ \
>  		/* fixture data is alloced, setup, and torn down per call. */ \
>  		FIXTURE_DATA(fixture_name) self; \
> @@ -304,7 +309,7 @@
>  		/* Let setup failure terminate early. */ \
>  		if (!_metadata->passed) \
>  			return; \
> -		fixture_name##_##test_name(_metadata, &self); \
> +		fixture_name##_##test_name(_metadata, &self, argc, argv); \
>  		fixture_name##_teardown(_metadata, &self); \
>  	} \
>  	static struct __test_metadata \
> @@ -321,7 +326,9 @@
>  	} \
>  	static void fixture_name##_##test_name( \
>  		struct __test_metadata __attribute__((unused)) *_metadata, \
> -		FIXTURE_DATA(fixture_name) __attribute__((unused)) *self)
> +		FIXTURE_DATA(fixture_name) __attribute__((unused)) *self, \
> +		int __attribute__ ((unused)) argc, \
> +		char __attribute__ ((unused)) **argv)
>  
>  /**
>   * TEST_HARNESS_MAIN - Simple wrapper to run the test harness
> @@ -634,7 +641,7 @@
>  /* Contains all the information for test execution and status checking. */
>  struct __test_metadata {
>  	const char *name;
> -	void (*fn)(struct __test_metadata *);
> +	void (*fn)(struct __test_metadata *, int, char **);
>  	int termsig;
>  	int passed;
>  	int trigger; /* extra handler after the evaluation */
> @@ -695,7 +702,7 @@ 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 __test_metadata *t, int argc, char **argv)
>  {
>  	pid_t child_pid;
>  	int status;
> @@ -709,7 +716,7 @@ void __run_test(struct __test_metadata *t)
>  		printf("ERROR SPAWNING TEST CHILD\n");
>  		t->passed = 0;
>  	} else if (child_pid == 0) {
> -		t->fn(t);
> +		t->fn(t, argc, argv);
>  		/* return the step that failed or 0 */
>  		_exit(t->passed ? 0 : t->step);
>  	} else {
> @@ -755,8 +762,7 @@ void __run_test(struct __test_metadata *t)
>  	alarm(0);
>  }
>  
> -static int test_harness_run(int __attribute__((unused)) argc,
> -			    char __attribute__((unused)) **argv)
> +static int test_harness_run(int argc, char **argv)
>  {
>  	struct __test_metadata *t;
>  	int ret = 0;
> @@ -768,7 +774,7 @@ static int test_harness_run(int __attribute__((unused)) argc,
>  	       __test_count, __fixture_count + 1);
>  	for (t = __test_list; t; t = t->next) {
>  		count++;
> -		__run_test(t);
> +		__run_test(t, argc, argv);
>  		if (t->passed)
>  			pass_count++;
>  		else
> -- 
> 2.17.2 (Apple Git-113)
>
Kuniyuki Iwashima March 5, 2020, 10:57 a.m. UTC | #2
From:   Kees Cook <keescook@chromium.org>
Date:   Wed, 4 Mar 2020 10:17:41 -0800
> On Wed, Mar 04, 2020 at 05:52:04PM +0900, Kuniyuki Iwashima wrote:
> > Currently tests are often written in C and shell script. In many cases, the
> > script passes some arguments to the C program. However, the helper
> > functions do not support arguments, so many tests are written without
> > helper functions.
> > 
> > This patch allows us to handle argc and argv in each tests and makes it
> > easier to write tests flexibly with helper functions.
> > 
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp>
> 
> Interesting! Do you have an example that uses this? I wonder if it might
> make sense instead to allow extending the struct __test_metadata with
> test-specific options so that individual tests don't have to re-parse
> argv every time (the main test running could instead do it once and set
> variables in struct __test_metadata (or somewhere else).

I added a sample test program at the end of this mail.

There are some functions that are not TEST() but are passed __test_metadata to
in order to use ASSERT_EQ in the function. I did not extend __test_metadata
because I thought argc and argv would not be used in such functions.

  e.g. kill_thread_or_group() in tools/testing/selftests/seccomp/seccomp_bpf.c

But, I have not thought about re-parsing, thank you!
Also I thought up that it is better to pass argc and argv to
FIXTURE_SETUP/TEARDOWN.

Now I have two idea.

  1. pass argc and argv to FIXTURE_SETUP/TEARDOWN.
  2. define COMMON_FIXTURE and COMMON_FIXTURE_SETUP/TEARDOWN,
       and pass COMMON_FIXTURE to all tests.
       (I think it is not good to extend __test_metadata because argc and
        argv is not metadata, so it is good to setup another vars with args)

I think each has pros and cons.

1.
Pros
  - shell script only has to call a C program once with some arguments and
      each FIXTURE_SETUP differs from one another
  - shell script can call the same C program with different arguments and
      each FIXTURE_SETUP differs from one another
Cons:
  - if TEST()s use the same FIXTURE, the same FIXTURE_SETUP is called in each TEST()s.

2.
Pros:
  - we do not have to re-parse argc and argv in each TEST()s.
Cons:
  - 1. may give more flexibility than 2.

Which would you think is better?
I would be happy if you tell me another idea!

Thanks.


===sample===
#include "./kselftest_harness.h"


TEST(argc_test)
{
	int i;
	for (i = 0; i < argc; i++)
		TH_LOG("argv[%d]: %s", i, argv[i]);
}

FIXTURE(argc_f) {
	int data;
};

FIXTURE_SETUP(argc_f) {
	self->data = 92;
	ASSERT_EQ(92, self->data);
}

FIXTURE_TEARDOWN(argc_f) {
}

TEST_F(argc_f, argc_test_f) {
	int i;
	for (i = 0; i < argc; i++)
		TH_LOG("fixture: %d\targv[%d]: %s", self->data, i, argv[i]);
}

TEST_HARNESS_MAIN
============

Patch
diff mbox series

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 5336b26506ab..75bee67b87fa 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -167,7 +167,8 @@ 
 #define TEST_SIGNAL(test_name, signal) __TEST_IMPL(test_name, signal)
 
 #define __TEST_IMPL(test_name, _signal) \
-	static void test_name(struct __test_metadata *_metadata); \
+	static void test_name(struct __test_metadata *_metadata, \
+		int argc, char **argv); \
 	static struct __test_metadata _##test_name##_object = \
 		{ .name = "global." #test_name, \
 		  .fn = &test_name, .termsig = _signal, \
@@ -177,7 +178,9 @@ 
 		__register_test(&_##test_name##_object); \
 	} \
 	static void test_name( \
-		struct __test_metadata __attribute__((unused)) *_metadata)
+		struct __test_metadata __attribute__((unused)) *_metadata, \
+		int __attribute__((unused)) argc, \
+		char __attribute__((unused)) **argv)
 
 /**
  * FIXTURE_DATA(datatype_name) - Wraps the struct name so we have one less
@@ -293,9 +296,11 @@ 
 #define __TEST_F_IMPL(fixture_name, test_name, signal, tmout) \
 	static void fixture_name##_##test_name( \
 		struct __test_metadata *_metadata, \
-		FIXTURE_DATA(fixture_name) *self); \
+		FIXTURE_DATA(fixture_name) *self, \
+		int argc, char **argv); \
 	static inline void wrapper_##fixture_name##_##test_name( \
-		struct __test_metadata *_metadata) \
+		struct __test_metadata *_metadata, \
+		int argc, char **argv) \
 	{ \
 		/* fixture data is alloced, setup, and torn down per call. */ \
 		FIXTURE_DATA(fixture_name) self; \
@@ -304,7 +309,7 @@ 
 		/* Let setup failure terminate early. */ \
 		if (!_metadata->passed) \
 			return; \
-		fixture_name##_##test_name(_metadata, &self); \
+		fixture_name##_##test_name(_metadata, &self, argc, argv); \
 		fixture_name##_teardown(_metadata, &self); \
 	} \
 	static struct __test_metadata \
@@ -321,7 +326,9 @@ 
 	} \
 	static void fixture_name##_##test_name( \
 		struct __test_metadata __attribute__((unused)) *_metadata, \
-		FIXTURE_DATA(fixture_name) __attribute__((unused)) *self)
+		FIXTURE_DATA(fixture_name) __attribute__((unused)) *self, \
+		int __attribute__ ((unused)) argc, \
+		char __attribute__ ((unused)) **argv)
 
 /**
  * TEST_HARNESS_MAIN - Simple wrapper to run the test harness
@@ -634,7 +641,7 @@ 
 /* Contains all the information for test execution and status checking. */
 struct __test_metadata {
 	const char *name;
-	void (*fn)(struct __test_metadata *);
+	void (*fn)(struct __test_metadata *, int, char **);
 	int termsig;
 	int passed;
 	int trigger; /* extra handler after the evaluation */
@@ -695,7 +702,7 @@  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 __test_metadata *t, int argc, char **argv)
 {
 	pid_t child_pid;
 	int status;
@@ -709,7 +716,7 @@  void __run_test(struct __test_metadata *t)
 		printf("ERROR SPAWNING TEST CHILD\n");
 		t->passed = 0;
 	} else if (child_pid == 0) {
-		t->fn(t);
+		t->fn(t, argc, argv);
 		/* return the step that failed or 0 */
 		_exit(t->passed ? 0 : t->step);
 	} else {
@@ -755,8 +762,7 @@  void __run_test(struct __test_metadata *t)
 	alarm(0);
 }
 
-static int test_harness_run(int __attribute__((unused)) argc,
-			    char __attribute__((unused)) **argv)
+static int test_harness_run(int argc, char **argv)
 {
 	struct __test_metadata *t;
 	int ret = 0;
@@ -768,7 +774,7 @@  static int test_harness_run(int __attribute__((unused)) argc,
 	       __test_count, __fixture_count + 1);
 	for (t = __test_list; t; t = t->next) {
 		count++;
-		__run_test(t);
+		__run_test(t, argc, argv);
 		if (t->passed)
 			pass_count++;
 		else