On Jun 09, 2025 / 10:40, Bart Van Assche wrote:
> On 6/5/25 8:56 PM, Shin'ichiro Kawasaki wrote:
> > The second problem is impact on following test cases. When the error-
> > checking is enabled for a test case, the test case script exits
> > immediately encountering an error. This skips the cleanup steps in the
> > test case, potentially leaving the test system in a dirty state, which
> > may affect subsequent test cases.
> >
> > To address the two problems, introduce the new flag ERR_EXIT. When a
> > test case sets ERR_EXIT=1 at the beginning of its file, the blktests
> > check script calls "set -e" for the test case. The test case does not
> > need to call "set -e". This allows the strict error-checking to be
> > applied to test cases selectively.
> >
> > If a test case with ERR_EXIT=1 exits due to a non-zero status, catch the
> > exit in _cleanup(). Then print an error message to the user and stop the
> > test script. This ensures the following test cases are not run.
>
> Has the following alternative been considered? Let test script authors add
> "set -e" and "set +e" where appropriate but only in a subshell. If a failure
> occurs, only the subshell will be exited and cleanup code will still be
> executed if it occurs past the subshell. A disadvantage of this
> approach is that global variables can't be set from inside the subshell
> and that another mechanism is needed than local variables to pass output
> from the subshell to the context outside it, e.g. a pipe.
This idea sounds something to consider, but I'm not sure if I fully understand
it. The word "subshell" is not clear for me. Do you mean subshells those test
case authors create in each test script? If so I'm not sure how to ensure that
"set -e" and "set +e" only happen in the subshells. Or do you mean to modify the
check script to create subshells dedicated for each test case run? If so, I will
need some work to understand its impact.
>
> > @@ -372,6 +380,7 @@ _call_test() {
> > fi
> > TIMEFORMAT="%Rs"
> > + ((ERR_EXIT)) && set -e
> > pushd . >/dev/null || return
> > { time "$test_func" >"${seqres}.out" 2>&1; } 2>"${seqres}.runtime"
> > TEST_RUN["exit_status"]=$?
>
> This change makes it harder to write test code because it forces authors
> to surround cleanup code with something like if cleanup_code; then :; fi.
I see, this point makes sense. I'm okay to not call "set -e" in the
_call_test(). If we take this approach, test cases can do "set -e" and "set +e"
wherever in the test scripts, but they must declare ERR_EXIT=1.
