801652 - More difficult than it should be to diagnose problems when remote reftests/mochitests fail

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[William Lachance (:wlach)]

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

Attachment: 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 ) (UTC -8)]

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

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

Inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/47302ab431e9

Comment 5

[William Lachance (:wlach)]

Forgot to fix jmaher's nit, pushed as a followup:

https://hg.mozilla.org/integration/mozilla-inbound/rev/fdbf73f25fba

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/47302ab431e9
https://hg.mozilla.org/mozilla-central/rev/fdbf73f25fba