532184 - automation.py: add optional maxTime argument to runApp

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Jesse Ruderman]

Attachment: patch

The existing "timeout" argument to runApp only applies if the app stops producing output.  I often get Firefox into states with infinite loops that include assertions, warnings, or js errors, so I need a way to stop those too.

I know we chose a buildbot-level solution for unit tests on debug builds (see bug 510552), but that doesn't work as well for fuzzing, where I want to recover and perhaps make a reduced testcase for a hang.

Comment 1

[(not currently active) Ted Mielczarek]

Comment on attachment 415463 [details] [diff] [review]
patch

>diff --git a/build/automation.py.in b/build/automation.py.in
>--- a/build/automation.py.in
>+++ b/build/automation.py.in
> from datetime import datetime
>+from datetime import timedelta

from datetime import datetime, timedelta


>+  Run the app, log the duration it took to execute, return the status code.
>+  Kills the app if it takes longer than |maxTime| seconds or outputs nothing for |timeout| seconds.
>+  """

Can we phrase this a little more clearly? Perhaps "Kills the app if it runs for longer than |maxTime| seconds, or outputs nothing for |timeout| seconds."

>+      if not hitMaxTime and maxTime and datetime.now() - startTime > timedelta(seconds = maxTime):

Any reason you chose to use datetime and timedelta as opposed to just using time.time()? (There's an example in the Win32 implementation of readWithTimeout above.

>+        # Kill the main process, but continue reading from stack fixer so as not to deadlock on stackFixerProcess.wait().
>+        hitMaxTime = True
>+        log.info("TEST-UNEXPECTED-FAIL | automation.py | application timed out after %d seconds (despite continual output)", int(maxTime))

I think this could use a different phrasing. Maybe "application ran for longer than allowed maximum run time of %d seconds"? That's a little clumsy, but something along those lines.

r=me with those changes.

Comment 2

[Jesse Ruderman]

I use datetime/timedelta (rather than time) because runApp already uses datetime explicitly and timedelta implicitly, as in the line:

log.info("INFO | automation.py | Application ran for: %s", str(datetime.now() - startTime))

Comment 3

[Jesse Ruderman]

Attachment: patch

Comment 4

[Jesse Ruderman]

http://hg.mozilla.org/mozilla-central/rev/1a99b8a94b71