Closed Bug 1288684 Opened 5 years ago Closed 5 years ago

Add basic MessageContext and L10nRegistry xpcshell tests

Categories

(L20n :: JS Library, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: stas, Assigned: zbraniecki)

References

Details

(Whiteboard: [gecko-l20n])

Attachments

(1 file)

It would be great to leverage mozilla-central's integration testing to provide good test coverage for MessageContext.
Whiteboard: [gecko-l20n]
No longer blocks: 1288639
Blocks: 1291693
Assignee: nobody → gandalf
Summary: Find a way to run MessageContext's tests in mozilla-central → Add basic MessageContext and L10nRegistry xpcshell tests
Comment on attachment 8808523 [details]
Bug 1288684 - Add basic xpcshell tests for MessageContext and L10nRegistry.

https://reviewboard.mozilla.org/r/91354/#review91226

::: toolkit/modules/tests/xpcshell/test_IntlMessageContext.js:60
(Diff revision 1)
> +  mc.addMessages('key = Hello, { $name2 }');
> +
> +  const entity = mc.messages.get('key');
> +
> +  equal(mc.format(entity, {name: "Mary"}, errors), 'Hello, name2');
> +

nit: remove some of the blank lines?

::: toolkit/modules/tests/xpcshell/test_L10nRegistry.js:47
(Diff revision 1)
> +
> +  equal(Array.from(result.supportedLocales).length, 1);
> +  equal(result.bundles[0].locale, 'en-US');
> +  equal(result.bundles[0].resources['browser/brand.ftl'].locale, 'en-US');
> +  equal(result.bundles[0].resources['browser/brand.ftl'].source, 'app');
> +  equal(result.bundles[0].resources['browser/brand.ftl'].data, null);

Why is this `null`?

::: toolkit/modules/tests/xpcshell/test_L10nRegistry.js:54
(Diff revision 1)
> +
> +add_task(function* test_fetchResource() {
> +  const result =
> +    yield L10nRegistry.fetchResource('app', 'browser/brand.ftl', 'en-US');
> +
> +  equal(result, null);

Why is this `null`?
Attachment #8808523 - Flags: review?(stas) → review+
I believe that the reason we don't return data here is because the test operates in xpcshell which doesn't load toolkit or app.

It makes me think that in the future we may want to move the registering of FileSources to those two so that in xpcshell you have none, in pure Gecko you have 'platform' and only in browser you have two.

I'm going to add a whole-page test - either mochi (which works like karma), or reftest (which is a screenshot to screenshot comparison), which will cover loading the resource as part of it.
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #3)
> I believe that the reason we don't return data here is because the test
> operates in xpcshell which doesn't load toolkit or app.
> 
> It makes me think that in the future we may want to move the registering of
> FileSources to those two so that in xpcshell you have none, in pure Gecko
> you have 'platform' and only in browser you have two.

File a bug for this please.

> 
> I'm going to add a whole-page test - either mochi (which works like karma),
> or reftest (which is a screenshot to screenshot comparison), which will
> cover loading the resource as part of it.

Also in a new bug, maybe?
https://hg.mozilla.org/projects/larch/rev/4bf4d2d21fb5ea638f76bdcb28b87930bddc35f6
Bug 1288684 - Add basic xpcshell tests for MessageContext and L10nRegistry. r?stas
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.