[PATCH bpf] selftests/bpf: Fix test_progs-flavor run getting number of tests

[PATCH bpf] selftests/bpf: Fix test_progs-flavor run getting number of tests

[Jesper Dangaard Brouer]

Commit 643e7233aa94 ("selftests/bpf: Test_progs option for getting number of
tests") introduced ability to getting number of tests, which is targeted
towards scripting.  As demonstrate in the commit the number can be use as a
shell variable for further scripting.

The test_progs program support "flavor", which is detected by the binary
have a "-flavor" in the executable name. One example is test_progs-no_alu32,
which load bpf-progs compiled with disabled alu32, located in dir 'no_alu32/'.

The problem is that invoking a "flavor" binary prints to stdout e.g.:
 "Switching to flavor 'no_alu32' subdirectory..."
Thus, intermixing with the number of tests, making it unusable for scripting.

Fix the issue by printing "flavor" info to stderr instead of stdout.

Fixes: 643e7233aa94 ("selftests/bpf: Test_progs option for getting number of tests")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 tools/testing/selftests/bpf/test_progs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index b1e4dadacd9b..d858e883bd75 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -618,7 +618,7 @@ int cd_flavor_subdir(const char *exec_name)
 	if (!flavor)
 		return 0;
 	flavor++;
-	fprintf(stdout, "Switching to flavor '%s' subdirectory...\n", flavor);
+	fprintf(stderr, "Switching to flavor '%s' subdirectory...\n", flavor);
 	return chdir(flavor);
 }
 

Re: [PATCH bpf] selftests/bpf: Fix test_progs-flavor run getting number of tests

[Andrii Nakryiko]

On Fri, Aug 21, 2020 at 8:09 AM Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> Commit 643e7233aa94 ("selftests/bpf: Test_progs option for getting number of
> tests") introduced ability to getting number of tests, which is targeted
> towards scripting.  As demonstrate in the commit the number can be use as a
> shell variable for further scripting.
>
> The test_progs program support "flavor", which is detected by the binary
> have a "-flavor" in the executable name. One example is test_progs-no_alu32,
> which load bpf-progs compiled with disabled alu32, located in dir 'no_alu32/'.
>
> The problem is that invoking a "flavor" binary prints to stdout e.g.:
>  "Switching to flavor 'no_alu32' subdirectory..."
> Thus, intermixing with the number of tests, making it unusable for scripting.
>
> Fix the issue by printing "flavor" info to stderr instead of stdout.
>
> Fixes: 643e7233aa94 ("selftests/bpf: Test_progs option for getting number of tests")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  tools/testing/selftests/bpf/test_progs.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index b1e4dadacd9b..d858e883bd75 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -618,7 +618,7 @@ int cd_flavor_subdir(const char *exec_name)
>         if (!flavor)
>                 return 0;
>         flavor++;
> -       fprintf(stdout, "Switching to flavor '%s' subdirectory...\n", flavor);
> +       fprintf(stderr, "Switching to flavor '%s' subdirectory...\n", flavor);
>         return chdir(flavor);
>  }
>
>
>

Re: [PATCH bpf] selftests/bpf: Fix test_progs-flavor run getting number of tests

[Alexei Starovoitov]

On Fri, Aug 21, 2020 at 8:08 AM Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> Commit 643e7233aa94 ("selftests/bpf: Test_progs option for getting number of
> tests") introduced ability to getting number of tests, which is targeted
> towards scripting.  As demonstrate in the commit the number can be use as a
> shell variable for further scripting.
>
> The test_progs program support "flavor", which is detected by the binary
> have a "-flavor" in the executable name. One example is test_progs-no_alu32,
> which load bpf-progs compiled with disabled alu32, located in dir 'no_alu32/'.
>
> The problem is that invoking a "flavor" binary prints to stdout e.g.:
>  "Switching to flavor 'no_alu32' subdirectory..."
> Thus, intermixing with the number of tests, making it unusable for scripting.
>
> Fix the issue by printing "flavor" info to stderr instead of stdout.
>
> Fixes: 643e7233aa94 ("selftests/bpf: Test_progs option for getting number of tests")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
>  tools/testing/selftests/bpf/test_progs.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index b1e4dadacd9b..d858e883bd75 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -618,7 +618,7 @@ int cd_flavor_subdir(const char *exec_name)
>         if (!flavor)
>                 return 0;
>         flavor++;
> -       fprintf(stdout, "Switching to flavor '%s' subdirectory...\n", flavor);
> +       fprintf(stderr, "Switching to flavor '%s' subdirectory...\n", flavor);

Hmm. May be move it under -v flag instead?
The person or script that runs test_progs-no_alu32 knows what's happening.
That message either to stdout or stderr will be fine under extra verbose flag.

Re: [PATCH bpf] selftests/bpf: Fix test_progs-flavor run getting number of tests

[Jesper Dangaard Brouer]

On Fri, 21 Aug 2020 12:45:08 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> > index b1e4dadacd9b..d858e883bd75 100644
> > --- a/tools/testing/selftests/bpf/test_progs.c
> > +++ b/tools/testing/selftests/bpf/test_progs.c
> > @@ -618,7 +618,7 @@ int cd_flavor_subdir(const char *exec_name)
> >         if (!flavor)
> >                 return 0;
> >         flavor++;
> > -       fprintf(stdout, "Switching to flavor '%s' subdirectory...\n", flavor);
> > +       fprintf(stderr, "Switching to flavor '%s' subdirectory...\n", flavor);  
> 
> Hmm. May be move it under -v flag instead?
> The person or script that runs test_progs-no_alu32 knows what's happening.
> That message either to stdout or stderr will be fine under extra verbose flag.

Sounds good to me with a -v flag, sending V2

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer