Closed Bug 1262866 Opened 9 years ago Closed 9 years ago

Can't run mochitests locally with --disable-crashreporter

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox48 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla48
Tracking Flags:
Tracking Status
firefox48 --- fixed

People

(Reporter: chmanchester, Assigned: chmanchester)

References

Details

Attachments

(1 file)

MozReview Request: Bug 1262866 - Move crashreport.sjs from browser.ini to TEST_HARNESS_FILES to prevent failure in --disable-crashrepoter builds. r=mshal
(deleted), text/x-review-board-request
mshal
: review+
ted
: review+
Details
We're requiring a manifest in the crashreporter get parsed so we know how to install a file in a neighboring directory, but that manifest's directory isn't traversed by the build system when the crashreporter is disabled. The quick fix is to traverse that test directory unconditionally (https://pastebin.mozilla.org/8866783), which is probably ok, but I need to think about whether a more general solution can avoid this hazard.
https://pastebin.mozilla.org/8866804 is the workaround for non-artifact builds.
Comment on attachment 8739217 [details] MozReview Request: Bug 1262866 - Move crashreport.sjs from browser.ini to TEST_HARNESS_FILES to prevent failure in --disable-crashrepoter builds. r=mshal https://reviewboard.mozilla.org/r/45121/#review42059
Attachment #8739217 - Flags: review?(mshal) → review+
We actually have a --disable-crashreporter build in automation, the ASan build. That patch caused it to start running the crashreporter tests, which as you might imagine do not actually succeed for a --disable-crashreporter build. Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/6cd73ef4a652798423cf0c95a621b16d986cda83
Yeah, this patch is not correct. The right fix would be to move crashreport.sjs to a manifest that's not behind --disable-crashreporter.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #6) > Yeah, this patch is not correct. The right fix would be to move > crashreport.sjs to a manifest that's not behind --disable-crashreporter. That's not quite going to work, because it will change the install destination for the file. We can make these tests skip-if = !crashreporter, or we can move this particular file to TEST_HARNESS_FILES in a moz.build that's not behind --disable-crashreporter.
Either of those are fine, although the latter is probably nicer because otherwise the manifest would have to not live near the tests, which is odd. Having crashreport.sjs in TEST_HARNESS_FILES in a common area seems sensible because it's shared for tests across the tree.
Comment on attachment 8739217 [details] MozReview Request: Bug 1262866 - Move crashreport.sjs from browser.ini to TEST_HARNESS_FILES to prevent failure in --disable-crashrepoter builds. r=mshal Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45121/diff/1-2/
Attachment #8739217 - Attachment description: MozReview Request: Bug 1262866 - Include toolkit/crashreporter/test/browser/browser.ini in a more central moz.build to avoid a failure when running tests against --disable-crashreporter builds. r=mshal → MozReview Request: Bug 1262866 - Move crashreport.sjs from browser.ini to TEST_HARNESS_FILES to prevent failure in --disable-crashrepoter builds. r=mshal
Attachment #8739217 - Flags: review?(ted)
Attachment #8739217 - Flags: review?(ted) → review+
Comment on attachment 8739217 [details] MozReview Request: Bug 1262866 - Move crashreport.sjs from browser.ini to TEST_HARNESS_FILES to prevent failure in --disable-crashrepoter builds. r=mshal https://reviewboard.mozilla.org/r/45121/#review42393 Works for me. (Probably would be nicer if it lived somewhere more generic, but whatever.)
https://hg.mozilla.org/mozilla-central/rev/bd86df4974b4
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: