Re: [PATCH bpf-next 3/9] selftests/bpf: add test selectors by number and name to test_progs

[Stanislav Fomichev <sdf@fomichev.me>]

On 07/26, Andrii Nakryiko wrote:
> Add ability to specify either test number or test name substring to
> narrow down a set of test to run.
> 
> Usage:
> sudo ./test_progs -n 1
> sudo ./test_progs -t attach_probe
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  tools/testing/selftests/bpf/test_progs.c | 43 +++++++++++++++++++++---
>  1 file changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index eea88ba59225..6e04b9f83777 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -4,6 +4,7 @@
>  #include "test_progs.h"
>  #include "bpf_rlimit.h"
>  #include <argp.h>
> +#include <string.h>
>  
>  int error_cnt, pass_cnt;
>  bool jit_enabled;
> @@ -164,6 +165,7 @@ void *spin_lock_thread(void *arg)
>  
>  struct prog_test_def {
>  	const char *test_name;
> +	int test_num;
>  	void (*run_test)(void);
>  };
>  
> @@ -181,26 +183,49 @@ const char *argp_program_bug_address = "<bpf@vger.kernel.org>";
>  const char argp_program_doc[] = "BPF selftests test runner";
>  
>  enum ARG_KEYS {
> +	ARG_TEST_NUM = 'n',
> +	ARG_TEST_NAME = 't',
>  	ARG_VERIFIER_STATS = 's',
>  };
>  	
>  static const struct argp_option opts[] = {
> +	{ "num", ARG_TEST_NUM, "NUM", 0,
> +	  "Run test number NUM only " },
> +	{ "name", ARG_TEST_NAME, "NAME", 0,
> +	  "Run tests with names containing NAME" },
>  	{ "verifier-stats", ARG_VERIFIER_STATS, NULL, 0,
>  	  "Output verifier statistics", },
>  	{},
>  };
>  
>  struct test_env {
> +	int test_num_selector;
> +	const char *test_name_selector;
>  	bool verifier_stats;
>  };
>  
> -static struct test_env env = {};
> +static struct test_env env = {
> +	.test_num_selector = -1,
> +};
>  
>  static error_t parse_arg(int key, char *arg, struct argp_state *state)
>  {
>  	struct test_env *env = state->input;
>  
>  	switch (key) {
[..]
> +	case ARG_TEST_NUM: {
> +		int test_num;
> +
> +		errno = 0;
> +		test_num = strtol(arg, NULL, 10);
> +		if (errno)
> +			return -errno;
> +		env->test_num_selector = test_num;
> +		break;
> +	}
Do you think it's really useful? I agree about running by name (I
usually used grep -v in the Makefile :-), but I'm not sure about running
by number.

Or is the idea is that you can just copy-paste this number from the
test_progs output to rerun the tests? In this case, why not copy-paste
the name instead?

> +	case ARG_TEST_NAME:
> +		env->test_name_selector = arg;
> +		break;
>  	case ARG_VERIFIER_STATS:
>  		env->verifier_stats = true;
>  		break;
> @@ -223,7 +248,7 @@ int main(int argc, char **argv)
>  		.parser = parse_arg,
>  		.doc = argp_program_doc,
>  	};
> -	const struct prog_test_def *def;
> +	struct prog_test_def *test;
>  	int err, i;
>  
>  	err = argp_parse(&argp, argc, argv, 0, NULL, &env);
> @@ -237,8 +262,18 @@ int main(int argc, char **argv)
>  	verifier_stats = env.verifier_stats;
>  
>  	for (i = 0; i < ARRAY_SIZE(prog_test_defs); i++) {
> -		def = &prog_test_defs[i];
> -		def->run_test();
> +		test = &prog_test_defs[i];
> +
> +		test->test_num = i + 1;
> +
> +		if (env.test_num_selector >= 0 &&
> +		    test->test_num != env.test_num_selector)
> +			continue;
> +		if (env.test_name_selector &&
> +		    !strstr(test->test_name, env.test_name_selector))
> +			continue;
> +
> +		test->run_test();
>  	}
>  
>  	printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
> -- 
> 2.17.1
>