+++ 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.
Summary: Enhance TestDateTimeFormat.cpp to test that requests for a non-en-US locale → Enhance TestDateTimeFormat.cpp to test requests for a non-en-US locale
Created attachment 8856985 [details] [diff] [review] 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
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
OK, didn't work, Linux and Mac still append "GMT", so let's strip that.
Created attachment 8857074 [details] [diff] [review] 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 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.).
Attachment #8857074 - Flags: feedback?(gandalf) → feedback+
(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.
Attachment #8857074 - Flags: review?(VYV03354) → review+
This depends on bug 1351427 which needs to be checked in first. Successful try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=15d2adb2e0bc0030286b41b9e3fe5dc85f90aca3
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/1ca47045ab08 Enhance TestDateTimeFormat.cpp with non-en-US locale. r=emk
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.