Closed
Bug 1372981
Opened 7 years ago
Closed 7 years ago
Limit the number of times reftest retries the harness after a crash
Categories
(Testing :: Reftest, enhancement)
Testing
Reftest
Tracking
(firefox56 fixed)
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: ahal, Assigned: ahal)
References
Details
Attachments
(1 file)
Bug 1344991 implemented a neat new feature that tries to resume the test run in the event a test crashed. However, sometimes a developer might do a try push with a very crashy Firefox. In this case, the reftest harness will crash at every test and keep restarting.
This takes a very long time and makes the logs enormous (~250MB) due to all the crash stacks. We should put a limit on the number of crashes the harness will attempt to recover from before giving up. I arbitrarily propose 4 (though would be fine with a higher number too).
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
Nice, this will help with a problem I had recently where I'd do a test run with |--debugger=rr|, but because of restarting after a crash, it would start recording a subsequent rr session, when really I just needed the first one.
Assignee | ||
Comment 3•7 years ago
|
||
Good point. Should we automatically set --max-retries=0 if a debugger is attached?
Comment 4•7 years ago
|
||
That sounds good to me. Although perhaps I should just be using --run-until-failure, which hopefully stops after the first crash too.
Assignee | ||
Updated•7 years ago
|
Attachment #8877696 -
Flags: review?(shing.lyu) → review?(jmaher)
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8877696 [details]
Bug 1372981 - Limit reftest to 4 attempts at recovering after a crash,
https://reviewboard.mozilla.org/r/149130/#review155692
::: layout/tools/reftest/runreftest.py:706
(Diff revision 1)
> self.log.info("Process mode: {}".format('e10s' if options.e10s else 'non-e10s'))
> mozleak.process_leak_log(self.leakLogFile,
> leak_thresholds=options.leakThresholds,
> stack_fixer=get_stack_fixer_function(options.utilityPath,
> options.symbolsPath))
> - self.cleanup(profileDir)
> + if status == 0:
do we not need to cleanup anymore?
Attachment #8877696 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 6•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8877696 [details]
Bug 1372981 - Limit reftest to 4 attempts at recovering after a crash,
https://reviewboard.mozilla.org/r/149130/#review155692
> do we not need to cleanup anymore?
We do. If you expand the source down a bit (probably outside of this diff), we're calling self.cleanup() in a finally: block. So before this change, cleanup was getting called twice.
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8877696 [details]
Bug 1372981 - Limit reftest to 4 attempts at recovering after a crash,
https://reviewboard.mozilla.org/r/149130/#review155728
thanks for pointing that out, r+!
Attachment #8877696 -
Flags: review- → review+
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1990807be524
Limit reftest to 4 attempts at recovering after a crash, r=jmaher
Comment 9•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•