Closed
Bug 1477807
Opened 7 years ago
Closed 7 years ago
Distinguish ADBTimeoutErrors from other exceptions in Android remote tests
Categories
(Testing :: General, defect)
Testing
General
Tracking
(firefox63 fixed)
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: bc, Assigned: bc)
References
Details
Attachments
(2 files, 1 obsolete file)
17.21 KB,
patch
|
gbrown
:
review+
|
Details | Diff | Splinter Review |
2.20 KB,
patch
|
gbrown
:
review+
|
Details | Diff | Splinter Review |
There are several places in our test automation which do not distinguish between normal exceptions which may not indicate a problem with the device under test and ADBTimeoutErrors which typically are fatal to a test run. This leads to situations where the test framework continues to attempt to contact the device resulting in multiple ADBTimeoutErrors. For example,
<https://treeherder.mozilla.org/#/jobs?repo=try&revision=21f968df8951596158030b40c9e96060505baaf4&filter-tier=1&filter-tier=2&filter-tier=3&selectedJob=188983291>
<https://treeherder.mozilla.org/logviewer.html#?job_id=188983291&repo=try&lineNumber=2526>
which took 42 minutes to complete due to 4 ADBTimeoutErrors.
The following shows the effect of handling ADBTimeoutErrors separately where a single ADBTimeoutError results in a run time of 27 minutes.
<https://treeherder.mozilla.org/#/jobs?repo=try&revision=a476ade1be650715aa69861bec8eacc4fbe461d2&filter-tier=1&filter-tier=2&filter-tier=3&selectedJob=189292242>
<https://treeherder.mozilla.org/logviewer.html#?job_id=189292242&repo=try&lineNumber=2540>
A full run of android tests with this patch on try:
<https://treeherder.mozilla.org/#/jobs?repo=try&revision=ef305f6ec28913e6ed2bf7d79b2e616461d49329&filter-tier=1&filter-tier=2&filter-tier=3>
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8994301 -
Flags: review?(gbrown)
Assignee | ||
Updated•7 years ago
|
Attachment #8994301 -
Attachment is obsolete: true
Attachment #8994301 -
Flags: review?(gbrown)
Assignee | ||
Comment 2•7 years ago
|
||
My previous patch left out the part which prevented the clean up and device info from running when an ADBTimeoutError occured. The previous g5 06-10 try run did include both patches however so that does show the effect of the ADBTimeoutError. I've folded the two patches together and submitted the result to try again. This does not include the g5s so doesn't have an explicit ADBTimeoutError.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=27827ca1d6b76c15fd954f903ca96dc4187f5094&filter-tier=1&filter-tier=2&filter-tier=3
Attachment #8994785 -
Flags: review?(gbrown)
![]() |
||
Comment 3•7 years ago
|
||
Comment on attachment 8994785 [details] [diff] [review]
adb-exceptions.patch
Review of attachment 8994785 [details] [diff] [review]:
-----------------------------------------------------------------
::: testing/mochitest/runtests.py
@@ +65,5 @@
> from leaks import ShutdownLeaks, LSANLeaks
> from mochitest_options import (
> MochitestArgumentParser, build_obj, get_default_valgrind_suppression_files
> )
> +from mozdevice import ADBTimeoutError
I think runtests.py is otherwise independent of mozdevice or any other Android complications. Does this risk causing mozdevice import failures for folks running 'mach mochitest' locally in desktop-only configurations? Could we live without this change to runtests.py?
Attachment #8994785 -
Flags: review?(gbrown) → review+
![]() |
||
Comment 4•7 years ago
|
||
I took the precaution of pushing to try again and hit consistent robocop failures:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4359d495a651ffd0b1eb455622a3e72bb1058b1
but now I see you already pushed to try in comment 2 and did not hit this problem. Not sure what to make of that...
![]() |
||
Comment 5•7 years ago
|
||
Nevermind -- the robocop failures are unrelated to your changes.
https://bugzilla.mozilla.org/show_bug.cgi?id=1451513#c19
Assignee | ||
Comment 6•7 years ago
|
||
I've been trying to check if I have any problems running the desktop tests locally but recent nss changes I think have broken my ability to build. Bisecting that at the moment but I'll look into keeping the device related changes in the "remote" device oriented files.
Assignee | ||
Comment 7•7 years ago
|
||
Removes need for the import. See https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4252d8ee7addcfdb9609729c38d3203e9cebd8f&filter-tier=1&filter-tier=2&filter-tier=3&filter-searchStr=android-hw
I'll fold this into the previous patch if it is ok with you. I haven't been able to get a build locally but this shouldn't have any effect on non-android platforms.
(Un)fortunately, the new Pixel devices have been having issues but this allows us to test failure paths. The device issues should be resolved going forward I hope.
https://treeherder.mozilla.org/logviewer.html#?job_id=190316822&repo=try&lineNumber=2712 shows hitting the new code.
Other device issues:
https://treeherder.mozilla.org/logviewer.html#?job_id=190314772&repo=try&lineNumber=10
https://treeherder.mozilla.org/logviewer.html#?job_id=190314835&repo=try&lineNumber=1266
https://treeherder.mozilla.org/logviewer.html#?job_id=190316936&repo=try&lineNumber=3043
https://treeherder.mozilla.org/logviewer.html#?job_id=190316876&repo=try&lineNumber=10
https://treeherder.mozilla.org/logviewer.html#?job_id=190316873&repo=try&lineNumber=2019
Attachment #8995380 -
Flags: review?(gbrown)
Assignee | ||
Comment 8•7 years ago
|
||
fwiw, I finally got a build (must be a compiler error) and mach seems to work fine on reftests and mochitests.
![]() |
||
Comment 9•7 years ago
|
||
Comment on attachment 8995380 [details] [diff] [review]
followup.patch
Review of attachment 8995380 [details] [diff] [review]:
-----------------------------------------------------------------
Clever!
Attachment #8995380 -
Flags: review?(gbrown) → review+
Comment 10•7 years ago
|
||
Pushed by bclary@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/69d756618c48
Distinguish ADBTimeoutErrors from other exceptions in Android remote tests, r=gbrown.
Comment 11•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in
before you can comment on or make changes to this bug.
Description
•