| Message ID | 20201209000120.2709992-1-kpsingh@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,v2] selftests/bpf: Silence ima_setup.sh when not running in verbose mode. | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 31 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Tue, Dec 8, 2020 at 4:01 PM KP Singh <kpsingh@kernel.org> wrote: > > Currently, ima_setup.sh spews outputs from commands like mkfs and dd > on the terminal without taking into account the verbosity level of > the test framework. Update test_progs to set the environment variable > SELFTESTS_VERBOSE=1 when a verbose output is requested. This > environment variable is then used by ima_setup.sh (and can be used by > other similar scripts) to obey the verbosity level of the test harness > without needing to re-implement command line options for verbosity. > > Fixes: 34b82d3ac105 ("bpf: Add a selftest for bpf_ima_inode_hash") > Reported-by: Andrii Nakryiko <andrii@kernel.org> > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > Signed-off-by: KP Singh <kpsingh@kernel.org> > --- > tools/testing/selftests/bpf/ima_setup.sh | 6 ++++++ > tools/testing/selftests/bpf/test_progs.c | 10 ++++++++++ > 2 files changed, 16 insertions(+) > > diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh > index 2bfc646bc230..7490a9bae33e 100755 > --- a/tools/testing/selftests/bpf/ima_setup.sh > +++ b/tools/testing/selftests/bpf/ima_setup.sh > @@ -81,9 +81,15 @@ main() > > local action="$1" > local tmp_dir="$2" > + local verbose="${SELFTESTS_VERBOSE:=0}" > > [[ ! -d "${tmp_dir}" ]] && echo "Directory ${tmp_dir} doesn't exist" && exit 1 > > + if [[ "${verbose}" -eq 0 ]]; then > + exec 1> /dev/null > + exec 2>&1 can't this be done with one exec, though: exec 2>&1 1>/dev/null ? It also actually would be nice to not completely discard the output, but rather redirect it to a temporary file and emit it on error with trap. test_progs behavior is no extra output on success, but emit it fully at the end if test is failing. Would be nice to preserve this for shell script as well, as otherwise debugging this in CI would be nearly impossible. > + fi > + > if [[ "${action}" == "setup" ]]; then > setup "${tmp_dir}" > elif [[ "${action}" == "cleanup" ]]; then > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c > index 5ef081bdae4e..7d077d48cadd 100644 > --- a/tools/testing/selftests/bpf/test_progs.c > +++ b/tools/testing/selftests/bpf/test_progs.c > @@ -587,6 +587,16 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state) > return -EINVAL; > } > } > + > + if (env->verbosity > VERBOSE_NONE) { > + if (setenv("SELFTESTS_VERBOSE", "1", 1) == -1) { > + fprintf(stderr, > + "Unable to setenv SELFTESTS_VERBOSE=1 (errno=%d)", > + errno); > + return -1; > + } > + } yep, this is what I had in mind, thanks. > + > break; > case ARG_GET_TEST_CNT: > env->get_test_cnt = true; > -- > 2.29.2.576.ga3fc446d84-goog >
[...] > > > > + if [[ "${verbose}" -eq 0 ]]; then > > + exec 1> /dev/null > > + exec 2>&1 > > can't this be done with one exec, though: > > exec 2>&1 1>/dev/null Yep, fixed. > > ? > > It also actually would be nice to not completely discard the output, > but rather redirect it to a temporary file and emit it on error with > trap. test_progs behavior is no extra output on success, but emit it > fully at the end if test is failing. Would be nice to preserve this > for shell script as well, as otherwise debugging this in CI would be > nearly impossible. > Yep, this is better for debuggability. I will update and send another version.
diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh index 2bfc646bc230..7490a9bae33e 100755 --- a/tools/testing/selftests/bpf/ima_setup.sh +++ b/tools/testing/selftests/bpf/ima_setup.sh @@ -81,9 +81,15 @@ main() local action="$1" local tmp_dir="$2" + local verbose="${SELFTESTS_VERBOSE:=0}" [[ ! -d "${tmp_dir}" ]] && echo "Directory ${tmp_dir} doesn't exist" && exit 1 + if [[ "${verbose}" -eq 0 ]]; then + exec 1> /dev/null + exec 2>&1 + fi + if [[ "${action}" == "setup" ]]; then setup "${tmp_dir}" elif [[ "${action}" == "cleanup" ]]; then diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 5ef081bdae4e..7d077d48cadd 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -587,6 +587,16 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state) return -EINVAL; } } + + if (env->verbosity > VERBOSE_NONE) { + if (setenv("SELFTESTS_VERBOSE", "1", 1) == -1) { + fprintf(stderr, + "Unable to setenv SELFTESTS_VERBOSE=1 (errno=%d)", + errno); + return -1; + } + } + break; case ARG_GET_TEST_CNT: env->get_test_cnt = true;
Currently, ima_setup.sh spews outputs from commands like mkfs and dd on the terminal without taking into account the verbosity level of the test framework. Update test_progs to set the environment variable SELFTESTS_VERBOSE=1 when a verbose output is requested. This environment variable is then used by ima_setup.sh (and can be used by other similar scripts) to obey the verbosity level of the test harness without needing to re-implement command line options for verbosity. Fixes: 34b82d3ac105 ("bpf: Add a selftest for bpf_ima_inode_hash") Reported-by: Andrii Nakryiko <andrii@kernel.org> Suggested-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: KP Singh <kpsingh@kernel.org> --- tools/testing/selftests/bpf/ima_setup.sh | 6 ++++++ tools/testing/selftests/bpf/test_progs.c | 10 ++++++++++ 2 files changed, 16 insertions(+)