| Message ID | 20180817161404.9420-1-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tests/boot-serial-test: Bump timeout to 6 minutes | expand |
On 17/08/18 17:14, Peter Maydell wrote: > On a SPARC host that I'm using as a build test machine, the > boot-serial-test for the SPARC guest machines takes about 65 > seconds to execute. This means that it hits the current > 60 second timer on these tests. Push the timeout up so > that it doesn't trigger spuriously on slow hosts like this one. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > tests/boot-serial-test.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c > index 1355df924dd..fca5f2f5da9 100644 > --- a/tests/boot-serial-test.c > +++ b/tests/boot-serial-test.c > @@ -116,8 +116,8 @@ static bool check_guest_output(const testdef_t *test, int fd) > int i, nbr = 0, pos = 0, ccnt; > char ch; > > - /* Poll serial output... Wait at most 60 seconds */ > - for (i = 0; i < 6000; ++i) { > + /* Poll serial output... Wait at most 360 seconds */ > + for (i = 0; i < 36000; ++i) { > ccnt = 0; > while (ccnt++ < 512 && (nbr = read(fd, &ch, 1)) == 1) { > if (ch == test->expect[pos]) { Wow that is amazingly slow... however I guess it's not completely unreasonable to increase the timeout accordingly. Acked-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> ATB, Mark.
On 2018-08-17 18:14, Peter Maydell wrote: > On a SPARC host that I'm using as a build test machine, the > boot-serial-test for the SPARC guest machines takes about 65 > seconds to execute. This means that it hits the current > 60 second timer on these tests. Push the timeout up so > that it doesn't trigger spuriously on slow hosts like this one. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > tests/boot-serial-test.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c > index 1355df924dd..fca5f2f5da9 100644 > --- a/tests/boot-serial-test.c > +++ b/tests/boot-serial-test.c > @@ -116,8 +116,8 @@ static bool check_guest_output(const testdef_t *test, int fd) > int i, nbr = 0, pos = 0, ccnt; > char ch; > > - /* Poll serial output... Wait at most 60 seconds */ > - for (i = 0; i < 6000; ++i) { > + /* Poll serial output... Wait at most 360 seconds */ > + for (i = 0; i < 36000; ++i) { 6 minutes is really a lot already. I guess most users will hit CTRL-C before waiting so long if there is a realy problem here ... If the current tests just takes a little bit more than 1 minute on the Sparc machine, maybe 2 or 3 minutes would sufficient, too? Thomas
On 18 August 2018 at 10:07, Thomas Huth <thuth@redhat.com> wrote: > 6 minutes is really a lot already. I guess most users will hit CTRL-C > before waiting so long if there is a realy problem here ... If the > current tests just takes a little bit more than 1 minute on the Sparc > machine, maybe 2 or 3 minutes would sufficient, too? Or maybe not. I don't like tests that run close to their timeout limits, because that tends to mean that my automated test runs are flaky when the machine happens to be heavily loaded when a test is running. I think the purpose of a timeout is to prevent the test run hanging indefinitely; as you say, console users can hit ctrl-c if they get bored anyway. thanks -- PMM
On 2018-08-18 12:10, Peter Maydell wrote: > On 18 August 2018 at 10:07, Thomas Huth <thuth@redhat.com> wrote: >> 6 minutes is really a lot already. I guess most users will hit CTRL-C >> before waiting so long if there is a realy problem here ... If the >> current tests just takes a little bit more than 1 minute on the Sparc >> machine, maybe 2 or 3 minutes would sufficient, too? > > Or maybe not. I don't like tests that run close to their timeout > limits, because that tends to mean that my automated test runs > are flaky when the machine happens to be heavily loaded when > a test is running. I think the purpose of a timeout is to prevent > the test run hanging indefinitely; as you say, console users can > hit ctrl-c if they get bored anyway. ... or maybe somebody tries to run TCI on a Sparc host one day ... ok, you've convinced me, let's go with the 360 seconds: Reviewed-by: Thomas Huth <thuth@redhat.com> Paolo, could you please queue up this patch (since I don't have anything else pending for assembling a PULL request)? Thomas
On 20 August 2018 at 07:21, Thomas Huth <thuth@redhat.com> wrote: > On 2018-08-18 12:10, Peter Maydell wrote: >> On 18 August 2018 at 10:07, Thomas Huth <thuth@redhat.com> wrote: >>> 6 minutes is really a lot already. I guess most users will hit CTRL-C >>> before waiting so long if there is a realy problem here ... If the >>> current tests just takes a little bit more than 1 minute on the Sparc >>> machine, maybe 2 or 3 minutes would sufficient, too? >> >> Or maybe not. I don't like tests that run close to their timeout >> limits, because that tends to mean that my automated test runs >> are flaky when the machine happens to be heavily loaded when >> a test is running. I think the purpose of a timeout is to prevent >> the test run hanging indefinitely; as you say, console users can >> hit ctrl-c if they get bored anyway. > > ... or maybe somebody tries to run TCI on a Sparc host one day ... ok, > you've convinced me, let's go with the 360 seconds: > > Reviewed-by: Thomas Huth <thuth@redhat.com> > > Paolo, could you please queue up this patch (since I don't have anything > else pending for assembling a PULL request)? I can just apply it directly, since it's useful for buildtesting. thanks -- PMM
On 2018-08-20 10:45, Peter Maydell wrote: > On 20 August 2018 at 07:21, Thomas Huth <thuth@redhat.com> wrote: >> On 2018-08-18 12:10, Peter Maydell wrote: >>> On 18 August 2018 at 10:07, Thomas Huth <thuth@redhat.com> wrote: >>>> 6 minutes is really a lot already. I guess most users will hit CTRL-C >>>> before waiting so long if there is a realy problem here ... If the >>>> current tests just takes a little bit more than 1 minute on the Sparc >>>> machine, maybe 2 or 3 minutes would sufficient, too? >>> >>> Or maybe not. I don't like tests that run close to their timeout >>> limits, because that tends to mean that my automated test runs >>> are flaky when the machine happens to be heavily loaded when >>> a test is running. I think the purpose of a timeout is to prevent >>> the test run hanging indefinitely; as you say, console users can >>> hit ctrl-c if they get bored anyway. >> >> ... or maybe somebody tries to run TCI on a Sparc host one day ... ok, >> you've convinced me, let's go with the 360 seconds: >> >> Reviewed-by: Thomas Huth <thuth@redhat.com> >> >> Paolo, could you please queue up this patch (since I don't have anything >> else pending for assembling a PULL request)? > > I can just apply it directly, since it's useful for buildtesting. Ok, great, that's fine of course, too. Thomas
diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c index 1355df924dd..fca5f2f5da9 100644 --- a/tests/boot-serial-test.c +++ b/tests/boot-serial-test.c @@ -116,8 +116,8 @@ static bool check_guest_output(const testdef_t *test, int fd) int i, nbr = 0, pos = 0, ccnt; char ch; - /* Poll serial output... Wait at most 60 seconds */ - for (i = 0; i < 6000; ++i) { + /* Poll serial output... Wait at most 360 seconds */ + for (i = 0; i < 36000; ++i) { ccnt = 0; while (ccnt++ < 512 && (nbr = read(fd, &ch, 1)) == 1) { if (ch == test->expect[pos]) {
On a SPARC host that I'm using as a build test machine, the boot-serial-test for the SPARC guest machines takes about 65 seconds to execute. This means that it hits the current 60 second timer on these tests. Push the timeout up so that it doesn't trigger spuriously on slow hosts like this one. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/boot-serial-test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)