Zac C (:zac) Reporter
	Description • 10 years ago

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.

	David Burns :automatedtester
	Comment 2 • 10 years ago

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?

	Zac C (:zac) Reporter
	Comment 3 • 10 years ago

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.

	Zac C (:zac) Reporter
	Comment 5 • 10 years ago

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

	Dave Hunt [:davehunt] [he/him] ⌚BST
	Comment 6 • 10 years ago

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.

	Zac C (:zac) Reporter
	Comment 7 • 10 years ago

OK, makes sense.

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

	Dave Hunt [:davehunt] [he/him] ⌚BST
	Comment 8 • 10 years ago

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

	Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me) Assignee
	Comment 9 • 9 years ago

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

	Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me) Assignee
	Comment 10 • 9 years ago

Bug 1040742: Only ignore expected exceptions; r?automatedtester

	David Burns :automatedtester
	Comment 11 • 9 years ago

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

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

	Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me) Assignee
	Comment 12 • 9 years ago

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

	Pulsebot
	Comment 13 • 9 years ago

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

	Carsten Book [:Tomcat]
	Comment 14 • 9 years ago

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