Shane points out that it's perfectly reasonable for a provider to not have a worker, so there's no reason to enforce the presence of a worker URL in the provider object.

This also merges the tests into one, since the duplication was getting to me (I know I just landed that a few hours ago). I'm not sure I understand the AsyncRunner magic well enough to be confident that this is right, but it seems to work!

Comment on attachment 639480 [details] [diff] [review] patch Review of attachment 639480 [details] [diff] [review]: ----------------------------------------------------------------- I prefer small, separate test files per SocialService method rather than a big omnibus test_SocialService.js that's only going to get bigger. (We can factor out common code into head.js, which is what head.js is for.) I'd really like that changed back before landing, but I bet you disagree. ::: toolkit/components/social/test/xpcshell/test_getProvider.js @@ +23,5 @@ > Cu.import("resource://gre/modules/SocialService.jsm"); > > let runner = new AsyncRunner(); > + runner.appendIterator(testGetProvider(manifests, runner.next.bind(runner))); > + runner.appendIterator(testGetProviderList(manifests, runner.next.bind(runner))); Store runner.next.bind(runner) in a local variable and pass that variable to the generator functions so that we're not repeating it every time a new generator is added. (If you keep test_SocialService.js.) @@ +45,5 @@ > + do_check_true(providers.length >= manifests.length); > + for (let i = 0; i < manifests.length; i++) { > + do_check_neq(providers.map(function (p) p.origin).indexOf(manifests[i].origin), -1); > + do_check_neq(providers.map(function (p) p.workerURL).indexOf(manifests[i].workerURL), -1); > + do_check_neq(providers.map(function (p) p.name).indexOf(manifests[i].name), -1); These just check that the origin, workerURL, and name exist in some provider in the list. They don't ensure that all three are in the same provider. What I would do is for each manifest, first find the provider with the right origin, and then check that its workerURL and name are right.

Made the test changes you suggested, but kept test_SocialService - we can always split it up again if it gets too unwieldy. https://hg.mozilla.org/integration/mozilla-inbound/rev/aef382189fef