Flux-core: Tests that use `jq` not marked with `HAVE_JQ`

[SteVwonder]

The following tests failed without jq installed, and then passed after installing it:

FAIL: t2204-job-info.t 45 - flux job list outputs user job name
FAIL: t2204-job-info.t 46 - flux job lists first argument for job name
FAIL: t2204-job-info.t 47 - flux job lists basename of first argument for job name
FAIL: t2204-job-info.t 48 - flux job lists full path for job name if basename fails on first arg
FAIL: t2204-job-info.t 50 - verify job names preserved across restart
FAIL: t2204-job-info.t 51 - flux job list outputs ntasks correctly (1 task)
FAIL: t2204-job-info.t 52 - flux job list outputs ntasks correctly (4 tasks)
FAIL: t2204-job-info.t 54 - verify task count preserved across restart
FAIL: t2204-job-info.t 55 - flux job list outputs nnodes/ranks correctly (1 task / 1 node)
FAIL: t2204-job-info.t 56 - flux job list outputs nnodes/ranks correctly (2 tasks, / 1 node)
FAIL: t2204-job-info.t 57 - flux job list outputs nnodes/ranks correctly (3 tasks, / 2 nodes)
FAIL: t2204-job-info.t 58 - flux job list outputs nnodes/ranks correctly (5 tasks, / 3 nodes)
FAIL: t2204-job-info.t 59 - flux job list outputs success correctly (true)
FAIL: t2204-job-info.t 60 - flux job list outputs success correctly (false)
FAIL: t2204-job-info.t 61 - flux job list outputs exceptions correctly (no exception)
FAIL: t2204-job-info.t 62 - flux job list outputs exceptions correctly (exception)
FAIL: t2204-job-info.t 64 - verify nnodes preserved across restart
FAIL: t2204-job-info.t 102 - flux job wait-event w/ bad match-context fails (invalid input)
FAIL: t2204-job-info.t 103 - flux job wait-event -p works (eventlog)

FAIL: t2603-job-shell-initrc.t 5 - flux-shell: initrc: in jobspec works

FAIL: t2610-job-shell-mpir.t 1 - flux-shell: 1N/1P: trace+mpir works
FAIL: t2610-job-shell-mpir.t 2 - flux-shell: 2N/2P: trace+mpir works
FAIL: t2610-job-shell-mpir.t 3 - flux-shell: 2N/4P: trace+mpir works
FAIL: t2610-job-shell-mpir.t 4 - flux-shell: 4N/4P: trace+mpir works
FAIL: t2610-job-shell-mpir.t 5 - flux-shell: 4N/8P: trace+mpir works
FAIL: t2610-job-shell-mpir.t 6 - flux-shell: 4N/7P: trace+mpir works
FAIL: t2610-job-shell-mpir.t 7 - flux-shell: test security of proctable method

FAIL: t2612-job-shell-pty.t 1 - pty: submit a job with pty
FAIL: t2612-job-shell-pty.t 2 - pty: run job with pty

FAIL: t2700-mini-cmd.t 32 - flux mini submit --gpus-per-task adds gpus to task slot

For the t2204-job-info.t test case, so many of the tests use jq, it is probably worth just marking the whole test as requiring it.

Ultimately it isn't a huge deal, but does anyone have any thoughts on how to prevent regressions in the future? We could maybe setup a single builder to not have jq, but that builder will then skip a bunch of tests. We could require jq as part of the configure but that is one more dependency in order to build. Not sure if we can do something similar to the chain-lint to check for miss-marked tests automatically.

[grondo]

I don't prefer checking for test-only requirements in ./configure. Then you require users to install all test dependencies even when their only intention is to run make and not make check (e.g. perhaps in a package build system). Maybe a --with-test-environment or similar flag could check for test dependencies? But then users who are going to run make check would complain that it is not the default.

I fear you can't satisfy everyone all the time. I was even on one distro where hostname wasn't installed by default.

We could maybe setup a single builder to not have jq, but that builder will then skip a bunch of tests.

I guess that would be the point, right? We could have one builder with none of the test dependencies installed, and ensure make check skips tests instead of failing. However, we would have to maintain a separate docker image in order to do this.

It may be less work to just keep things at the status quo, and fix missing test prereqs as they come up.

[grondo]

Not sure if we can do something similar to the chain-lint to check for miss-marked tests automatically.

I had an idea here. We could optionally have the test suite create wrapper script for all utilities for which we use prereqs (and maybe move the set of the prereqs to a global location to ensure the same string is used).

E.g, with the below patch in place, all tests that currently skip_all when jq is not found instead use

skip_all_unless_have jq

And current per-test set of HAVE_JQ is removed, since this is now set globally.

Then for example, if we try to run one of the tests that uses jq without HAVE_JQ, the test fails and prints an error:

$ TEST_CHECK_PREREQS=t ./t2610-job-shell-mpir.t  -d -v
expecting success: 
    id=$(flux mini submit -o stop-tasks-in-exec \
             -n${TASKS} -N${NODES} /bin/true)  &&
        flux job wait-event -vt 5 -p guest.exec.eventlog \
                -m sync=true ${id} shell.start &&
        ${mpir} $(shell_leader_rank $id) $(shell_service $id) &&
        flux job kill -s CONT ${id} &&
        flux job attach ${id}

1589208953.788322 init
1589208953.790529 starting
1589208953.827128 shell.init leader-rank=0 size=1 service="1000-shell-11878268928"
1589208953.829705 shell.start task-count=1 sync=true
Use of jq without prereq HAVE_JQ!
Use of jq without prereq HAVE_JQ!
mpir-test: Usage: /home/grondo/git/flux-core.git/t/shell/.libs/mpir LEADER-RANK SERVICE

not ok 1 - flux-shell: 1N/1P: trace+mpir works

diff --git a/t/sharness.d/flux-sharness.sh b/t/sharness.d/flux-sharness.sh
index e71d95071..b8e69f356 100644
--- a/t/sharness.d/flux-sharness.sh
+++ b/t/sharness.d/flux-sharness.sh
@@ -162,6 +162,63 @@ test_on_rank() {
     flux exec --rank=${ranks} "$@"
 }

+#
+# Check for a program and skip all tests immediately if not found.
+# Exports the program in SHARNESS_test_skip_all_prereq for later
+# check in TEST_CHECK_PREREQS
+#
+skip_all_unless_have()
+{
+    prog_path=$(which $1 2>/dev/null)
+    if test -z "$prog_path"; then
+        skip_all="$1 not found. Skipping all tests"
+        test_done
+    fi
+    eval "$1=$prog_path"
+    export SHARNESS_test_skip_all_prereq="$SHARNESS_test_skip_all_prereq,$1"
+}
+
+GLOBAL_PROGRAM_PREREQS="HAVE_JQ:jq"
+
+#
+#  Check for programs in GLOBAL_PROGRAM_PREREQS and set prereq and
+#   "<name>=<program_path>" if found. If TEST_CHECK_PREREQS is set, then
+#   create a wrapper script in trash-directory/bin which will ensure the
+#   prereq (or global skip_all above) has been used before each invocation
+#   of program. This will catch places in testsuite where program is used
+#   without testing the prerequisite.
+#
+for prereq in $GLOBAL_PROGRAM_PREREQS; do
+    prog=${prereq#*:}
+    path_prog=$(which ${prog} 2>/dev/null || echo "/bin/false")
+    req=${prereq%:*}
+    test "${path_prog}" = "/bin/false" || test_set_prereq ${req}
+    eval "${prog}=${path_prog}"
+    if test -n "$TEST_CHECK_PREREQS"; then
+               dir=${SHARNESS_TRASH_DIRECTORY}/bin
+               mkdir -p ${dir}
+               cat <<-EOF > ${dir}/$prog
+               #!/bin/sh
+               saved_IFS=\$IFS
+               IFS=,
+               for x in \$test_prereq; do
+                       test "\$x" = "$req" && ok=t
+               done
+               for x in \$SHARNESS_test_skip_all_prereq; do
+               test "\$x" = "$prog" && ok=t
+               done
+               test -n "\$ok" && exec $path_prog "\$@"
+               echo >&2 "Use of $prog without prereq $req!"
+               exit 1
+               EOF
+               chmod +x ${dir}/$prog
+    fi
+done
+
+if test -n "$TEST_CHECK_PREREQS"; then
+    export PATH=${SHARNESS_TRASH_DIRECTORY}/bin:${PATH}
+fi
+
 #  Export a shorter name for this test
 TEST_NAME=$SHARNESS_TEST_NAME
 export TEST_NAME