1288684 - Add basic MessageContext and L10nRegistry xpcshell tests

Status: RESOLVED FIXED

(L20n :: JS Library, defect)

Description

[Staś Małolepszy :stas]

It would be great to leverage mozilla-central's integration testing to provide good test coverage for MessageContext.

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1288684 - Add basic xpcshell tests for MessageContext and L10nRegistry.

Review commit: https://reviewboard.mozilla.org/r/91354/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/91354/

Comment 2

[Staś Małolepszy :stas]

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`?

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

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.

Comment 4

[Staś Małolepszy :stas]

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

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

Filed as bug 1316068 and bug 1316069

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

https://hg.mozilla.org/projects/larch/rev/4bf4d2d21fb5ea638f76bdcb28b87930bddc35f6
Bug 1288684 - Add basic xpcshell tests for MessageContext and L10nRegistry. r?stas