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

RESOLVED FIXED in mozilla36

Status

Testing
Marionette
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: zac, Assigned: zac)

Tracking

unspecified
mozilla36
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

3 years ago
Created attachment 8513493 [details]
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
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

Updated

3 years ago
Duplicate of this bug: 1092170
(Assignee)

Comment 3

3 years ago
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.
Assignee: nobody → zcampbell
(Assignee)

Comment 4

3 years ago
Created attachment 8515970 [details] [diff] [review]
bug_1090962.patch

Try:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ca819c387ecd
Attachment #8515970 - Flags: review?(dave.hunt)
(Assignee)

Comment 5

3 years ago
Created attachment 8515977 [details] [review]
gaiatest-side pull request
Attachment #8515977 - Flags: review?(dave.hunt)
(Assignee)

Updated

3 years ago
Attachment #8515970 - Flags: feedback?(cmanchester)
Comment on attachment 8515970 [details] [diff] [review]
bug_1090962.patch

Looks good, thanks!
Attachment #8515970 - Flags: review?(dave.hunt) → review+

Updated

3 years ago
Attachment #8515977 - Flags: review?(dave.hunt) → review+
Comment on attachment 8515970 [details] [diff] [review]
bug_1090962.patch

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

Looks good to me, thanks!
Attachment #8515970 - Flags: feedback?(cmanchester) → feedback+
(Assignee)

Comment 8

3 years ago
Thanks Dave and Chris.
Keywords: checkin-needed
(Assignee)

Comment 9

3 years ago
Gaia side merged:
https://github.com/mozilla-b2g/gaia/commit/488c347e9e8b8a6a0fdce0b2c8e37d8c1e8bc779
https://hg.mozilla.org/integration/mozilla-inbound/rev/83312696fd68
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/83312696fd68
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.