Closed Bug 1059248 Opened 8 years ago Closed 8 years ago

[mozrunner] Accept a test name when checking for crashes in DeviceRunner

Categories

(Testing :: Mozbase, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla34

People

(Reporter: davehunt, Assigned: davehunt)

References

Details

Attachments

(1 file, 1 obsolete file)

Currently mozrunner's DeviceRunner tries to determine the last test name by looking at the process output, however this is not working at least for physical devices. We should allow the test harness to specify the test name when we check for crashes.
Attachment #8479851 - Flags: review?(ahalberstadt)
Blocks: 1038868
Comment on attachment 8479851 [details] [diff] [review]
Accept a test name when checking for crashes. v1.0

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

Thanks this looks good, but r- for now because this will break desktop marionette.

::: testing/marionette/client/marionette/marionette.py
@@ +729,5 @@
>                  returncode = self.emulator.proc.returncode
>                  name = 'emulator'
>                  crashed = True
>          elif self.instance:
> +            if self.instance.check_for_crashes(test_name=self.test_name):

I don't think 'instance.check_for_crashes' has a test_name either:
http://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/geckoinstance.py#96

There's a lot of unnecessary abstraction going on here. Maybe we can just change this to instance.runner.check_for_crashes and then delete instance.check_for_crashes (I don't know if it is used anywhere else though).
Attachment #8479851 - Flags: review?(ahalberstadt) → review-
I've removed the check_for_crashes from geckoinstance.py as suggested. Running a more expansive try to see if anything breaks, but I couldn't find anywhere else this was being called.

Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f4cf21d66956
Attachment #8479851 - Attachment is obsolete: true
Attachment #8479979 - Flags: review?(ahalberstadt)
Comment on attachment 8479979 [details] [diff] [review]
Accept a test name when checking for crashes. v1.1

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

Lgtm, thanks!
Attachment #8479979 - Flags: review?(ahalberstadt) → review+
Depends on: 1059404
Flags: needinfo?(dave.hunt)
Blocks: 1059404
No longer depends on: 1059404
Flags: needinfo?(dave.hunt)
Keywords: checkin-needed
Blocks: 1027607
https://hg.mozilla.org/mozilla-central/rev/97d41094b879
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
No longer blocks: 1027607
Duplicate of this bug: 1027607
You need to log in before you can comment on or make changes to this bug.