Closed Bug 1167986 Opened 10 years ago Closed 9 years ago

TEST-UNEXPECTED-FAIL | xpcshell-unpack.ini:toolkit/mozapps/extensions/test/xpcshell/test_signed_install.js (and signing failures)

Categories

(Toolkit :: Add-ons Manager, defect)

Product:
Toolkit
Component:
Add-ons Manager
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla43
Tracking Flags:
Tracking Status
firefox41 --- fixed
firefox42 --- fixed
firefox43 --- fixed
firefox-esr38 --- unaffected

People

(Reporter: mkmelin, Assigned: Fallen)

References

Details

(Keywords: intermittent-failure, regression)

Attachments

(1 file, 3 obsolete files)

Disable tests with MOZ_ADDON_SIGNING=0 - v2
(deleted), patch
Fallen
: review+
ritu
: approval-mozilla-aurora+
ritu
: approval-mozilla-beta+
Details | Diff | Splinter Review
Timing is a little too convenient for this not to be from bug 1164168. 05:09:44 WARNING - TEST-UNEXPECTED-FAIL | xpcshell-unpack.ini:toolkit/mozapps/extensions/test/xpcshell/test_signed_install.js | - 6 == 4 05:09:44 INFO - /builds/slave/test/build/tests/xpcshell/tests/toolkit/mozapps/extensions/test/xpcshell/test_signed_install.js:test_install_broken:110 05:09:44 INFO - self-hosted:next:624 05:09:44 INFO - _run_next_test@/builds/slave/test/build/tests/xpcshell/head.js:1440:9 05:09:44 INFO - do_execute_soon/<.run@/builds/slave/test/build/tests/xpcshell/head.js:653:9 05:09:44 INFO - _do_main@/builds/slave/test/build/tests/xpcshell/head.js:207:5 05:09:44 INFO - _execute_test@/builds/slave/test/build/tests/xpcshell/head.js:513:5 05:09:44 INFO - @-e:1:1 05:09:44 INFO - exiting test 05:09:44 INFO - Unexpected exception 2147500036 http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/2015/05/2015-05-24-03-02-20-comm-central/comm-central_ubuntu32_vm_test-xpcshell-bm02-tests1-linux32-build58.txt.gz
Summary: xpcshell-unpack.ini:toolkit/mozapps/extensions/test/xpcshell/test_signed_install.js (and signing failures) → TEST-UNEXPECTED-FAIL | xpcshell-unpack.ini:toolkit/mozapps/extensions/test/xpcshell/test_signed_install.js (and signing failures)
Attached patch bug1167986_signing_test_failures.patch (obsolete) (deleted) — — Splinter Review
Might this be it? https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d193432e52b0
Assignee: nobody → mkmelin+mozilla
Comment on attachment 8609897 [details] [diff] [review] bug1167986_signing_test_failures.patch Review of attachment 8609897 [details] [diff] [review]: ----------------------------------------------------------------- Seems try was happy
Attachment #8609897 - Flags: review?(standard8)
Attachment #8609897 - Flags: review?(standard8) → review?(Pidgeot18)
My understanding is that we were not going to require addon signing in Thunderbird. Doesn't this change enable required addon signing?
Comment on attachment 8609897 [details] [diff] [review] bug1167986_signing_test_failures.patch Review of attachment 8609897 [details] [diff] [review]: ----------------------------------------------------------------- Yeah this is not what you want. You want to disable the test_signed_* tests when MOZ_ADDON_SIGNING isn't set. Not sure how to do that
Attachment #8609897 - Flags: review?(Pidgeot18) → review-
(In reply to Kent James (:rkent) from comment #10) > My understanding is that we were not going to require addon signing in > Thunderbird. Doesn't this change enable required addon signing? Yes for official builds. I don't think we ever discussed more than briefly. But I don't see why we wouldn't follow firefox's policy.
status-thunderbird40: --- → affected
status-thunderbird41: --- → affected
(In reply to Magnus Melin from comment #12) > (In reply to Kent James (:rkent) from comment #10) > > My understanding is that we were not going to require addon signing in > > Thunderbird. Doesn't this change enable required addon signing? > > Yes for official builds. > I don't think we ever discussed more than briefly. But I don't see why we > wouldn't follow firefox's policy. We can discuss that, but the decision should not be buried in a bug that is purportedly about stopping a test failure.
Depends on: 1168571
Fair enough, filed bug 1168571. Frankly I didn't think that'd be a big thing - we change a lot of things all the times to align with firefox and avoid test failures. Given that it's a very contained patch I don't think there's much point in holding it up and having the tree all red while discussing. It's a pity all the config variables aren't available to do skips on for xpcxhell tests. If we want to disable the tests we probably just have to have them "skip-if = appname == "thunderbird"".
I don't understand the details, but can we disable this test or something? This shows up in my local |make xpcshell-tests| and I am not sure if I can ignore them, and was about to submit a report when I found this bugzilla entry. It seems that it boils down to enable/disable check on the signing of add-on. Why not then we create one signed dummy add-on and non-signed dummy add-on and see if TB recognizes the signed add-on as such and non-signed dummy add-on as such. (Higher-level policy decision can be dealt with later?) I mean, we simply check the low-level mechanism that supports such a policy decision. TIA
Hi, I noticed a syntactic error in Bug 1178041 SyntaxError: redefining arguments is deprecated _tests/xpcshell/toolkit/mozapps/extensions/test/xpcshell/test_signed_migrate.js I wonder if this has anything to do with this bug. (I mean the efforts to clean up the issue may not have worked due to the syntax error. The new bug 1178041 seems to be rather new.) TIA
I don't think so. This bug would be solved by bug 1168571.
Attached patch Disable affected tests for thunderbird (obsolete) (deleted) — — Splinter Review
Magnus, can we please disable these failing tests?
Attachment #8644155 - Flags: feedback?(mkmelin+mozilla)
I think we should rather do what Dave suggested, disable the tests when MOZ_ADDON_SIGNING=0 and not specific for Thunderbird. If we can't get this flag into mozinfo, I bet we could omit the whole test manifest for addon signing tests in moz.build.
Attached patch Disable tests with MOZ_ADDON_SIGNING=0 (obsolete) (deleted) — — Splinter Review
Untested, but this should do it. Here is the try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=de62dfe1fdad
Assignee: mkmelin+mozilla → philipp
Attachment #8609897 - Attachment is obsolete: true
Attachment #8644155 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8644155 - Flags: feedback?(mkmelin+mozilla)
Attachment #8650352 - Flags: review?(dtownsend)
Comment on attachment 8650352 [details] [diff] [review] Disable tests with MOZ_ADDON_SIGNING=0 Review of attachment 8650352 [details] [diff] [review]: ----------------------------------------------------------------- Looks ok to me but I'd want gps or some build person to sign off on the mozinfo change ::: toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini @@ +245,2 @@ > [test_signed_inject.js] > +skip-if = !addon_signing Could use run-if here
Attachment #8650352 - Flags: review?(gps)
Attachment #8650352 - Flags: review?(dtownsend)
Attachment #8650352 - Flags: feedback+
Attachment #8650352 - Flags: review?(gps) → review+
The c-c try run didn't work because I forgot the --apply-patches option, but as I am pretty sure this will do it and the m-c try run works, I'm pushing this now. Will request backports once c-c is ok.
Ah damn, forgot the review comment :-/ Extra push coming up
https://hg.mozilla.org/mozilla-central/rev/a42aeadfced4 https://hg.mozilla.org/mozilla-central/rev/ff478557098f
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox43: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 43.0
m-c doesn't have any failures related to this bug and c-c no longer runs the tests as expected. Here is the combined patch for backporting, I'd like to have these on beta/aurora to also fix test failures there. Approval Request Comment [Feature/regressing bug #]: addon signing [User impact if declined]: comm-beta/aurora xpcshell tests fail [Describe test coverage new/current, TreeHerder]: c-c no longer has previous signing failures: https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=95e56121a3ff m-c works: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=ba43a48d3c52 [Risks and why]: none, aside from maybe the test not running when it should. [String/UUID change made/needed]: none
Attachment #8650352 - Attachment is obsolete: true
status-firefox43: fixed → ---
Attachment #8651938 - Flags: review+
Attachment #8651938 - Flags: approval-mozilla-beta?
Attachment #8651938 - Flags: approval-mozilla-aurora?
Ryan, I have no idea how you set status-firefox43:fixed, for me only the Thunderbird flags show up. I guess this is also the reason it was reset in the last comment. Bug in bugzilla, or edge case?
status-thunderbird43: --- → fixed
Flags: needinfo?(ryanvm)
Bugherder does it automatically. You don't think we manually mark bugs after merging, do you? :P
Flags: needinfo?(ryanvm)
BTW, why is this filed under Thunderbird when it's a Toolkit issue? I can pretty much guarantee you that the uplift queries aren't going to see this when it's approved given that...
status-thunderbird42: --- → affected
I'm liking bugherder, didn't know about that one :) This was initially a tb-only failure, but it turned out to be a Toolkit bug to be fixed and we forgot to do that. Moving now to make it more visible.
status-thunderbird40: affected → ---
status-thunderbird41: affected → ---
status-thunderbird42: affected → ---
status-thunderbird43: fixed → ---
Component: General → Add-ons Manager
Product: Thunderbird → Toolkit
Target Milestone: Thunderbird 43.0 → mozilla43
Comment on attachment 8651938 [details] [diff] [review] Disable tests with MOZ_ADDON_SIGNING=0 - v2 Seems like a test only patch. Let's uplift to Aurora42 and Beta41.
Attachment #8651938 - Flags: approval-mozilla-beta?
Attachment #8651938 - Flags: approval-mozilla-beta+
Attachment #8651938 - Flags: approval-mozilla-aurora?
Attachment #8651938 - Flags: approval-mozilla-aurora+
BTW, typically the checking comment should be summarizing what the patch is doing rather than restating the problem being fixed (especially when throwing up things like TEST-UNEXPECTED-FAIL which can trip up the Treeherder log parser too). https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#Commit_Message_Conventions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: