Closed Bug 1477807 Opened 4 years ago Closed 4 years ago

Distinguish ADBTimeoutErrors from other exceptions in Android remote tests


(Testing :: General, defect)

Not set


(firefox63 fixed)

Tracking Status
firefox63 --- fixed


(Reporter: bc, Assigned: bc)




(2 files, 1 obsolete file)

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,



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.



A full run of android tests with this patch on try:

Attached patch adb-exceptions.patch (obsolete) — Splinter Review
Attachment #8994301 - Flags: review?(gbrown)
Attachment #8994301 - Attachment is obsolete: true
Attachment #8994301 - Flags: review?(gbrown)
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.
Attachment #8994785 - Flags: review?(gbrown)
Comment on attachment 8994785 [details] [diff] [review]

Review of attachment 8994785 [details] [diff] [review]:

::: testing/mochitest/
@@ +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 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
Attachment #8994785 - Flags: review?(gbrown) → review+
I took the precaution of pushing to try again and hit consistent robocop failures:

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

Review of attachment 8995380 [details] [diff] [review]:

Attachment #8995380 - Flags: review?(gbrown) → review+
Pushed by
Distinguish ADBTimeoutErrors from other exceptions in Android remote tests, r=gbrown.
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Blocks: 1483695
See Also: → 1533445
You need to log in before you can comment on or make changes to this bug.