220.28 KB, text/plain
4.40 KB, patch
|Details | Diff | Splinter Review|
46 bytes, text/x-github-pull-request
|Details | Review | Splinter Review|
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
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
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)
Created attachment 8515977 [details] [review] gaiatest-side pull request
Attachment #8515977 - Flags: review?(dave.hunt)
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.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.