Closed Bug 1090962 Opened 5 years ago Closed 5 years ago

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

Categories

(Testing :: Marionette, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla36

People

(Reporter: zcampbell, Assigned: zcampbell)

References

Details

Attachments

(3 files)

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
Duplicate of this bug: 1092170
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
Attachment #8515977 - Flags: review?(dave.hunt)
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+
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+
Thanks Dave and Chris.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/83312696fd68
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.