Closed
Bug 1136892
Opened 10 years ago
Closed 9 years ago
Split add-on manager xpcshell and mochitest-browser tests into their own test suites
Categories
(Testing :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mossop, Assigned: jgriffin)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
chmanchester
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
ahal
:
review+
|
Details | Diff | Splinter Review |
The tentative plan for bug 1133838 involves us running add-on manager tests on the non-branded beta and release builds instead of the real beta and release builds. This probably means moving these tests into their own test suites. We have both xpcshell and mochitest-browser tests. There is also a single mochitest test that uses an XPI but it might make more sense to just sign that XPI rather than splitting that off into its own suite.
Assignee | ||
Updated•9 years ago
|
Component: General Automation → General
Product: Release Engineering → Testing
QA Contact: catlee
Version: other → Trunk
Assignee | ||
Comment 1•9 years ago
|
||
There are also some addons tests in mochitest-chrome and a single addon test (dom/apps/tests/test_app_addons.html) in mochitest-plain; would we want to split these out as well?
Assignee | ||
Comment 2•9 years ago
|
||
The number of addon tests (at least those with 'addon' in the path) for xpcshell is pretty small:
toolkit/mozapps/extensions/test/xpcshell/test_no_addons.js
services/sync/tests/unit/test_addons_engine.js
services/sync/tests/unit/test_addons_store.js
services/healthreport/tests/xpcshell/test_provider_addons.js
services/sync/tests/unit/test_addons_reconciler.js
services/sync/tests/unit/test_addons_tracker.js
Ideally, we'd only split this out on the 'allow-unsigned-addons' build and just run them inside the main xpcshell job on the other build.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jgriffin
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•9 years ago
|
||
We may want to convert this to a subsuite, depending on the timing of getting signed addons available to the tests, but we can start with just a tag.
Attachment #8635560 -
Flags: review?(cmanchester)
Reporter | ||
Comment 4•9 years ago
|
||
(In reply to Jonathan Griffin (:jgriffin) from comment #1)
> There are also some addons tests in mochitest-chrome and a single addon test
> (dom/apps/tests/test_app_addons.html) in mochitest-plain; would we want to
> split these out as well?
I'd argue no, for a few tests like that we can probably just check signed versions into the tree.
(In reply to Jonathan Griffin (:jgriffin) from comment #2)
> The number of addon tests (at least those with 'addon' in the path) for
> xpcshell is pretty small:
This really isn't a good way to check whether a test uses an add-on. Almost every single test in toolkit/mozapps/extensions/test/xpcshell uses one or more add-ons, some generated on the fly by JS code, some generated at build time from toolkit/mozapps/extensions/test/addons, some checked in under
toolkit/mozapps/extensions/test/xpcshell/data. Likewise toolkit/mozapps/extensions/test/xpinstall almost all use add-ons.
Assignee | ||
Comment 5•9 years ago
|
||
> This really isn't a good way to check whether a test uses an add-on.
Oh I agree. I was approaching this from the angle of, "what are the smallest set of tests that we want to run on the unsigned-addons-allowed build", once we have both of them running.
What is the actual goal here? To identify all of the tests which use addons that will need build-time signing?
Comment 6•9 years ago
|
||
Comment on attachment 8635560 [details] [diff] [review]
Create an xpcshell-addons tag for running addon-specific xpcshell tests,
Review of attachment 8635560 [details] [diff] [review]:
-----------------------------------------------------------------
::: services/healthreport/tests/xpcshell/xpcshell.ini
@@ +7,5 @@
> [test_profile.js]
> [test_healthreporter.js]
> [test_provider_addons.js]
> skip-if = buildapp == 'mulet'
> +tags = addons
Adding a tag doesn't exclude the test when running without a tag specified, I think we need to do something like mochitest does with subsuites.
Attachment #8635560 -
Flags: review?(cmanchester)
Assignee | ||
Comment 7•9 years ago
|
||
That was actually my intention; one one of the builds, we (may want to) only run the addons tests; on the other build, we'll run them all without splitting apart the addons tests.
But I'll wait for a response to comment #5 before changing or reflagging for review.
Comment 8•9 years ago
|
||
>> What is the actual goal here? To identify all of the tests which use addons that will need build-time signing?
Yes. We need to identify the tests that we can run on the builds where add on signing is enabled versus the tests that can run where addon signing is not enforced so we can invoke them separately.
Assignee | ||
Comment 9•9 years ago
|
||
So, most of our existing tests require addons incidentally (even if they're not explicitly testing addons), including _all_ of mochitest and reftest and their derivatives. The only harness we could create this definitive split for is xpcshell.
If what we're doing is finding all the tests that validate addon handling so we can invoke only those, separately, on the unsigned-addons-allowed build, then we can do that. What we can't do is split out tests so we can exclude addon-related tests from the signed-addons-required build until we have a signing mechanism in place, since nearly all of them required addons that will have to be signed.
Dave, can you clarify?
Flags: needinfo?(dtownsend)
Reporter | ||
Comment 10•9 years ago
|
||
I'm not sure what you're asking me for here. Looks like your statement is accurate.
Flags: needinfo?(dtownsend)
Assignee | ||
Comment 11•9 years ago
|
||
Take 2; this grabs all the addon-related xpcshell tests I could identify, which should be enough for testing the unsigned-addons-allowed build, even if I missed a couple. I don't want to use subsuite atm, since the goal is to leave the jobs as-is for the signed-addons-required build, but allow just the addon-related tests to be signed on the other.
Attachment #8649011 -
Flags: review?(ahalberstadt)
Assignee | ||
Updated•9 years ago
|
Attachment #8635560 -
Attachment is obsolete: true
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8649011 [details] [diff] [review]
Create an xpcshell-addons tag for running addon-specific xpcshell tests,
Review of attachment 8649011 [details] [diff] [review]:
-----------------------------------------------------------------
Whoops, wrong reviewer.
Attachment #8649011 -
Flags: review?(ahalberstadt) → review?(cmanchester)
Updated•9 years ago
|
Attachment #8649011 -
Flags: review?(cmanchester) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: leave-open
Comment 13•9 years ago
|
||
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/cf099fe29296 for causing mass bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=12997088&repo=mozilla-inbound
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=688775a8227f
Flags: needinfo?(jgriffin)
You need commas before your added lines in linux_unittest.py and mac_unittest.py and win_unittest.py to make the syntax valid, right?
Assignee | ||
Comment 16•9 years ago
|
||
(In reply to Wes Kocher (:KWierso) from comment #15)
> You need commas before your added lines in linux_unittest.py and
> mac_unittest.py and win_unittest.py to make the syntax valid, right?
Indeed, sorry for the bustage.
Flags: needinfo?(jgriffin)
Comment 17•9 years ago
|
||
Assignee | ||
Comment 19•9 years ago
|
||
Same thing, for mochitest-browser-chrome. This time, with green try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f62aacc4111
Attachment #8651110 -
Flags: review?(ahalberstadt)
Comment 20•9 years ago
|
||
Comment on attachment 8651110 [details] [diff] [review]
Create a browser-chrome-addons tag for running addon-specific browser-chrome tests,
Review of attachment 8651110 [details] [diff] [review]:
-----------------------------------------------------------------
I think you meant to flag chmanchester again, but this looks good to me :).
Attachment #8651110 -
Flags: review?(ahalberstadt) → review+
Assignee | ||
Comment 21•9 years ago
|
||
(In reply to Andrew Halberstadt [:ahal] from comment #20)
> Comment on attachment 8651110 [details] [diff] [review]
> Create a browser-chrome-addons tag for running addon-specific browser-chrome
> tests,
>
> Review of attachment 8651110 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> I think you meant to flag chmanchester again, but this looks good to me :).
He was PTO so I r?'d you. Thanks!
Comment 22•9 years ago
|
||
Comment 23•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Keywords: leave-open
Assignee | ||
Comment 24•9 years ago
|
||
This is complete, at least as far as we understand the requirements. I'll leave it open until we can verify this is enough, or understand what additional work is needed.
Assignee | ||
Comment 25•9 years ago
|
||
I'm going to call this done for now; will open a new bug if more work is needed.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•