Distinguish ADBTimeoutErrors from other exceptions in Android remote tests

RESOLVED FIXED in Firefox 63

Status

defect
RESOLVED FIXED
11 months ago
4 months ago

People

(Reporter: bc, Assigned: bc)

Tracking

(Blocks 1 bug)

Trunk
mozilla63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Assignee

Description

11 months ago
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

11 months ago
Posted patch adb-exceptions.patch (obsolete) — Splinter Review
Attachment #8994301 - Flags: review?(gbrown)
Assignee

Updated

11 months ago
Attachment #8994301 - Attachment is obsolete: true
Attachment #8994301 - Flags: review?(gbrown)
Assignee

Comment 2

11 months 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 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+
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...
Nevermind -- the robocop failures are unrelated to your changes.

https://bugzilla.mozilla.org/show_bug.cgi?id=1451513#c19
Assignee

Comment 6

11 months 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 8

11 months ago
fwiw, I finally got a build (must be a compiler error) and mach seems to work fine on reftests and mochitests.
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

11 months 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

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/69d756618c48
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Assignee

Updated

6 months ago
Blocks: 1483695
You need to log in before you can comment on or make changes to this bug.