Closed Bug 436564 Opened 12 years ago Closed 4 years ago

distribution.js module needs tests

Categories

(Firefox :: Distributions, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 47
Tracking Status
firefox47 --- fixed

People

(Reporter: hello, Assigned: mkaply)

Details

Attachments

(1 file)

The distribution customizations module needs tests.

I'm not sure what the correct framework is for automated testing.  We need to be able to drop in files to the appdir (in the distribution/ directory), then launch firefox with a new profile and check that the correct changes have been made to the profile.

Manual testing (e.g., litmus) would also be useful.
(In reply to comment #0)
> 
> Manual testing (e.g., litmus) would also be useful.
> 

Let me know what kind of tests do you need :) 
We need a sample distribution tree that exercises a wide range of functionality.  Kev, can you help with that?  It should include search plugins, different locales, etc.

And the test should be something like:

1) locate the appdir
2) remove any distribution/ directory if it exists
3) unzip the sample distribution tree into appdir
4) launch firefox with a new profile
5) check various settings the distribution customizes
I can put one together. We also need to check special character handling, and js links, etc. Have a pretty good idea, and I'll add a couple sample distribution directories to cover use cases.
Assignee: nobody → kev
Taking
Assignee: kev → mozilla
This is still on the try server, but wanted to give Shane a chance to look at it (try is taking forever).
Comment on attachment 8723697 [details]
MozReview Request: Bug 436564 - Add unit tests for distribution.js; r?mixedpuppy

https://reviewboard.mozilla.org/r/36679/#review33241

if the todo exists, lets use that, otherwise r+

::: browser/components/tests/unit/test_distribution.js:76
(Diff revision 1)
> +  // Assert.throws(() => Services.prefs.getComplexValue("distribution.test.reset", Ci.nsIPrefLocalizedString));

is there an Assert.todo?  The older tests have a todo(bool,msg) and todo_is(a,b,msg)
Attachment #8723697 - Flags: review?(mixedpuppy) → review+
Try run:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fc14d5b2b980&selectedJob=17242886

All those errors are unrelated, so I think I'm good to push here.
Yes. I based this test on that :).

There's also search engine tests as well:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/search/tests/xpcshell/test_async_distribution.js
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/search/tests/xpcshell/test_sync_distribution.js

I honestly don't think it makes sense to combine them, since they all rely on the services where they are located.

I'm going to close this bug once we merge and if we decide to beef up the tests we'll do it in a separate bug.
Component: General → Distributions
https://hg.mozilla.org/mozilla-central/rev/8a390011304e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
You need to log in before you can comment on or make changes to this bug.