On Fri, 2022-03-18 at 09:46 +0100, Marc Hartmayer wrote:
> Nico Boehr <nrb@xxxxxxxxxxxxx> writes:
[...]
> > Which looks like a completely fine TAP file, but actually we ran
> > into a timeout
> > and didn't even run all tests.
> >
> > With this patch, we get an additional line at the end which
> > properly shows
> > something went wrong:
> >
> > not ok 7 - diag288: timeout; duration=1s
>
> This results from the fact that the TAP13 test result is generated by
> the function `RUNTIME_log_stdout` and not by `print_result` (see
> commit
> 6e1d3752d7ca ("tap13: list testcases individually")). In
> `RUNTIME_log_stdout` we don’t have access to the QEMU command exit
> code.
Basically yes. If we had that we could do all the TAP special handling
there.
>
[...]
> > diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> > index 6d5fced94246..b41b3d444e27 100644
> > --- a/scripts/runtime.bash
> > +++ b/scripts/runtime.bash
> > @@ -163,9 +163,19 @@ function run()
> > print_result "SKIP" $testname "$summary"
> > elif [ $ret -eq 124 ]; then
> > print_result "FAIL" $testname "" "timeout;
> > duration=$timeout"
> > + if [[ $tap_output != "no" ]]; then
> > + echo "not ok TEST_NUMBER - ${testname}: timeout;
> > duration=$timeout" >&3
> > + fi
> > elif [ $ret -gt 127 ]; then
> > - print_result "FAIL" $testname "" "terminated on SIG$(kill
> > -l $(($ret - 128)))"
> > + signame="SIG"$(kill -l $(($ret - 128)))
> > + print_result "FAIL" $testname "" "terminated on $signame"
> > + if [[ $tap_output != "no" ]]; then
> > + echo "not ok TEST_NUMBER - ${testname}: terminated on
> > $signame" >&3
> > + fi
> > else
> > + if [ $ret -eq 127 ] && [[ $tap_output != "no" ]]; then
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This is a new case, no? If so please add a separate
> patch creating another `elif` branch.
Probably depends on what you mean by 'new'. The else branch handles the
test aborting (for example, exception in the guest) _and_ the case of
at least one report failing.
In the latter case, I wanted no additional line in the TAP because we
can already see the failed report there.
Making the if an elif makes sense, will do that.
I don't get what you would want to see in a separate patch, can you
please make a pseudocode example?
