193620 – REGRESSION(r238848): ICU upgrade broke jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout

Bug 193620 - REGRESSION(r238848): ICU upgrade broke jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout

Summary: REGRESSION(r238848): ICU upgrade broke jsc-layout-tests.yaml/js/script-tests/...

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	JavaScriptCore (show other bugs)
Version:	WebKit Nightly Build
Hardware:	PC Linux

Importance:	P2 Normal
Assignee:	Andy VanWagoner

URL:
Keywords:	InRadar

Depends on:
Blocks:

Reported:	2019-01-20 11:20 PST by Michael Catanzaro
Modified:	2019-02-05 12:31 PST (History)
CC List:	10 users (show)

See Also:

Attachments
Patch (10.14 KB, patch) 2019-02-04 13:45 PST, Andy VanWagoner	no flags	Details \| Formatted Diff \| Diff
Patch (11.93 KB, patch) 2019-02-05 08:18 PST, Andy VanWagoner	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Michael Catanzaro 2019-01-20 11:20:13 PST

When r238848 upgraded the version of ICU used by GTK port's test bot from 57.1 to 63.1, it broke jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout. Diff:

--- /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/layout-test-results/js/intl-numberformat-expected.txt
+++ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/layout-test-results/js/intl-numberformat-actual.txt
@@ -203,10 +203,10 @@
 PASS Intl.NumberFormat('en').format(0) is '0'
 PASS Intl.NumberFormat('en').format(-0) is '0'
 PASS Intl.NumberFormat('en').format(Number.MIN_VALUE) is '0'
-PASS Intl.NumberFormat('en').format(Number.MAX_VALUE) is '179,769,313,486,232,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000'
+FAIL Intl.NumberFormat('en').format(Number.MAX_VALUE) should be 179,769,313,486,232,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000. Was 179,769,313,486,231,570,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000.
 PASS Intl.NumberFormat('en').format(1234.567) is '1,234.567'
 PASS Intl.NumberFormat('es').format(1234.567) is '1.234,567'
-PASS Intl.NumberFormat('fr').format(1234.567) is '1\xA0234,567'
+FAIL Intl.NumberFormat('fr').format(1234.567) should be 1Â 234,567. Was 1â€¯234,567.
 PASS Intl.NumberFormat('en-u-nu-latn').format(1234.567) is '1,234.567'
 PASS Intl.NumberFormat('en-u-nu-fullwide').format(1234.567) is 'ï¼‘,ï¼’ï¼“ï¼”.ï¼•ï¼–ï¼—'
 PASS Intl.NumberFormat('th-u-nu-thai').format(1234.567) is 'à¹‘,à¹’à¹“à¹”.à¹•à¹–à¹—'
@@ -217,14 +217,14 @@
 PASS Intl.NumberFormat('en', {style: 'currency', currency: 'USD'}).format(4) is '$4.00'
 PASS Intl.NumberFormat('en', {style: 'currency', currency: 'USD'}).format(4.2) is '$4.20'
 PASS Intl.NumberFormat('en', {style: 'currency', currency: 'USD'}).format(-4.2) is '-$4.20'
-PASS Intl.NumberFormat('en', {style: 'currency', currency: 'USD'}).format(NaN) is 'NaN'
+FAIL Intl.NumberFormat('en', {style: 'currency', currency: 'USD'}).format(NaN) should be NaN. Was $NaN.
 PASS Intl.NumberFormat('en', {style: 'currency', currency: 'USD'}).format(Infinity) is '$âˆž'
 PASS Intl.NumberFormat('en', {style: 'currency', currency: 'JPY'}).format(4.2) is 'Â¥4'
-PASS Intl.NumberFormat('en', {style: 'currency', currency: 'xXx'}).format(4.2) is 'XXX4.20'
-PASS Intl.NumberFormat('en', {style: 'currency', currency: 'USD', currencyDisplay: 'code'}).format(4) is 'USD4.00'
+FAIL Intl.NumberFormat('en', {style: 'currency', currency: 'xXx'}).format(4.2) should be XXX4.20. Was Â¤4.20.
+FAIL Intl.NumberFormat('en', {style: 'currency', currency: 'USD', currencyDisplay: 'code'}).format(4) should be USD4.00. Was USDÂ 4.00.
 PASS Intl.NumberFormat('en', {style: 'currency', currency: 'USD', currencyDisplay: 'symbol'}).format(4) is '$4.00'
 PASS Intl.NumberFormat('en', {style: 'currency', currency: 'USD', currencyDisplay: 'name'}).format(4) is '4.00 US dollars'
-PASS Intl.NumberFormat('en', {style: 'currency', currency: 'JPY', currencyDisplay: 'code'}).format(-4.2) is '-JPY4'
+FAIL Intl.NumberFormat('en', {style: 'currency', currency: 'JPY', currencyDisplay: 'code'}).format(-4.2) should be -JPY4. Was -JPYÂ 4.
 PASS Intl.NumberFormat('en', {style: 'currency', currency: 'JPY', currencyDisplay: 'symbol'}).format(-4.2) is '-Â¥4'
 PASS Intl.NumberFormat('en', {style: 'currency', currency: 'JPY', currencyDisplay: 'name'}).format(-4.2) is '-4 Japanese yen'
 PASS Intl.NumberFormat('fr', {style: 'currency', currency: 'USD', currencyDisplay: 'name'}).format(4) is '4,00 dollars des Ã‰tats-Unis'
@@ -250,7 +250,7 @@
 PASS Intl.NumberFormat('en', {maximumSignificantDigits: 4}).format(1234567) is '1,235,000'
 PASS Intl.NumberFormat('en', {useGrouping: true}).format(1234567.123) is '1,234,567.123'
 PASS Intl.NumberFormat('es', {useGrouping: true}).format(1234567.123) is '1.234.567,123'
-PASS Intl.NumberFormat('fr', {useGrouping: true}).format(1234567.123) is '1\xA0234\xA0567,123'
+FAIL Intl.NumberFormat('fr', {useGrouping: true}).format(1234567.123) should be 1Â 234Â 567,123. Was 1â€¯234â€¯567,123.
 PASS Intl.NumberFormat('en', {useGrouping: false}).format(1234567.123) is '1234567.123'
 PASS Intl.NumberFormat('es', {useGrouping: false}).format(1234567.123) is '1234567,123'
 PASS Intl.NumberFormat('fr', {useGrouping: false}).format(1234567.123) is '1234567,123'

There are basically two good solutions to resolve this bug:

 * Rewrite the few failing checks to expect modern ICU and commit new -expected results for Mac where the tests fail
 * Commit expected results for GTK alone including the failures

I'm not going to do either, since I hate adding FAIL expectations to test results and there's really no way to resolve this without doing so, since the test isn't going to work for both versions of ICU at the same time. Instead, I've just added a failure expectation for GTK.

Comment 1 Michael Catanzaro 2019-01-20 11:22:56 PST

Committed r240214: <https://trac.webkit.org/changeset/240214>

Comment 2 Alexey Proskuryakov 2019-01-20 18:25:34 PST

We'll get the ICU update on all platform sooner or later. Perhaps we should always update expectations to the newest version, and land custom expectations to all old platforms/versions.

Seems better than marking future as failure.

Comment 3 Michael Catanzaro 2019-01-20 21:37:15 PST

I agree. Note it requires changes to the test itself (intl-numberformat.js).

Comment 4 Michael Catanzaro 2019-01-21 07:50:31 PST

(In reply to Michael Catanzaro from comment #1)
> Committed r240214: <https://trac.webkit.org/changeset/240214>

This didn't work. The test is run as part of both the layout test suite and the JSC test suite. Now it's no longer unexpectedly failing during the layout test suite (as the failure is now expected) but it's still unexpectedly failing during the JSC tests:

** The following JSC stress test failures have been introduced:
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-ftl-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-ftl-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-ftl
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-llint

Does anybody know how to handle this? Do we have to skip the tests altogether?

Comment 5 Andy VanWagoner 2019-02-04 10:07:13 PST

I can try to update the expectations to handle both versions. These have always been a bit tricky since localization data can change.

Comment 6 Michael Catanzaro 2019-02-04 12:50:35 PST

That'd be appreciated, thanks!

Comment 7 Andy VanWagoner 2019-02-04 13:45:25 PST

Created attachment 361093 [details]
Patch

Comment 8 Andy VanWagoner 2019-02-04 14:01:48 PST

Is the `gtk-wk2` build in EWS the same bot that was broken by the upgrade?

Comment 9 Michael Catanzaro 2019-02-04 17:13:03 PST

Yes, but it doesn't run tests at all. Only the Mac and iOS EWS run tests. :(

Not to worry: I will test this locally and report back (probably tomorrow). Thanks!

Comment 10 Michael Catanzaro 2019-02-04 20:43:33 PST

Almost! Only one difference remaining:

--- /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/layout-test-results/js/intl-numberformat-expected.txt
+++ /home/mcatanzaro/Projects/WebKit/WebKitBuild/Debug/layout-test-results/js/intl-numberformat-actual.txt
@@ -203,7 +203,7 @@
 PASS Intl.NumberFormat('en').format(0) is '0'
 PASS Intl.NumberFormat('en').format(-0) is '0'
 PASS Intl.NumberFormat('en').format(Number.MIN_VALUE) is '0'
-PASS Intl.NumberFormat('en').format(Number.MAX_VALUE) is '179,769,313,486,232,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000'
+FAIL Intl.NumberFormat('en').format(Number.MAX_VALUE) should be 179,769,313,486,232,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000. Was 179,769,313,486,231,570,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000.
 PASS Intl.NumberFormat('en').format(1234.567) is '1,234.567'
 PASS Intl.NumberFormat('es').format(1234.567) is '1.234,567'
 PASS Intl.NumberFormat('de').format(1234.567) is '1.234,567'

Comment 11 Andy VanWagoner 2019-02-05 08:18:00 PST

Created attachment 361191 [details]
Patch

Comment 12 Michael Catanzaro 2019-02-05 12:27:46 PST

Comment on attachment 361191 [details]
Patch

Works, nice!

Comment 13 WebKit Commit Bot 2019-02-05 12:31:00 PST

Comment on attachment 361191 [details]
Patch

Clearing flags on attachment: 361191

Committed r240988: <https://trac.webkit.org/changeset/240988>

Comment 14 WebKit Commit Bot 2019-02-05 12:31:02 PST

All reviewed patches have been landed.  Closing bug.

Comment 15 Radar WebKit Bug Importer 2019-02-05 12:31:40 PST

<rdar://problem/47827289>