1403902 - Migrate browser_webconsole_bug_580454_timestamp_l10n.js to the new frontend

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P1)

Description

[Nicolas Chevobbe [:nchevobbe]]

this might be better to have it as an xpcshell test (or mocha if we get to make l10 work in it)

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1403902 - migrate browser_webconsole_bug_580454_timestamp_l10n to xpcshell test;

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

Comment 2

[Julian Descottes [:jdescottes]]

try (mochitest + xpcshell) at https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa74f51591ee81ffbd432abf7e34d62915f06c91

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8946726 [details]
Bug 1403902 - migrate browser_webconsole_bug_580454_timestamp_l10n to xpcshell test;

https://reviewboard.mozilla.org/r/216694/#review222630

This looks good to me, thanks Julian !
The only question I have is if we should have this test in this filder or in the new-console-output one.
Eventually the webconsole/test folder will be replaced by the one in new-console when we delete the old code, so here I fear we might end up wiping out the xpcshell test as well.
This is probably unlikely, but having it in new-console-output would make the operation a no brainer. What do you think ?

::: devtools/client/webconsole/test/unit/test_webconsole_l10n.js:6
(Diff revision 1)
> +/* Any copyright is dedicated to the Public Domain.
> +   http://creativecommons.org/publicdomain/zero/1.0/ */
> +
> +"use strict";
> +
> +/* eslint no-unused-vars: [2, {"vars": "local"}] */

Why is it needed ? I don't see any variable not being used. Is it for run_test ?

::: devtools/client/webconsole/test/unit/test_webconsole_l10n.js:16
(Diff revision 1)
> +  ok(localizedString.indexOf(date.getHours()) != -1, "the localized " +
> +        "timestamp contains the hours");
> +  ok(localizedString.indexOf(date.getMinutes()) != -1, "the localized " +
> +        "timestamp contains the minutes");
> +  ok(localizedString.indexOf(date.getSeconds()) != -1, "the localized " +
> +        "timestamp contains the seconds");
> +  ok(localizedString.indexOf(date.getMilliseconds()) != -1, "the localized " +
> +        "timestamp contains the milliseconds");

nit: could we use `includes` instead of `indexOf` ? I find it slightly easier to read

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 8946726 [details]
Bug 1403902 - migrate browser_webconsole_bug_580454_timestamp_l10n to xpcshell test;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216694/diff/1-2/

Comment 5

[Julian Descottes [:jdescottes]]

Comment on attachment 8946726 [details]
Bug 1403902 - migrate browser_webconsole_bug_580454_timestamp_l10n to xpcshell test;

https://reviewboard.mozilla.org/r/216694/#review222630

Thanks! I wasn't sure if the test folder was supposed to stay under new-console-output or not. If it's going up one level after the migration then it makes sense to have the "unit" folder here.

> Why is it needed ? I don't see any variable not being used. Is it for run_test ?

Good catch, removed.

> nit: could we use `includes` instead of `indexOf` ? I find it slightly easier to read

Sure! This was a leftover from the initial test. For clarity I also amended to perform a hg move rather than a handmade copy.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/811278d126bb
migrate browser_webconsole_bug_580454_timestamp_l10n to xpcshell test;r=nchevobbe

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Great !

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/811278d126bb