Closed Bug 532184 Opened 13 years ago Closed 13 years ago add optional maxTime argument to runApp


(Firefox Build System :: General, enhancement)

Not set


(Not tracked)



(Reporter: jruderman, Assigned: jruderman)



(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]

>diff --git a/build/ b/build/
>--- a/build/
>+++ b/build/
> 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 - 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
>+"TEST-UNEXPECTED-FAIL | | 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:"INFO | | Application ran for: %s", str( - startTime))
Attached patch patchSplinter Review
Attachment #415463 - Attachment is obsolete: true
Closed: 13 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.