1040742 - MarionetteTestCase should not try to delete_session/call the server if a crash has occurred

Status: RESOLVED FIXED

(Testing :: Marionette Client and Harness, defect)

Description

[Zac C (:zac)]

If a crash has occurred in b2g and tearDown has been called then the tearDown will fail because the marionette server is not up.

Seeing as MarionetteTestCase can/will detect when a crash has occurred, we can use this to conditionally execute the remainder of MarionetteTestCase.tearDown.

Put simply, if a crash has occurred we should not make any more calls to the marionette server and cleanup, but still do cleanup inside MarionetteTestCase where required.

Comment 2

[David Burns :automatedtester]

What is the error that you get? Looking at the code it should handle there being no marionette server to speak to.

Do you know if there is a way to guarantee a crash?

Comment 3

[Zac C (:zac)]

Just running this again, I think some recent changes to mozrunner and so forth have changed the way test failures are reported so I don't see tearDown failures reported any more. However the code does still exist, here:

http://dxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/marionette_test.py#548

you can loosely replicate a crash by doing `adb shell stop b2g` while a test is running.

Running with a known crasher I can see it may affect logging too as TEST-END is not executed when the crash occurs.

Some logging depends on this so it may be two bugs here; one to remove marionette calls from tearDown and another to log correctly even after a crash.

Comment 4

[Zac C (:zac)]

Attachment: bug_1040742.diff

Comment 5

[Zac C (:zac)]

NB - this won't do anything because check_for_crashes doesn't execute when testing against device.

Comment 6

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8500964 [details] [diff] [review]
bug_1040742.diff

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

::: testing/marionette/client/marionette/marionette_test.py
@@ +542,5 @@
>          self.marionette.execute_script("log('TEST-START: %s:%s')" %
>                                         (self.filepath.replace('\\', '\\\\'), self.methodName))
>  
>      def tearDown(self):
> +        if not self.marionette.check_for_crash():

This will only return true if it detects a crash in this call. If a crash has previously been detected then this may return false. We now keep track of crashes in mozrunner (see bug 1075647) but we'll need a mozrunner release and bug 1038868 fixed before we can revisit this patch.

Comment 7

[Zac C (:zac)]

OK, makes sense.

If we keep track of crashes in mozrunner does it make the check_for_crashes method redundant?

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Zac C (:zac) from comment #7)
> If we keep track of crashes in mozrunner does it make the check_for_crashes
> method redundant?

No, we still need to check for crashes. The change in mozrunner just keeps track of crashes detected when calling check_for_crashes.

Comment 9

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

It looks like this got fixed in Bug 1141519, but we might want to catch a specific exception when logging test-end fails.

Comment 10

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Attachment: MozReview Request: Bug 1040742: Only ignore expected exceptions; r?automatedtester

Bug 1040742: Only ignore expected exceptions; r?automatedtester

Comment 11

[David Burns :automatedtester]

Comment on attachment 8669995 [details]
MozReview Request: Bug 1040742: Only ignore expected exceptions; r?automatedtester

https://reviewboard.mozilla.org/r/21313/#review19221

Comment 12

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2a7b044cfb46

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/373d69235038

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/373d69235038

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Moving bugs for Marionette client due to component changes.