1136892 - Split add-on manager xpcshell and mochitest-browser tests into their own test suites

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Dave Townsend [:mossop]]

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.

Comment 1

[Jonathan Griffin (:jgriffin)]

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?

Comment 2

[Jonathan Griffin (:jgriffin)]

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.

Comment 3

[Jonathan Griffin (:jgriffin)]

Attachment: Create an xpcshell-addons tag for running addon-specific xpcshell tests,

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.

Comment 4

[Dave Townsend [:mossop]]

(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.

Comment 5

[Jonathan Griffin (:jgriffin)]

> 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

[Chris Manchester (limited bugmail, email directly)]

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.

Comment 7

[Jonathan Griffin (:jgriffin)]

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

[Kim Moir [:kmoir] ET]

>> 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.

Comment 9

[Jonathan Griffin (:jgriffin)]

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?

Comment 10

[Dave Townsend [:mossop]]

I'm not sure what you're asking me for here. Looks like your statement is accurate.

Comment 11

[Jonathan Griffin (:jgriffin)]

Attachment: Create an xpcshell-addons tag for running addon-specific xpcshell tests,

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.

Comment 12

[Jonathan Griffin (:jgriffin)]

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.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/688775a8227f

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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?

Comment 16

[Jonathan Griffin (:jgriffin)]

(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.

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/860b424c5ed9

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/860b424c5ed9

Comment 19

[Jonathan Griffin (:jgriffin)]

Attachment: Create a browser-chrome-addons tag for running addon-specific browser-chrome tests,

Same thing, for mochitest-browser-chrome.  This time, with green try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f62aacc4111

Comment 20

[Andrew Halberstadt [:ahal]]

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 :).

Comment 21

[Jonathan Griffin (:jgriffin)]

(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

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ffef2160548d

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ffef2160548d

Comment 24

[Jonathan Griffin (:jgriffin)]

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.

Comment 25

[Jonathan Griffin (:jgriffin)]

I'm going to call this done for now; will open a new bug if more work is needed.