Closed Bug 1262866 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: chmanchester, Assigned: chmanchester)

References

Details

Attachments

(1 file)

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.
Duplicate of this bug: 1262626
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: 4 years ago
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.