On Tue, Dec 22, 2015 at 07:02:21PM +0100, Radim Krčmář wrote:
> 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.
OK, I'll do something allowing us to easily enable a long default
timeout.
>
> (I'd probably allow something as evil as `eval`ing the TIMEOUT, for
> unlikely stuff like TIMEOUT='$(( ${timeout:-10} / 2 ))')
I'd prefer to avoid the evil^Weval stuff... And the timeout duration can
already be a floating point.
>
> > 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?
arm tests are sometimes tcg only. Hey, we'll take what we get for
arm, as we're sadly missing everything...
>
> > 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.
OK
Thanks,
drew
> --
> 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
--
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
