Bug 182468 - [JSC] Update Test262 to Feb 9 version
Summary: [JSC] Update Test262 to Feb 9 version
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-02-04 07:24 PST by Yusuke Suzuki
Modified: 2018-02-08 22:46 PST (History)
7 users (show)

See Also:


Attachments
Patch (30.54 KB, patch)
2018-02-04 07:25 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (30.60 KB, patch)
2018-02-04 07:35 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (352.92 KB, patch)
2018-02-08 09:30 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (328.54 KB, patch)
2018-02-08 09:57 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (323.81 KB, patch)
2018-02-08 11:00 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (317.27 KB, patch)
2018-02-08 11:18 PST, Yusuke Suzuki
sbarati: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2018-02-04 07:24:25 PST
[JSC] Update Test262 to Feb 5 version
Comment 1 Yusuke Suzuki 2018-02-04 07:25:43 PST
Created attachment 333052 [details]
Patch
Comment 2 Yusuke Suzuki 2018-02-04 07:35:08 PST
Created attachment 333053 [details]
Patch
Comment 3 Yusuke Suzuki 2018-02-04 07:44:33 PST
Hmm, this `JSTests/test262/test/language/literals/regexp/7.8.5-1.js` patching always fails while the rebase target is not changed.
I created the above patch on ToT, and EWS applies this patch to ToT, completely same revision, and it fails.
Does anyone know to avoid this?


Parsed 20 diffs from patch file(s).
patching file JSTests/ChangeLog
patching file JSTests/test262.yaml
patching file JSTests/test262/test/built-ins/Array/prototype/flatMap/depth-always-one.js
patching file JSTests/test262/test/built-ins/Atomics/wake/wake-all-on-loc.js
patching file JSTests/test262/test/built-ins/Atomics/wake/wake-all.js
patching file JSTests/test262/test/built-ins/Function/prototype/toString/line-terminator-normalisation-CR.js
patch: **** malformed patch at line 7: // This code is governed by the BSD license found in the LICENSE file.

patching file JSTests/test262/test/built-ins/Promise/prototype/finally/invokes-then-with-function.js
patching file JSTests/test262/test/built-ins/Promise/prototype/finally/subclass-species-constructor-resolve-count.js
patching file JSTests/test262/test/built-ins/TypedArrays/typedarray-arg-detached-when-species-retrieved-different-type.js
patching file JSTests/test262/test/built-ins/TypedArrays/typedarray-arg-detached-when-species-retrieved-same-type.js
patching file JSTests/test262/test/language/expressions/assignment/white-space.js
patching file JSTests/test262/test/language/expressions/delete/white-space-line-terminator-between-delete-unaryexpression-allowed.js
patching file JSTests/test262/test/language/literals/regexp/7.8.5-1.js
Hunk #1 FAILED at 2.
1 out of 1 hunk FAILED -- saving rejects to file JSTests/test262/test/language/literals/regexp/7.8.5-1.js.rej
patching file JSTests/test262/test/language/module-code/privatename-valid-no-earlyerr.js
patching file JSTests/test262/test/language/statements/class/privatefieldget-typeerror-2.js
patching file JSTests/test262/test/language/statements/class/privatefieldget-typeerror-5.js
patching file JSTests/test262/test/language/statements/class/privatefieldset-typeerror-2.js
patching file JSTests/test262/test/language/statements/class/privatefieldset-typeerror-5.js
patching file JSTests/test262/test/language/statements/class/privatename-valid-no-earlyerr.js
patching file JSTests/test262/test262-Revision.txt

Failed to run "[u'/home/yusukesuzuki/dev/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /home/yusukesuzuki/dev/WebKit
Comment 4 Alexey Proskuryakov 2018-02-04 18:09:53 PST
This file has a malformed line ending (0a 0a 0d 0a). You can land a change to correct than manually first (no review needed), and then build on top of that.
Comment 5 Leo Balter 2018-02-08 08:46:15 PST
Hi, Yusuke! 

I'm new with WebKit, but please let me know if I can help in any way. I have plans for follow ups on updating Test262 and the respective runners and getting this patch up to speed will be helpful.
Comment 6 Yusuke Suzuki 2018-02-08 09:07:15 PST
(In reply to Alexey Proskuryakov from comment #4)
> This file has a malformed line ending (0a 0a 0d 0a). You can land a change
> to correct than manually first (no review needed), and then build on top of
> that.

Oh, super nice catch! I'll fix it with the manual landing and update this patch :D

(In reply to Leo Balter from comment #5)
> Hi, Yusuke! 
> 
> I'm new with WebKit, but please let me know if I can help in any way. I have
> plans for follow ups on updating Test262 and the respective runners and
> getting this patch up to speed will be helpful.

Great! I have one question.
It seems that several test files in test262 (like "test262/test/language/statements/class/privatefieldset-typeerror-5.js") mixes tabs and spaces.
Is it welcomed to change them to spaces?

Some patches like "test262/test/language/expressions/assignment/white-space.js" intentionally use tab characters. But I think "test262/test/language/statements/class/privatefieldset-typeerror-5.js" is not intentional use of tab characters.
Comment 7 Yusuke Suzuki 2018-02-08 09:11:15 PST
Committed r228275: <https://trac.webkit.org/changeset/228275>
Comment 8 Radar WebKit Bug Importer 2018-02-08 09:12:48 PST
<rdar://problem/37354470>
Comment 9 Yusuke Suzuki 2018-02-08 09:20:37 PST
Oops, my unreviewed fixing patch closed this issue.
Comment 10 Yusuke Suzuki 2018-02-08 09:30:22 PST
Created attachment 333380 [details]
Patch
Comment 11 Leo Balter 2018-02-08 09:48:43 PST
> Is it welcomed to change them to spaces?

Of course, and are already working to fix all of this noise on Test262. Rick Waldron should come with a patch anytime soon for this. We might also set some linting tool to prevent it from happening again there.

This might mean we might get one other Patch here with issues, as we are fixing whitespace problems.

After this, the only cases we might expect this patch failure would be the files with intentional white space cases, as you already gave an example.

I hope this makes a better case for updating Test262 here and in other projects.
Comment 12 Yusuke Suzuki 2018-02-08 09:53:56 PST
Committed r228277: <https://trac.webkit.org/changeset/228277>
Comment 13 Yusuke Suzuki 2018-02-08 09:57:19 PST
Reopening to attach new patch.
Comment 14 Yusuke Suzuki 2018-02-08 09:57:21 PST
Created attachment 333382 [details]
Patch
Comment 15 Yusuke Suzuki 2018-02-08 10:10:59 PST
https://github.com/tc39/test262/pull/1407 I've opened test262 PR for unnecessary tabs.
Comment 16 Yusuke Suzuki 2018-02-08 11:00:23 PST
Created attachment 333390 [details]
Patch
Comment 17 EWS Watchlist 2018-02-08 11:05:00 PST
Attachment 333390 [details] did not pass style-queue:


ERROR: Unexpected diff format when parsing a chunk: '='
ERROR: Unexpected diff format when parsing a chunk: "'U+000D';"
ERROR: Unexpected diff format when parsing a chunk: '\xe2\x80\xa8\xe2\x80\xa9=\t\x0b\x0c \xc2\xa0'
ERROR: Unexpected diff format when parsing a chunk: "\xe2\x80\xa8\xe2\x80\xa9'U+0009U+000BU+000CU+0020U+00A0U+000DU+2028U+2029';"
ERROR: Unexpected diff format when parsing a chunk: '}'
ERROR: Unexpected diff format when parsing a chunk: '}'
ERROR: JSTests/test262/test/language/statements/function/line-terminator-strict.js:46:  Line contains tab character.  [whitespace/tab] [5]
ERROR: JSTests/test262/test/language/expressions/assignment/white-space.js:12:  Line contains tab character.  [whitespace/tab] [5]
ERROR: JSTests/test262/test/language/expressions/assignment/white-space.js:52:  Line contains tab character.  [whitespace/tab] [5]
ERROR: JSTests/test262/test/language/expressions/delete/white-space-line-terminator-between-delete-unaryexpression-allowed.js:16:  Line contains tab character.  [whitespace/tab] [5]
ERROR: JSTests/test262/test/language/expressions/delete/white-space-line-terminator-between-delete-unaryexpression-allowed.js:48:  Line contains tab character.  [whitespace/tab] [5]
ERROR: JSTests/test262/test/language/statements/function/line-terminator-non-strict.js:47:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 6 in 171 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 18 Yusuke Suzuki 2018-02-08 11:18:11 PST
Created attachment 333394 [details]
Patch
Comment 19 Yusuke Suzuki 2018-02-08 11:20:18 PST
OK, I believe the patch is ready :)
Comment 20 EWS Watchlist 2018-02-08 11:22:21 PST
Attachment 333394 [details] did not pass style-queue:


ERROR: Unexpected diff format when parsing a chunk: '='
ERROR: Unexpected diff format when parsing a chunk: "'U+000D';"
ERROR: Unexpected diff format when parsing a chunk: '\xe2\x80\xa8\xe2\x80\xa9=\t\x0b\x0c \xc2\xa0'
ERROR: Unexpected diff format when parsing a chunk: "\xe2\x80\xa8\xe2\x80\xa9'U+0009U+000BU+000CU+0020U+00A0U+000DU+2028U+2029';"
ERROR: Unexpected diff format when parsing a chunk: '}'
ERROR: Unexpected diff format when parsing a chunk: '}'
ERROR: JSTests/test262/test/language/statements/function/line-terminator-strict.js:46:  Line contains tab character.  [whitespace/tab] [5]
ERROR: JSTests/test262/test/language/expressions/assignment/white-space.js:12:  Line contains tab character.  [whitespace/tab] [5]
ERROR: JSTests/test262/test/language/expressions/assignment/white-space.js:52:  Line contains tab character.  [whitespace/tab] [5]
ERROR: JSTests/test262/test/language/expressions/delete/white-space-line-terminator-between-delete-unaryexpression-allowed.js:16:  Line contains tab character.  [whitespace/tab] [5]
ERROR: JSTests/test262/test/language/expressions/delete/white-space-line-terminator-between-delete-unaryexpression-allowed.js:48:  Line contains tab character.  [whitespace/tab] [5]
ERROR: JSTests/test262/test/language/statements/function/line-terminator-non-strict.js:47:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 6 in 171 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 Saam Barati 2018-02-08 14:05:35 PST
Comment on attachment 333394 [details]
Patch

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

r=me

> JSTests/test262.yaml:83859
> -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
>  - path: test262/test/language/expressions/tagged-template/cache-differing-expressions-new-function.js
> -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
>  - path: test262/test/language/expressions/tagged-template/cache-differing-expressions.js
> -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
>  - path: test262/test/language/expressions/tagged-template/cache-differing-expressions.js
> -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]

Any idea why we started to fail these?

> JSTests/test262.yaml:83879
> -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
>  - path: test262/test/language/expressions/tagged-template/cache-identical-source-eval.js
> -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
>  - path: test262/test/language/expressions/tagged-template/cache-identical-source-new-function.js
> -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
>  - path: test262/test/language/expressions/tagged-template/cache-identical-source-new-function.js
> -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
>  - path: test262/test/language/expressions/tagged-template/cache-identical-source.js
> -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
>  - path: test262/test/language/expressions/tagged-template/cache-identical-source.js
> -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]

ditto
Comment 22 Leo Balter 2018-02-08 14:14:22 PST
(In reply to Saam Barati from comment #21)
> Comment on attachment 333394 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=333394&action=review
> 
> r=me
> 
> > JSTests/test262.yaml:83859
> > -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> > +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> >  - path: test262/test/language/expressions/tagged-template/cache-differing-expressions-new-function.js
> > -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> > +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> >  - path: test262/test/language/expressions/tagged-template/cache-differing-expressions.js
> > -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> > +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> >  - path: test262/test/language/expressions/tagged-template/cache-differing-expressions.js
> > -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> > +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> 
> Any idea why we started to fail these?

The tests reflect a new normative change, the consensus was captured from the last meeting.

https://github.com/tc39/test262/pull/972
https://github.com/tc39/ecma262/issues/840


> 
> > JSTests/test262.yaml:83879
> > -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> > +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> >  - path: test262/test/language/expressions/tagged-template/cache-identical-source-eval.js
> > -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> > +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> >  - path: test262/test/language/expressions/tagged-template/cache-identical-source-new-function.js
> > -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> > +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> >  - path: test262/test/language/expressions/tagged-template/cache-identical-source-new-function.js
> > -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> > +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> >  - path: test262/test/language/expressions/tagged-template/cache-identical-source.js
> > -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> > +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], []
> >  - path: test262/test/language/expressions/tagged-template/cache-identical-source.js
> > -  cmd: runTest262 :normal, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> > +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
> 
> ditto
Comment 23 Yusuke Suzuki 2018-02-08 17:34:04 PST
Comment on attachment 333394 [details]
Patch

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

Thank you.

>>> JSTests/test262.yaml:83859
>>> +  cmd: runTest262 :fail, "NoException", ["../../../../harness/assert.js", "../../../../harness/sta.js"], [:strict]
>> 
>> Any idea why we started to fail these?
> 
> The tests reflect a new normative change, the consensus was captured from the last meeting.
> 
> https://github.com/tc39/test262/pull/972
> https://github.com/tc39/ecma262/issues/840

Yeah, that's the reason.
Comment 24 WebKit Commit Bot 2018-02-08 17:49:38 PST
Comment on attachment 333394 [details]
Patch

Rejecting attachment 333394 [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', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 333394, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
ng rebase:
:040000 040000 8b8bf95e8002d180d84ad10f924a5d31b9fc3bf6 7ec897d2b70e3a4f715be6429741a055cb2255da M	JSTests
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
Current branch master is up to date.

Full output: http://webkit-queues.webkit.org/results/6423442
Comment 25 Yusuke Suzuki 2018-02-08 22:46:22 PST
Committed r228311: <https://trac.webkit.org/changeset/228311>