1090962 - If a test is skipped before a session is created, reporting mixin's gather_debug will fail when it tries to use the Marionette session

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Zac C (:zac)]

Attachment: gather_debug_no_session.txt

STR: 
1. Test is skipped in the setUp, but before MarionetteTestCase.setUp() has been called, 
2. MarionetteTestCase.tearDown() will then try to use a marionette session.


The logs are polluted with Marionette exceptions ("Please start a session")

I can't see any ill effects in this scenario but the assumption that a session is running should probably be avoided.

Attachment is of the polluted log.

The test that is causing that error in the log is:

https://github.com/mozilla-b2g/gaia/blob/bbcdd15c4080a14fc6f84310a1583ea721966cb6/tests/python/gaia-ui-tests/gaiatest/tests/functional/email/test_IMAP_email_notification.py#L20

Comment 1

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

Zac and I just talked on IRC about this. I don't think the issue is new, but recent changes have made it noisier. Basically we're trying to gather debug if the test does not pass (so that includes tests that are skipped if not done by manifest). It will have no harm because the gathering of debug is already in a try/except, but we should reduce the noise.

We could exclude skipped tests from gathering debug (there is little point as these will likely have no useful information), but it would also make sense to check we have a Marionette session before we attempt to gather any debug. Here are the places this would need to be fixed:

http://hg.mozilla.org/mozilla-central/file/80e18ff7c7b2/testing/marionette/client/marionette/runner/mixins/reporting.py#l244
http://hg.mozilla.org/mozilla-central/file/80e18ff7c7b2/testing/marionette/client/marionette/runner/base.py#l498
https://github.com/mozilla-b2g/gaia/blob/872f8d3fe6e5f6b6101ff4ce5e895fea2aa8c451/tests/python/gaia-ui-tests/gaiatest/runtests.py#L58

Comment 3

[Zac C (:zac)]

Possibly in the future the reason for the skip could be useful in the debug so I think we should still debug the test even if it's empty.

At the very least checking for the marionette session will remove the console noise so I'll start with a patch for that and we can go from there.

Comment 4

[Zac C (:zac)]

Attachment: bug_1090962.patch

Try:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ca819c387ecd

Comment 5

[Zac C (:zac)]

Attachment: gaiatest-side pull request

Comment 6

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

Comment on attachment 8515970 [details] [diff] [review]
bug_1090962.patch

Looks good, thanks!

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8515970 [details] [diff] [review]
bug_1090962.patch

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

Looks good to me, thanks!

Comment 8

[Zac C (:zac)]

Thanks Dave and Chris.

Comment 9

[Zac C (:zac)]

Gaia side merged:
https://github.com/mozilla-b2g/gaia/commit/488c347e9e8b8a6a0fdce0b2c8e37d8c1e8bc779

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/83312696fd68

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/83312696fd68