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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Chris Manchester (limited bugmail, email directly)]

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.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

https://pastebin.mozilla.org/8866804 is the workaround for non-artifact builds.

Comment 2

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1262866 - Move crashreport.sjs from browser.ini to TEST_HARNESS_FILES to prevent failure in --disable-crashrepoter builds. r=mshal

Review commit: https://reviewboard.mozilla.org/r/45121/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45121/

Comment 3

[Michael Shal [:mshal]]

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

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/76411e442ff3

Comment 5

[Phil Ringnalda (:philor)]

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

Comment 6

[(not currently active) Ted Mielczarek]

Yeah, this patch is not correct. The right fix would be to move crashreport.sjs to a manifest that's not behind --disable-crashreporter.

Comment 7

[Chris Manchester (limited bugmail, email directly)]

(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.

Comment 8

[(not currently active) Ted Mielczarek]

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 10

[Chris Manchester (limited bugmail, email directly)]

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/

Comment 11

[(not currently active) Ted Mielczarek]

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.)

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bd86df4974b4

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bd86df4974b4