Closed Bug 921502 Opened 6 years ago Closed 6 years ago

Startup cache test uses JS from external linkage

Categories

(Core :: XPCOM, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: benjamin, Assigned: benjamin)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Currently there is a test TestStartupCache.cpp. This test is a standalone C++ test (external linkage) which also uses JS code. Since we're going to stop exporting JS symbols from libxul, this is going to stop working.

I remember being the initial reviewer of this test, which IIRC couldn't be an xpcshell test because we didn't want to make nsIStartupCache scriptable for fear of being abused by addons.

Since then in bug 711297 we've added tests to ensure that various telemetry data surrounding the startup cache are correct. This currently involves mucking about with JSAPI from within the C++ test, which has caused pain several times because this is one of the few or the only test which does this.

I *think* that we could probably achieve better results and maintenance by doing something like putting the telemetry tests into a JS component which the test calls into.

It would be nice to move this test to be a gtest, but since it has lots of weird side-effects, I really don't want to do that without first fixing gtest to be able to run some tests in their own process.

I'm going to play with this, but bholley/nfroyd please let me know if you have thoughts about the best way to do this.
Assignee: nobody → benjamin
Attachment #811190 - Flags: review?(nfroyd)
Comment on attachment 811190 [details] [diff] [review]
Use a JS component to collect telemetry from the startup cache C++ test, instead of using raw JSAPI, r?nfroyd or bholley

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

Works for me.  That JS API stuff was a pain to write in the first place, glad it's gone.
Attachment #811190 - Flags: review?(nfroyd) → review+
Comment on attachment 811190 [details] [diff] [review]
Use a JS component to collect telemetry from the startup cache C++ test, instead of using raw JSAPI, r?nfroyd or bholley

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

I'm happy with nfroyd's review here.
Attachment #811190 - Flags: review?(bobbyholley+bmo)
https://hg.mozilla.org/mozilla-central/rev/8ee9a40710b7
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.