2015-12-21 13:45-0600, Andrew Jones:
> On Mon, Dec 21, 2015 at 06:04:20PM +0100, Radim Krčmář wrote:
>> 2015-12-17 14:10-0600, Andrew Jones:
>> > diff --git a/run_tests.sh b/run_tests.sh
>> > @@ -21,6 +21,7 @@ function run()
>> > + local timeout="${9:-$TIMEOUT}"
>> > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
>> > @@ -97,8 +98,12 @@ if [ "\$QEMU" ]; then
>> > +if [ "$timeout" ]; then
>> > + timeout_cmd='timeout --foreground $timeout'
>>
>> Both would be nicer if they took the TIMEOUT variable as an override.
>
> Everything already takes TIMEOUT as an override, i.e.
>
> TIMEOUT=3 ./run_tests.sh
>
> and
>
> TIMEOUT=3 arm/run arm/test.flat
>
> will both already set a timeout for any test that didn't have a timeout
> set in unittests.cfg, or wasn't run with run()/unittests.cfg.
Tests made with mkstandalone.sh ignore the TIMEOUT variable ...
> Or, did
> you mean that you'd prefer TIMEOUT to override the timeout in
> unittests.cfg?
... and yes, I think that we could have a
- global timeout for all tests. Something far longer than any tests
should take (2 minutes?). To automatically handle random hangs.
- per-test timeout in unittests.cfg. When the test is known to timeout
often and the usual time to fail is much shorter than the global
default. (Shouldn't be used much.)
- TIMEOUT variable. It has to override the global timeout and I think
that if we are ever going to use it, it's because we want something
weird. Like using `TIMEOUT=0 ./run_tests.sh` to disable all
timeouts, prolonging/shortening timeouts because of a special
configuration, ...
Because we should improve our defaults otherwise.
(I'd probably allow something as evil as `eval`ing the TIMEOUT, for
unlikely stuff like TIMEOUT='$(( ${timeout:-10} / 2 ))')
> That does make some sense, in the case the one in the
> config is longer than desired, however it also makes sense the way I
> have it now when the one in the config is shorter than TIMEOUT (the
> fallback default). I think I like it this way better.
Ok, the difference was negligible to begin with.
>> We already don't do that for accel and the patch seems ok in other
>> regards,
>
> Hmm, for accel I see a need for a patch allowing us to do
>
> ACCEL=?? ./run_tests.sh
Btw. why do we have ACCEL when the project is *kvm*_unit_tests?
> as I already have for TIMEOUT. Also, for both I should add a
> mkstandalone patch allowing
>
> TIMEOUT=? ACCEL=? make standalone
I'd also handle TIMEOUT/ACCEL in resulting standalone tests.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
