Closed Bug 1470177 Opened 3 years ago Closed 3 years ago

Task timeouts in geckoview-junit

Categories

(Testing :: General, enhancement)

Version 3
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gbrown, Assigned: gbrown)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

(In reply to Geoff Brown [:gbrown] from comment #0)
> If the test is hung, why isn't the harness timing out?

runjunit.py passes options.max_time as the timeout parameter, but options.max_time is a string.

>>> 10 <= "2400"
True
>>> 2500 <= "2400"
True
>>> 

Also, runjunit.py doesn't check the return value from shell to report a timeout.
In case there are other callers that pass a string timeout parameter...

(I also briefly considered checking the type of the timeout parameter, but that's complicated by the possibility of None, and because there are so many functions taking a timeout parameter.)

Seems to do no harm:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c9941bfd6eb87025f1b2ac0f038e8a38dd6a36a
Attachment #8987194 - Flags: review?(bob)
These patches will improve diagnostics for timeouts, but I still need to investigate the cause of the geckoview-junit hang...
Keywords: leave-open
Comment on attachment 8987192 [details] [diff] [review]
specify shell timeout correctly in runjunit, and check for timeout

Review of attachment 8987192 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm.
Attachment #8987192 - Flags: review?(bob) → review+
Comment on attachment 8987194 [details] [diff] [review]
ensure adb.py timeout comparison is numeric

Review of attachment 8987194 [details] [diff] [review]:
-----------------------------------------------------------------

That looks good as far as it goes but def _timed_read_line has a signal.alarm(timeout) that will fail if timeout is not an int. I don't see it being called with a timeout argument though so maybe it doesn't matter.
Attachment #8987194 - Flags: review?(bob) → review+
Thanks. As discussed, will land with a minor change to avoid the same sort of error in timed_read_line.
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/82632b0c2b94
Improve timeout handling in runjunit.py; r=bc
https://hg.mozilla.org/integration/mozilla-inbound/rev/27db2b2703d2
Ensure numeric comparison of timeout in adb.py; r=bc
(In reply to Geoff Brown [:gbrown] from comment #4)
> These patches will improve diagnostics for timeouts, but I still need to
> investigate the cause of the geckoview-junit hang...

Actually, now we have bug 1471080 for the hang -- following up there.
Status: NEW → RESOLVED
Closed: 3 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.