1355465 - Enhance TestDateTimeFormat.cpp to test requests for a non-en-US locale

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement, P3)

Description

[Jorg K (CEST = GMT+2)]

+++ This bug was initially created as a clone of Bug #1351427 +++

Enhance TestDateTimeFormat.cpp to test that requests for a non-en-US locale.

In bug 1351427 we changed C++ date/time formatting to that it respects the OS pattern. This is only tested for en-US on a test server with en-US system/user locale.

Enhance this test by requesting the OS pattern for a de-DE locale on the same en-US test server.

Expected result: de-DE patterns need to be used, not the en-US pattern.

The code in GetWindowsLocaleFor() should already do that.

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: 1355465-non-US-locale.patch (v1).

This works nicely on Windows with an en-US OS and en-US format locale.

We request de-DE and we get the de-DE Windows OS formats, not the en-US formats. This is how it's meant to work. Good job, Zibi!

Now let's see whether this already works on Linux or Mac, there are less variations to German formats, so it might already pass.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ac9f2c9fd53a2a6638dfae5133dcfcae94fb203

Comment 2

[Ben Bucksch (:BenB)]

Expected result:
Firefox and Thunderbird have follow the OS settings for date/time formats, independent of the UI language of the TB/FF build.

> On 11.04.2017 21:04, Bugzilla@Mozilla wrote:

That's the expected result. This uses the English locale, with the German date/time format that I chose.
That's what Thunderbird has always done.

Wrong would be:
> On 11/04/2017 21:04, Bugzilla@Mozilla wrote:
because
a) it does not follow the date/time format I configured.
b) it's ambiguous in the international context. An American would read "Nov 4", although it's "Apr 11". This is an absolute no-go.

Wrong would be:
> On 11.04.2017 21:04, Bugzilla@Mozilla schrieb:
because it has German text, which my English recipients cannot read

Comment 3

[Jorg K (CEST = GMT+2)]

OK, didn't work, Linux and Mac still append "GMT", so let's strip that.

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: 1355465-non-US-locale.patch (v2).

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e92da53aacef2fc1c9618bef6c42d41a496ff60
This passed on Linux now, I expect Mac to pass as well.

Comment 5

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

Comment on attachment 8857074 [details] [diff] [review]
1355465-non-US-locale.patch (v2).

Review of attachment 8857074 [details] [diff] [review]:
-----------------------------------------------------------------

Overall, looks good! I have one design suggestion for you to evaluate.

::: intl/locale/tests/gtest/TestDateTimeFormat.cpp
@@ +122,5 @@
> +  int32_t ind = datetime.Find(" GMT");
> +  if (ind != kNotFound)
> +    datetime.Truncate(ind);
> +
> +  // Strip leading "Donnerstag, " or "Mittwoch, " (found on Windows).

I think I'm a bit afraid of writing a test in a way that requires per-platform adjustments.

Can we instead of testing for string equality, test if certain bits are included?

Like, the date should have "02" or "March", but it doesn't matter if there was a soft or hard comma separator.
If you want to put more time into it, you could also test for order (month should  come after day or reverse), but I'd prefer to keep the output not very strictly tested since it may evolve (next release of CLDR may adjust some formattings etc.).

Comment 6

[Jorg K (CEST = GMT+2)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #5)
> Can we instead of testing for string equality, test if certain bits are
> included?
So suggested that in bug 1351427 comment #57 ("make partial tests like"). I read and remember your comments ;-)
Overall that is harder to do, so it's just plain laziness to whip up a test quickly.

Comment 7

[Jorg K (CEST = GMT+2)]

This depends on bug 1351427 which needs to be checked in first.
Successful try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=15d2adb2e0bc0030286b41b9e3fe5dc85f90aca3

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ca47045ab08
Enhance TestDateTimeFormat.cpp with non-en-US locale. r=emk

Comment 9

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/1ca47045ab08