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

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Catanzaro]

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.

[Michael Catanzaro]

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

[Alexey Proskuryakov]

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.

[Michael Catanzaro]

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

[Michael Catanzaro]

(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?

[Andy VanWagoner]

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

[Michael Catanzaro]

That'd be appreciated, thanks!

[Andy VanWagoner]

Created attachment 361093 [details]
Patch

[Andy VanWagoner]

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

[Michael Catanzaro]

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!

[Michael Catanzaro]

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'

[Andy VanWagoner]

Created attachment 361191 [details]
Patch

[Michael Catanzaro]

Comment on attachment 361191 [details]
Patch

Works, nice!

[WebKit Commit Bot]

Comment on attachment 361191 [details]
Patch

Clearing flags on attachment: 361191

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

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/47827289>