Re: [kvm-unit-tests PATCH 1/3] run_tests.sh: reduce return code ambiguity

[Andrew Jones <drjones@redhat.com>]

On Mon, Dec 21, 2015 at 05:31:24PM +0100, Radim Krčmář wrote:
> 2015-12-17 14:10-0600, Andrew Jones:
> > qemu/unittest exit codes are convoluted, causing codes 0 and 1
> > to be ambiguous. Here are the possible meanings
> > 
> >  .-----------------------------------------------------------------.
> >  |                | 0                                 | 1          |
> >  |-----------------------------------------------------------------|
> >  | QEMU           | did something successfully,       | FAILURE    |
> >  |                | but probably didn't run the       |            |
> >  |                | unittest, OR caught SIGINT,       |            |
> >  |                | SIGHUP, or SIGTERM                |            |
> >  |-----------------------------------------------------------------|
> >  | unittest       | for some reason exited using      | SUCCESS    |
> >  |                | ACPI/PSCI, not with debug-exit    |            |
> >  .-----------------------------------------------------------------.
> > 
> > As we can see above, an exit code of zero is even ambiguous for each
> > row, i.e. QEMU could exit with zero because it successfully completed,
> > or because it caught a signal. unittest could exit with zero because
> > it successfully powered-off, or because for some odd reason it powered-
> > off instead of calling debug-exit.
> > 
> > And, the most fun is that exit-code == 1 means QEMU failed, but the
> > unittest succeeded.
> > 
> > This patch attempts to reduce that ambiguity, by also looking at stderr.
> 
> Nice.
> 
> > With it, we have
> > 
> >  0      - unexpected exit from qemu, or the unittest not using debug-exit.
> >           Consider it a FAILURE
> >  1      - unittest SUCCESS
> >  < 128  - something failed (could be the unittest, qemu, or a run script)
> >           Check the logs.
> >  >= 128 - signal (signum = code - 128)
> 
> I think this heuristic should be applied to {arm,x86}/run.
> run_tests.sh would inherit it and we would finally get reasonable exit
> values everywhere.

Good idea. We can add the table to a scripts/functions.bash function and
use it everywhere.

> 
> The resulting table would look like this:
> 
>  0     = unit-test passed
>  77    = unit-test skipped (not implemented yet)
>  124   = unit-test timeouted (implemented in [3/3])
>  127   = qemu returned 0 (debug-exit probably wasn't called)

We already use 127 for abort(), called from a unit test, see
lib/abort.c. I guess we can use 126 for "debug-exit probably wasn't
called". We should also add a (unit test called abort) message for
127.

>  > 128 = exited because of signal $? - 128
>  *     = unit-test failed
> 
> (Signal 0 is not used, so we could map 128 to mean "debug-exit probably
>  wasn't called", but others might not understand our signal convention.

I think we want 128 to be the beginning of signal space, which goes all
the way up to 255, in order to allow exit code masking to work.

>  Anyway, it'd be best for us to start at 200, for `case $? in 2??)` ...)

Start what at 200? I think we have everything covered above. The mapping
looks like this

0	= success
1-63	= unit test failure code
64-127	= test suite failure code
128-255	= signal

which sounds good to me.

> 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> > diff --git a/run_tests.sh b/run_tests.sh
> > @@ -54,10 +55,32 @@ function run()
> >  
> >      # extra_params in the config file may contain backticks that need to be
> >      # expanded, so use eval to start qemu
> > -    eval $cmdline >> test.log
> > +    errlog=$(mktemp)
> > +    eval $cmdline >> test.log 2> $errlog
> | [...]
> |      cat $errlog >> test.log
> 
> This assumes that stderr is always after stdout,

True. I'm not sure that matters when the unit test, which only uses stdout
will always output stuff serially with qemu, which could output a mix. But
your version below is fine by me if we want to pick up the need for the
pipe and tee.

> 
>   eval $cmdline 2>&1 >> test.log | tee $errlog >> test.log
> 
> has a chance to print lines in wrong order too, but I think it's going
> to be closer to the original.

I'll play with it and send a v2 soon.

Thanks,
drew

> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html