Closed Bug 1027607 Opened 9 years ago Closed 8 years ago
gaia-ui-test crashes don't pass a valid test name to mozcrash
.check _for _crashes()
In other test jobs, the harness keeps track of the last test seen, so when there is a crash, it can pass this to mozcrash, so that the resultant error-line-with-top-frame-of-stack includes the test name. However for crashes during gaia-ui tests, the test name is 'base.py' or 'runner.py', rather than the test name. eg: https://tbpl.mozilla.org/php/getParsedLog.php?id=42022963&tree=B2g-Inbound#error0 22:09:45 INFO - TEST-START test_keyboard.py 22:10:02 INFO - test_keyboard_basic (test_keyboard.TestKeyboard) ... mozcrash INFO | Downloading symbols from: https://ftp-ssl.mozilla.org/pub/mozilla.org/b2g/tinderbox-builds/b2g-inbound-macosx64_gecko/1403151453/en-US/b2g-33.0a1.en-US.mac64.crashreporter-symbols.zip 22:10:17 ERROR - PROCESS-CRASH | base.py | application crashed [@ mozilla::gl::GLBlitTextureImageHelper::BlitTextureImage(...)] Where the expected output would be: 22:10:17 ERROR - PROCESS-CRASH | test_keyboard.py | application crashed [@ mozilla::gl::GLBlitTextureImageHelper::BlitTextureImage(...)]
The way we call mozcrash elsewhere : > mozcrash.check_for_crashes(local_dump_dir, symbolsPath, test_name=self.lastTestSeen) Whereas: http://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/b2ginstance.py#63 63 if mozcrash.check_for_crashes(local_dump_dir, self.symbols_path): IMO we should get rid of the footgun in mozcrash and make the test name mandatory, but I'll file a separate bug for that. Andrew, would you mind modifying the call here to pass the test name? Guessing we can just use http://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/runner/base.py#30 similar to how is done for the TEST-UNEXPECTED-FAILS via getInfo(): http://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/runner/base.py#226  http://mxr.mozilla.org/mozilla-central/search?string=mozcrash.check_for_crashes&tree=mozilla-central
Component: Gaia::UI Tests → Marionette
Product: Firefox OS → Testing
Summary: gaia-ui-test doesn't set testLastSeen correctly for crashes (instead uses 'base.py') → b2ginstance.py doesn't pass the test name when calling mozcrash.check_for_crashes()
Version: unspecified → Trunk
I'm about to land a patch that deletes b2ginstance.py.. so I vote we WONTFIX this :). The new patch should do the right thing.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
Resolution: WONTFIX → DUPLICATE
Bug 997244 unfortunately didn't fix this: https://tbpl.mozilla.org/php/getParsedLog.php?id=42123951&tree=Mozilla-Inbound > PROCESS-CRASH | runner.py | ... https://tbpl.mozilla.org/php/getParsedLog.php?id=42128624&tree=Mozilla-Inbound > PROCESS-CRASH | runner.py | ... Whilst the mozcrash.check_for_crashes() call is passes test_name: http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozrunner/mozrunner/base/runner.py#182 test_name must still be None, since we get the summaries above. Afaict the entry points to runner.py's check_for_crashes() are: http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/b2g_desktop.py#91 91 self.runner.check_for_crashes(test_name=self.last_test) http://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/marionette.py#695 695 if self.runner.check_for_crashes(): http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.py#184 184 self.runner.check_for_crashes() http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozrunner/mozrunner/base/device.py#107 107 BaseRunner.check_for_crashes(self, dump_directory=dump_dir, test_name=self.last_test) The marionette.py#695 and runtestsb2g.py#184 instances don't pass test_name. I think we need to: 1) Fix those to pass test name. 2) Remove the footgun from runner.py's check_for_crashes() by making the test name mandatory 3) Also make the test name mandatory in mozcrash, but I'll file another bug for that. May you take a look at #1 and #2? :-)
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Summary: b2ginstance.py doesn't pass the test name when calling mozcrash.check_for_crashes() → gaia-ui-test crashes don't pass a valid test name to mozcrash.check_for_crashes()
Yes, I can update those places. Though I'm kind of on the fence about making it mandatory. In theory mozrunner can be used for non-testing purposes and we may not have a test_name to pass in. But I guess in practice it is mostly for testing :p I propose we make it mandatory, but call it something else. I also propose we do #2 in the same bug as #3.
(In reply to Andrew Halberstadt [:ahal] from comment #5) > Yes, I can update those places. Thank you :-) > I propose we make it mandatory, but call it something else. I also propose > we do #2 in the same bug as #3. Agree to both; and re the name, I thought the same in bug 1028129 - will give us greater flexibility :-)
Chris, I'm sorry to add more log parsing in marionette while you are in the middle of trying to get rid of it. Depending on how close you are to finishing, we can either block on your patch, or land this for now and update it later.. up to you.
Assignee: nobody → ahalberstadt
Status: REOPENED → ASSIGNED
Attachment #8443469 - Flags: review?(cmanchester)
Comment on attachment 8443469 [details] [diff] [review] always pass in test_name in mozrunner.check_for_crashes Review of attachment 8443469 [details] [diff] [review]: ----------------------------------------------------------------- I say land it. This additional parsing will not break with my patch in bug 956739, but will once we have mozharness reading a raw log from stdout. When this happens, and bug 1023371 lands, associating a crash with the last test seen will be the purview of the structured log formatter.
Attachment #8443469 - Flags: review?(cmanchester) → review+
Thanks! I agree, that sounds like the best way forward. https://hg.mozilla.org/integration/mozilla-inbound/rev/154690942f4d
Add missing 'self': https://hg.mozilla.org/integration/mozilla-inbound/rev/3399263d2702
Backed out for now, since the new test name was 'automation' rather than the actual test name, or the previous 'base.py' (which caused fewer false positives). remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/8f7692bed181 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/2bac65d5860b
I don't suppose you know when you might be able to take another look at this? :-)
Dave, Can you add this to your queue ... IIRC your working on this space (or very near this space)
Ed, is this still a problem? Or did bug 1059248 fix it..
Assignee: ahalberstadt → nobody
Appears fixed :-) https://bugzilla.mozilla.org/show_bug.cgi?id=1056047#c6
Status: ASSIGNED → RESOLVED
Closed: 9 years ago → 8 years ago
No longer depends on: 1059248
Resolution: --- → DUPLICATE
Attachment #8443469 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.