Closed Bug 532184 Opened 10 years ago Closed 10 years ago

automation.py: add optional maxTime argument to runApp

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: jruderman)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
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.
Attachment #415463 - Flags: review?(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.
Attachment #415463 - Flags: review?(ted.mielczarek) → review+
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))
Attached patch patchSplinter Review
Attachment #415463 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/1a99b8a94b71
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Blocks: 505520
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.