Last Comment Bug 801652 - More difficult than it should be to diagnose problems when remote reftests/mochitests fail
: More difficult than it should be to diagnose problems when remote reftests/mo...
Status: RESOLVED FIXED
:
Product: Testing
Classification: Components
Component: General (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla19
Assigned To: William Lachance (:wlach)
:
Mentors:
: 803841 (view as bug list)
Depends on:
Blocks: 732325
  Show dependency treegraph
 
Reported: 2012-10-15 07:52 PDT by William Lachance (:wlach)
Modified: 2014-03-04 11:25 PST (History)
5 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Print logcat even on mochitest/reftest failure (5.24 KB, patch)
2012-10-15 07:57 PDT, William Lachance (:wlach)
jmaher: review+
Details | Diff | Splinter Review

Description William Lachance (:wlach) 2012-10-15 07:52:49 PDT
This is a bit of a followup to some stuff I found in bug 756440.

The chief problem is that we only print the logcat when the tests are successful... but it's often when we have *problems* running the test that the logcat is most interesting, as it might be able to give us clues as to what went wrong.
Comment 1 William Lachance (:wlach) 2012-10-15 07:57:17 PDT
Created attachment 671439 [details] [diff] [review]
Print logcat even on mochitest/reftest failure

This mostly just does what it says. There's some cleanup in the surrounding code, as well as a fix for a syntax error in remote mochitest which meant that the cleanup code would never be called at all in the case of error (we were calling self.cleanup, but we were not part of the mochitest object!)
Comment 2 Joel Maher ( :jmaher) 2012-10-15 08:11:17 PDT
Comment on attachment 671439 [details] [diff] [review]
Print logcat even on mochitest/reftest failure

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

just a few questions, otherwise this looks good.

::: layout/tools/reftest/remotereftest.py
@@ +13,4 @@
>  from runreftest import RefTest
>  from runreftest import ReftestOptions
>  from automation import Automation
> +import devicemanager, devicemanagerADB, devicemanagerSUT, devicemanager

this doesn't look right?  If this is intentional, please comment why we need to import devicemanager twice.

::: testing/mochitest/runtestsremote.py
@@ +479,5 @@
>                  print "TEST-UNEXPECTED-FAIL | %s | Exception caught while running robocop tests." % sys.exc_info()[1]
>                  mochitest.stopWebServer(options)
>                  mochitest.stopWebSocketServer(options)
>                  try:
> +                    mochitest.cleanup(None, options)

why mochitest.cleanup vs self.cleanup?

@@ +502,5 @@
> +            print "TEST-UNEXPECTED-FAIL | %s | Exception caught while running tests." % sys.exc_info()[1]
> +            mochitest.stopWebServer(options)
> +            mochitest.stopWebSocketServer(options)
> +            try:
> +                mochitest.cleanup(None, options)

why not self.cleanup?
Comment 3 William Lachance (:wlach) 2012-10-15 08:51:27 PDT
Thanks for the quick review.

(In reply to Joel Maher (:jmaher) from comment #2)
> Comment on attachment 671439 [details] [diff] [review]
> Print logcat even on mochitest/reftest failure
> 
> Review of attachment 671439 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> just a few questions, otherwise this looks good.
> 
> ::: layout/tools/reftest/remotereftest.py
> @@ +13,4 @@
> >  from runreftest import RefTest
> >  from runreftest import ReftestOptions
> >  from automation import Automation
> > +import devicemanager, devicemanagerADB, devicemanagerSUT, devicemanager
> 
> this doesn't look right?  If this is intentional, please comment why we need
> to import devicemanager twice.

Obviously just a mistake. :) Will fix before pushing.

> ::: testing/mochitest/runtestsremote.py
> @@ +479,5 @@
> >                  print "TEST-UNEXPECTED-FAIL | %s | Exception caught while running robocop tests." % sys.exc_info()[1]
> >                  mochitest.stopWebServer(options)
> >                  mochitest.stopWebSocketServer(options)
> >                  try:
> > +                    mochitest.cleanup(None, options)
> 
> why mochitest.cleanup vs self.cleanup?

As I mentioned, this code isn't part of the mochitest object, so we get an error thrown if we try to access it (this was hidden by the fact that the code in question was in a try...except block)
Comment 4 William Lachance (:wlach) 2012-10-24 10:35:35 PDT
Inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/47302ab431e9
Comment 5 William Lachance (:wlach) 2012-10-24 10:44:13 PDT
Forgot to fix jmaher's nit, pushed as a followup:

https://hg.mozilla.org/integration/mozilla-inbound/rev/fdbf73f25fba
Comment 7 :Ms2ger (⌚ UTC+1/+2) 2014-03-04 11:25:57 PST
*** Bug 803841 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.