Bug 193620

Summary: REGRESSION(r238848): ICU upgrade broke jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: JavaScriptCoreAssignee: Andy VanWagoner <andy>
Status: RESOLVED FIXED    
Severity: Normal CC: andy, ap, commit-queue, dominik.infuehr, jlewis3, keith_miller, mcatanzaro, ryanhaddad, tsavell, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
Patch none

Michael Catanzaro
Reported 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.
Attachments
Patch (10.14 KB, patch)
2019-02-04 13:45 PST, Andy VanWagoner 		no flags
DetailsFormatted DiffDiff
Patch (11.93 KB, patch)
2019-02-05 08:18 PST, Andy VanWagoner 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Michael Catanzaro
Comment 1 2019-01-20 11:22:56 PST
Committed r240214: <https://trac.webkit.org/changeset/240214>
Alexey Proskuryakov
Comment 2 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.
Michael Catanzaro
Comment 3 2019-01-20 21:37:15 PST
I agree. Note it requires changes to the test itself (intl-numberformat.js).
Michael Catanzaro
Comment 4 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?
Andy VanWagoner
Comment 5 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.
Michael Catanzaro
Comment 6 2019-02-04 12:50:35 PST
That'd be appreciated, thanks!
Andy VanWagoner
Comment 7 2019-02-04 13:45:25 PST
Created attachment 361093 [details] Patch
Andy VanWagoner
Comment 8 2019-02-04 14:01:48 PST
Is the `gtk-wk2` build in EWS the same bot that was broken by the upgrade?
Michael Catanzaro
Comment 9 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!
Michael Catanzaro
Comment 10 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'
Andy VanWagoner
Comment 11 2019-02-05 08:18:00 PST
Created attachment 361191 [details] Patch
Michael Catanzaro
Comment 12 2019-02-05 12:27:46 PST
Comment on attachment 361191 [details] Patch Works, nice!
WebKit Commit Bot
Comment 13 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>
WebKit Commit Bot
Comment 14 2019-02-05 12:31:02 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 15 2019-02-05 12:31:40 PST
<rdar://problem/47827289>
Note You need to log in before you can comment on or make changes to this bug.