Bug 172720

Summary: REGRESSION: LayoutTest js/intl-datetimeformat.html is failing
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: JavaScriptCoreAssignee: Andy VanWagoner <andy>
Status: RESOLVED FIXED    
Severity: Normal CC: andy, ap, cdumez, commit-queue, darin, fpizlo, ggaren, keith_miller, mark.lam, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch darin: review+, commit-queue: commit-queue-

Description Ryan Haddad 2017-05-30 08:41:11 PDT
LayoutTest js/intl-datetimeformat.html is failing

https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/r217555%20(1840)/results.html

--- /Volumes/Data/slave/sierra-release-tests-wk2/build/layout-test-results/js/intl-datetimeformat-expected.txt
+++ /Volumes/Data/slave/sierra-release-tests-wk2/build/layout-test-results/js/intl-datetimeformat-actual.txt
@@ -377,7 +377,7 @@
 PASS Intl.DateTimeFormat('en', { minute:'2-digit', hour:'numeric', timeZoneName:'short' }).resolvedOptions().timeZoneName is 'short'
 PASS Intl.DateTimeFormat('en', { minute:'2-digit', hour:'numeric', timeZoneName:'short', timeZone: 'UTC' }).format(0) is '12:00 AM GMT'
 PASS Intl.DateTimeFormat('pt-BR', { minute:'2-digit', hour:'numeric', timeZoneName:'long' }).resolvedOptions().timeZoneName is 'long'
-PASS Intl.DateTimeFormat('pt-BR', { minute:'2-digit', hour:'numeric', timeZoneName:'long', timeZone: 'UTC' }).format(0) is '00:00 GMT'
+FAIL Intl.DateTimeFormat('pt-BR', { minute:'2-digit', hour:'numeric', timeZoneName:'long', timeZone: 'UTC' }).format(0) should be 00:00 GMT. Was 00:00 Horário do Meridiano de Greenwich.
 PASS 
     var options = { weekday: "short", year: "numeric", month: "short", day: "numeric", hour: "numeric", minute: "numeric", second: "numeric" };
     var resolved = Intl.DateTimeFormat("ar", options).resolvedOptions();
@@ -2551,33 +2551,13 @@
 PASS Intl.DateTimeFormat("zh-TW", { month: "long", day: "numeric" }).formatToParts() is an instance of Array
 PASS Intl.DateTimeFormat("zh-TW", { hour: "numeric", minute: "numeric", second: "numeric" }).formatToParts() is an instance of Array
 PASS Intl.DateTimeFormat("zh-TW", { hour: "numeric", minute: "numeric" }).formatToParts() is an instance of Array
-PASS JSON.stringify(
+FAIL JSON.stringify(
   Intl.DateTimeFormat('en-US', {
     hour: "numeric", minute: "numeric", second: "numeric",
     year: "numeric", month: "long", day: "numeric", weekday: "long",
     timeZoneName: "long", era: "long", timeZone: "UTC"
   }).formatToParts(0)
-) is JSON.stringify([
-  {"type":"weekday","value":"Thursday"},
-  {"type":"literal","value":", "},
-  {"type":"month","value":"January"},
-  {"type":"literal","value":" "},
-  {"type":"day","value":"1"},
-  {"type":"literal","value":", "},
-  {"type":"year","value":"1970"},
-  {"type":"literal","value":" "},
-  {"type":"era","value":"Anno Domini"},
-  {"type":"literal","value":", "},
-  {"type":"hour","value":"12"},
-  {"type":"literal","value":":"},
-  {"type":"minute","value":"00"},
-  {"type":"literal","value":":"},
-  {"type":"second","value":"00"},
-  {"type":"literal","value":" "},
-  {"type":"dayPeriod","value":"AM"},
-  {"type":"literal","value":" "},
-  {"type":"timeZoneName","value":"GMT"}
-])
+) should be [{"type":"weekday","value":"Thursday"},{"type":"literal","value":", "},{"type":"month","value":"January"},{"type":"literal","value":" "},{"type":"day","value":"1"},{"type":"literal","value":", "},{"type":"year","value":"1970"},{"type":"literal","value":" "},{"type":"era","value":"Anno Domini"},{"type":"literal","value":", "},{"type":"hour","value":"12"},{"type":"literal","value":":"},{"type":"minute","value":"00"},{"type":"literal","value":":"},{"type":"second","value":"00"},{"type":"literal","value":" "},{"type":"dayPeriod","value":"AM"},{"type":"literal","value":" "},{"type":"timeZoneName","value":"GMT"}]. Was [{"type":"weekday","value":"Thursday"},{"type":"literal","value":", "},{"type":"month","value":"January"},{"type":"literal","value":" "},{"type":"day","value":"1"},{"type":"literal","value":", "},{"type":"year","value":"1970"},{"type":"literal","value":" "},{"type":"era","value":"Anno Domini"},{"type":"literal","value":", "},{"type":"hour","value":"12"},{"type":"literal","value":":"},{"type":"minute","value":"00"},{"type":"literal","value":":"},{"type":"second","value":"00"},{"type":"literal","value":" "},{"type":"dayPeriod","value":"AM"},{"type":"literal","value":" "},{"type":"timeZoneName","value":"Greenwich Mean Time"}].
 PASS successfullyParsed is true
 
 TEST COMPLETE
Comment 1 Radar WebKit Bug Importer 2017-05-30 08:41:34 PDT
<rdar://problem/32462418>
Comment 2 Ryan Haddad 2017-05-30 10:41:25 PDT
Marked test as failing / skipped for JSC in http://trac.webkit.org/projects/webkit/changeset/217560
Comment 3 Alexey Proskuryakov 2017-05-30 17:17:47 PDT
Is this a regression?

The difference is "Greenwich Mean Time" vs. "GMT".
Comment 4 Ryan Haddad 2017-05-30 20:51:21 PDT
(In reply to Alexey Proskuryakov from comment #3)
> Is this a regression?
> 
> The difference is "Greenwich Mean Time" vs. "GMT".

It started after the Sierra bots were updated to 10.12.5 last Friday.
Comment 5 Andy VanWagoner 2017-05-31 09:37:10 PDT
It looks to me like it's actually more correct now than it was before, since timeZoneName:'long' means written out, but the expectation needs to be updated. This is likely due to updated ICU data.

How should we handle situations like this where some bots have different locale data? Is there a way to branch inside the test?

I could update the test to use array.includes to check if the result is one of an acceptable list.
Comment 6 Alexey Proskuryakov 2017-05-31 12:28:57 PDT
Supporting multiple successful results seems best. We can have per platform results (including different macOS versions), but that's harder to maintain.
Comment 7 Andy VanWagoner 2017-05-31 20:20:40 PDT
Created attachment 311675 [details]
Patch
Comment 8 Chris Dumez 2017-06-02 12:21:39 PDT
ping review?
Comment 9 Darin Adler 2017-06-06 11:19:41 PDT
Comment on attachment 311675 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311675&action=review

> LayoutTests/js/intl-datetimeformat-expected.txt:380
> +PASS ['00:00 GMT','00:00 Horário do Meridiano de Greenwich'].includes(Intl.DateTimeFormat('pt-BR', { minute:'2-digit', hour:'numeric', timeZoneName:'long', timeZone: 'UTC' }).format(0)) is true

Why these two specific strings? What about all the other languages?
Comment 10 WebKit Commit Bot 2017-06-06 11:28:19 PDT
Comment on attachment 311675 [details]
Patch

Rejecting attachment 311675 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 311675, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
ngeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/js/intl-datetimeformat-expected.txt
patching file LayoutTests/js/script-tests/intl-datetimeformat.js
patching file LayoutTests/platform/mac/TestExpectations
Hunk #1 FAILED at 1541.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/mac/TestExpectations.rej

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Darin Adler']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.webkit.org/results/3882769
Comment 11 Andy VanWagoner 2017-06-06 11:38:25 PDT
Comment on attachment 311675 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311675&action=review

>> LayoutTests/js/intl-datetimeformat-expected.txt:380
>> +PASS ['00:00 GMT','00:00 Horário do Meridiano de Greenwich'].includes(Intl.DateTimeFormat('pt-BR', { minute:'2-digit', hour:'numeric', timeZoneName:'long', timeZone: 'UTC' }).format(0)) is true
> 
> Why these two specific strings? What about all the other languages?

This specific test was for Brazilian Portuguese. Depending on the mac version (or rather ICU data) it may have a long name for the timezone, or may fallback on the short abbreviation.
Comment 12 Darin Adler 2017-06-09 15:45:04 PDT
Committed r218025: <http://trac.webkit.org/changeset/218025>