1297364 - A socket error in tearDown should also cause a test failure

Status: RESOLVED DUPLICATE of bug 1417051

(Remote Protocol :: Marionette, defect, P3)

Description

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

Shouldn't a socket error in tearDown cause a test failure or bustage? Right now we simply ignore all Marionette AND IOError exceptions:

https://dxr.mozilla.org/mozilla-central/source/testing/marionette/harness/marionette/marionette_test.py?q=file%3Amarionette_test.py&redirect_type=single#661

I would propose that we change that. Do you think differently?

One thing to note here would be that in case the socket doesn't exist anymore eg. due to a crash of Firefox, we should not try to send messages to the server.

Comment 1

[David Burns :automatedtester]

We are checking for a crash before we get into this. Once we are in that part getting near the try catch if the we don't really care if the browser has crashed since we have checked it already and logged the crash if it already happens. I suspect this code is there for when adb randomly dies on you killing the connection.

Comment 2

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

The problem here is not related to crashes or earlier socket disconnects, but actually to bug 1294456. When you call send_command() while due to some reason marionette-server is not responding, we will run into a socket timeout. Right now we completely hide this and mark the test job as success. I don't think that we should do that.

Comment 3

[David Burns :automatedtester]

let's mark it as a failure then.

Comment 4

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

The reference to the source code isn't valid anymore. As such here an updated link to how it looked at the time of bug creation:

https://hg.mozilla.org/releases/mozilla-esr52/file/tip/testing/marionette/harness/marionette_harness/marionette_test/testcases.py#l475

Nowadays the `tearDown` method looks like:

https://searchfox.org/mozilla-central/rev/0f4d50ff5211e8dd85e119ef683d04b63062ed90/testing/marionette/harness/marionette_harness/marionette_test/testcases.py#334

Nothing in here will prevent us from not failing a test. If `start_session()` fails an exception gets raised, which then bubbles up.

So due to my refactoring on bug 1417051 this has been fixed for Firefox 59.