1470177 - Task timeouts in geckoview-junit

Status: RESOLVED FIXED

(Testing :: General, enhancement)

Description

[Geoff Brown [:gbrown]]

https://treeherder.mozilla.org/logviewer.html#?job_id=184126249&repo=mozilla-inbound&lineNumber=1749

https://treeherder.mozilla.org/logviewer.html#?job_id=183618331&repo=autoland&lineNumber=2322

https://treeherder.mozilla.org/logviewer.html#?job_id=183521484&repo=mozilla-central&lineNumber=2710

https://treeherder.mozilla.org/logviewer.html#?job_id=183291646&repo=mozilla-inbound&lineNumber=2696

If the test is hung, why isn't the harness timing out?

Comment 1

[Geoff Brown [:gbrown]]

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

Comment 2

[Geoff Brown [:gbrown]]

Attachment: specify shell timeout correctly in runjunit, and check for timeout

Comment 3

[Geoff Brown [:gbrown]]

Attachment: ensure adb.py timeout comparison is numeric

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

Comment 4

[Geoff Brown [:gbrown]]

These patches will improve diagnostics for timeouts, but I still need to investigate the cause of the geckoview-junit hang...

Comment 5

[Bob Clary [:bc] (inactive)]

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.

Comment 6

[Bob Clary [:bc] (inactive)]

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.

Comment 7

[Geoff Brown [:gbrown]]

Thanks. As discussed, will land with a minor change to avoid the same sort of error in timed_read_line.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
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

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/82632b0c2b94
https://hg.mozilla.org/mozilla-central/rev/27db2b2703d2

Comment 10

[Geoff Brown [:gbrown]]

With the new timeout:

https://treeherder.mozilla.org/logviewer.html#?job_id=184783944&repo=mozilla-inbound&lineNumber=1181
https://treeherder.mozilla.org/logviewer.html#?job_id=184790458&repo=autoland&lineNumber=1973

Comment 11

[Geoff Brown [:gbrown]]

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