Hey Brian and Nika, not entirely sure if you are the right people to ask, but I was hoping by asking you first, you can help me find a path forward.

We would like to enable HTTPS-First in Nightly, but facing something like ~700 test failures. While ultimately we need to address and look into those test failures we would like to annotate the failing tests so we can enable the feature in Nightly faster. Our thinking was to write a script which reads a list of failing tests and annotates the tests in the *.ini file in the form of:

 [browser_aboutNetError_xfo_iframe.js]
+prefs = dom.security.https_first=false

In the specific case of browser.ini files however we get the following error:
The 'prefs' key must be set in the DEFAULT section of a manifest. Fix the following manifests: browser/base/content/test/tabs/browser.ini

I know that for Fission we had to annotate a lot of tests in a similar fashion.

Any suggestions - what's the best way forward? Extend our test-infra which would allow pref flipping for individual tests?

For Fission, we ended up starting a second set of test runs (the "M-fis" tests), and adding skip-if = fission or fail-if = fission annotations to the test, as it isn't possible to enable or disable fission at runtime, and we wanted to continue testing the shipping configuration. We ended up taking a more similar approach to what you're suggesting when disabling the new BFCacheInParent pref, however because we're doing it on a per-test basis the pref is set at runtime by the test itself (e.g. https://searchfox.org/mozilla-central/rev/da25888c4495585c532640f0e5efad07b1037621/devtools/client/netmonitor/test/browser_net_charts-01.js#11-15). I don't remember there being any support to set prefs in the .ini file on a per-test basis for any of mochitest-{plain,chrome,browser-chrome}, just at a per-directory (and therefore per-run) basis but perhaps I'm wrong on that.

I imagine the "manually update each test to set the pref" approach probably won't work well for you if you're facing ~700 test failures though, and I don't know what the best way forward would be. CC :kmag who has much more experience with the mochitest framework than me & did the fission mochitest stuff for us & might have more insight.

IIRC we currently set those prefs & parse the test manifest from the python runner script, configuring the profile with the specified prefs, and using the same prefs for an entire execution of firefox, which lasts for a directory. If we wanted to support setting them on a per-test basis in the .ini file, I think we'd need to reflect that data somehow into the actual running firefox instance, and add support to mochitest. Alternatively I suppose we'd need to split up tests with custom prefs to run separately or something like that?

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #2)

Any suggestions - what's the best way forward? Extend our test-infra which would allow pref flipping for individual tests?

That won't work. The prefs key in the manifest changes the preference before browser startup, so setting it for an individual test would require running that test in its own session. Which would have other side-effects.

You'd need to call SpecialPowers.pushPrefEnv from the individual test if you want to change the preference just for it.

Andrew, Joel, may I ask for your thoughts. It seems cumbersome to explicitly annotate tests using SpecialPowers.pushPrefEnv. Probably the better strategy would be to extend our test runner to allow annotating individual tests within a corresponding *.ini file (see comment 0). I understand that running those tests within their own session is not possible/ideal, but I was wondering if we could extend our test runner to handle that in some form?

What's your take? Doable with reasonable effort?

I wonder if we could do something like add a tag on all the tests and then modify the test harness to check if a test has that tag, and if so call SpecialPowers.pushPrefEnv before entering the test. Not sure if that's viable and/or too hacky, but it would also give you a way to locally run all of the failing tests in a bunch with ./mach mochitest --tag fails-in-https-first.

The reason we can't add prefs to individual tests is that we only restart Firefox between each manifest, and some prefs need a restart to take effect. We didn't want to introduce the risk of developers assuming their prefs were being applied when in reality they weren't, so decided to play it safe and just say prefs need to be set at the manifest level. I don't think we'll ever

Brian's idea should work, though it might not be trivial as I don't think we pass tags into the JS side of the harness. I can at least point you towards some code if this is an avenue you'd like to explore (though I'm not super familiar with the JS side myself).

Another option would be to re-factor the failing tests into a new mochitest-no-https-first.ini manifest that gets included by the original one, though this seems equally cumbersome as using pushPrefEnv and more annoying for developers working with the tests.

(In reply to Andrew Halberstadt [:ahal] from comment #7)

Brian's idea should work, though it might not be trivial as I don't think we pass tags into the JS side of the harness. I can at least point you towards some code if this is an avenue you'd like to explore (though I'm not super familiar with the JS side myself).

Yeah, would you mind pointing to that? I wasn't sure of the viability to pass tags over but wouldn't hurt to look around a bit.

Sure!

1. The Python harness dumps a json object to file which contains all tests
2. This file path is passed to the JS side via url parameter.
3. It is then parsed here
4. These parsed objects are then used here

So I think we could A) make sure the JSON object at 1 contains tags, then B) ensure we propagate that to the test objects at 3, then C) figure out where to use this data at 4. I'm not sure if TestRunner.js is the right place to be setting prefs or not, we might also need to propagate these tags elsewhere.

If we go this route, it might make sense to allow attaching arbitrary metadata to tests to make this easier the next time.

The closest thing we've ever had to that was the uses-unsafe-cpows I added as a stepping stone to removing unsafe CPOW support: https://hg.mozilla.org/mozilla-central/rev/cf17754e8ab3b698ef04ec8f3d7b2f9e79cb45df

Nice, yeah that does exactly what I was thinking.

Thanks Andrew and Kris, that sounds like a viable path forward for https first. So either one of:

• the uses-unsafe-cpows thing (see consumer at https://hg.mozilla.org/mozilla-central/rev/727dba5569a7#l1.12) with a new key like fails-in-https-first, and teaching the test runner to set a pref in that case
• using a tag and propagating it down in a similar manner & having the test runner know about that specific tag to set the pref

I guess I don't have an informed opinion on which one is better. I'd lean towards a tag so that you could run them all together to i.e. see if you have any that have been fixed by having a patch where you comment out the pref flip, run the whole tag and then look for tests that succeeded to know that you can remove the tag.

OTOH it's going to be harder to write a script to actually tag the tests in the ini file since you'd need to handle cases where the test already has a tags key and you need to append a new one to that line in addition to cases where there's no tags key and you need to add a new line

So maybe it's easiest to copy the uses-unsafe-cpows pattern and then update your existing patch to find/replace prefs = dom.security.https_first=false with fails-in-https-first or whatever.

(In reply to Kris Maglione [:kmag] from comment #11)

The closest thing we've ever had to that was the uses-unsafe-cpows I added as a stepping stone to removing unsafe CPOW support: https://hg.mozilla.org/mozilla-central/rev/cf17754e8ab3b698ef04ec8f3d7b2f9e79cb45df

Thanks everyone for your input - I think the uses-unsafe-cpows approach could work well - I filed Bug 1720535 which would add some form of can-not-use-https-first annotation to mochitest harness.

Backed out for causing bustage in gecko decision task

Backout link: https://hg.mozilla.org/integration/autoland/rev/419529c60777ab242da8a5c6cc3c36c4f4f2f8a9

Push with failure

Failure log

(In reply to Sandor Molnar from comment #18)

Backed out for causing bustage in gecko decision task

Backout link: https://hg.mozilla.org/integration/autoland/rev/419529c60777ab242da8a5c6cc3c36c4f4f2f8a9

Push with failure

Failure log

Ah, this is a rebase problem - I'll fix that!

https://hg.mozilla.org/mozilla-central/rev/6cf55eca13ae