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:
> On Fri, Jul 26, 2019 at 2:25 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > 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 @@
> 
> [...]
> 
> > >
> > >  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
> 
> Special request from Alexei :) But in one of the follow up patches, I
> extended this to allow to specify arbitrary subset of tests, e.g.:
> 1,2,5-10,7-8. So in that regard, it's more powerful than selecting by
> name and gives you ultimate freedom.
I guess I didn't read the series close enough; that '1,2,3' mode does seem
quite useful indeed!

> 
> > 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?
> 
> Both were simple to support, I didn't want to dictate one right way to
> do this :)
> 
> >
> > > +     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
> > >