remotereftest.py should print the actual exception if one occurred during dm.recordLogcat() or reftest.runTests()

RESOLVED FIXED in Firefox 17

Status

Testing
Reftest
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: emorley, Assigned: emorley)

Tracking

Trunk
mozilla19
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox17 fixed, firefox18 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
http://hg.mozilla.org/mozilla-central/file/ab099c9e1a09/layout/tools/reftest/remotereftest.py#l446

   439     try:
   440         cmdlineArgs = ["-reftest", manifest]
   441         if options.bootstrap:
   442             cmdlineArgs = []
   443         dm.recordLogcat()
   444         reftest.runTests(manifest, options, cmdlineArgs)
   445     except:
   446         print "TEST-UNEXPECTED-FAIL | | exception while running reftests"
(Assignee)

Updated

5 years ago
Summary: remotereftest.py shouldn't suppress exceptions during dm.recordLogcat() or reftest.runTests() → remotereftest.py should print the actual exception if one occurred during dm.recordLogcat() or reftest.runTests()
(Assignee)

Comment 1

5 years ago
Created attachment 678142 [details] [diff] [review]
Part 1: remotereftest.py should print the traceback

...and Automation Error seems more appropriate than TEST-UNEXPECTED-FAIL.

(at least from the try runs I've done so far, most have been device-manager related exceptions, eg: https://tbpl.mozilla.org/php/getParsedLog.php?id=16737220&tree=Try)
Attachment #678142 - Flags: review?(jmaher)
(Assignee)

Comment 2

5 years ago
Created attachment 678143 [details] [diff] [review]
Part 2: Make runTests() exception reporting consistent across test harnesses

No need to output sys.exc_info()[1] when traceback.print_exc() does the same; adds tracebacks where missing etc.
Attachment #678143 - Flags: review?(jmaher)
(Assignee)

Comment 3

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=de234f1e54fb
Comment on attachment 678142 [details] [diff] [review]
Part 1: remotereftest.py should print the traceback

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

please test this on try server for one of each OS.  adding a new python module could have odd consequences in our different versions of python.
Attachment #678142 - Flags: review?(jmaher) → review+
Comment on attachment 678143 [details] [diff] [review]
Part 2: Make runTests() exception reporting consistent across test harnesses

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

My biggest concern here is we are removing the info from the 'Automation Error' line which would make all exceptions generic from a parsing standpoint.  I see no technical problem with this approach and as this is coming from the guy who is making tbpl more streamlined, I will assume this is for the better.
Attachment #678143 - Flags: review?(jmaher) → review+
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4222b4c6a3e9
https://hg.mozilla.org/integration/mozilla-inbound/rev/267a4eb4b236

(In reply to Joel Maher (:jmaher) from comment #5)
> My biggest concern here is we are removing the info from the 'Automation
> Error' line which would make all exceptions generic from a parsing
> standpoint.  I see no technical problem with this approach and as this is
> coming from the guy who is making tbpl more streamlined, I will assume this
> is for the better.

Yeah, the exceptions I've seen on Try all have the appropriate prefixes :-)

eg:
{
DMError: Remote Device Error: unable to connect to 10.250.49.44 after 5 attempts
DMError: Automation Error: Timeout in command rmdr /mnt/sdcard/tests/profile
DMError: Remote Device Error: unable to connect to 10.250.51.55 after 5 attempts
Automation Error: error pushing file: Automation Error: Timeout in command push /mnt/sdcard/writetest 10378
Remote Device Error: unable to write to sdcard
}

(and nice that we can now see these :-D)
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/4222b4c6a3e9
https://hg.mozilla.org/mozilla-central/rev/267a4eb4b236
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
I was going to try to uplift this to Aurora, but there are some differences between the two and I didn't want to inadvertently break something. Ed, can we please do so?
Flags: needinfo?(bmo)
(Assignee)

Comment 9

5 years ago
(In reply to Ryan VanderMeulen from comment #8)
> I was going to try to uplift this to Aurora, but there are some differences
> between the two and I didn't want to inadvertently break something. Ed, can
> we please do so?

Sure :-)

https://hg.mozilla.org/releases/mozilla-aurora/rev/0460fcc8e2ef
https://hg.mozilla.org/releases/mozilla-aurora/rev/020af45119f4

https://hg.mozilla.org/releases/mozilla-beta/rev/8b5a77ed01e8
https://hg.mozilla.org/releases/mozilla-beta/rev/f49b7d8ed734
status-firefox17: --- → fixed
status-firefox18: --- → fixed
Flags: needinfo?(bmo)
You need to log in before you can comment on or make changes to this bug.