1259528 - Stop setting xpinstall.signatures.required in mochitest/reftest

Status: RESOLVED WONTFIX

(Testing :: General, defect)

Description

[Andrew Halberstadt [:ahal]]

Now that the tests all work with addon signing, we should stop disabling it on central/aurora to prevent future regressions at uplift time.

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: MozReview Request: Bug 1259528 - Stop setting xpinstall.signatures.required=False in mochitest and reftest, r?jgriffin

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

Comment 2

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8734460 [details]
MozReview Request: Bug 1259528 - Stop setting xpinstall.signatures.required=False in mochitest and reftest, r?jgriffin

https://reviewboard.mozilla.org/r/42263/#review38821

Comment 3

[Andrew Halberstadt [:ahal]]

Looks like there are some tests that fail when we remove the pref:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7815e7ab404b

They do however pass when addon signing is enforced by the build though.. maybe they're being skip-if'ed. I'll try turning the pref back on just for these particular tests.

Comment 4

[Andrew Halberstadt [:ahal]]

Quick update:

It seems the xpinstall.signatures.required needs a restart before it takes effect. At least, setting this pref at the start of the tests that need it didn't seem to do anything. The problematic tests are basically any ones that have skip-if = require_signing in their manifest:
https://dxr.mozilla.org/mozilla-central/search?q=skip-if+%3D+require_signing&redirect=true&case=false

The reason they fail is that signing isn't enforced by the build, but the pref to override signing is no longer being set.  We may need to create a special job just for these tests :/.

Comment 5

[Kim Moir [:kmoir] ET]

ahal: is this still a valid issue?  I'm trying to close out all the bugs regarding addon signing now that the preference is enabled on beta

Comment 6

[Andrew Halberstadt [:ahal]]

Yes, this one is still a valid issue. It's been in the back of my mind for awhile, but I've been neglecting it a bit in favour of some other projects.

I think we'll need to split out any tests that require signing disabled into their own jobs on treeherder. It isn't terribly difficult to do, but likely a week or two worth of work. I'll talk to jgriffin at our next 1on1 and see if I can slot some time for this or if there is anyone else around to tackle it. Thanks for going through all these bugs!

Comment 7

[Kim Moir [:kmoir] ET]

Is this something that should block bug 1233200 given that we already have signing enabled on beta?

Comment 8

[Andrew Halberstadt [:ahal]]

It doesn't really block the addons team or their effort to enforce signing directly. It's more us (ateam and releng) that this will inconvenience. As long as this isn't fixed, developers could inadvertently land new unsigned addons on central and we wouldn't notice until the uplift to beta.

So it depends on how we're using bug 1233200 I guess.. I'll let you decide whether to leave it blocking or not, I'm happy either way.

Comment 10

[Kim Moir [:kmoir] ET]

:ahal do you have an update on the status of this bug after your discussions with jgriffin in comment 6 above?

Comment 11

[Andrew Halberstadt [:ahal]]

Yeah sorry I didn't update the bug. I was supposed to keep working on my current deliverables until this bubbled up in priority a bit more. Maybe that time is now though. Is this causing trouble at every uplift?

Comment 12

[Kim Moir [:kmoir] ET]

It didn't cause a problem on the last uplift.  Coop was asking in our 1x1 when this was going to be implemented given the probability of it causing a problem in the future.

Comment 13

[Andrew Halberstadt [:ahal]]

I'm not sure if this is still something that needs to be fixed or not, but it's been a couple years and nothing seems to have blown up. I'm going to WONTFIX for now and we can either re-open or file a new issue when/if someone runs into this.