Bug 157797 – Add test262 harness support code

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Keith Miller]

Add test262 harness support code

[Keith Miller]

Created attachment 279140 [details]
Patch

[Filip Pizlo]

Comment on attachment 279140 [details]
Patch

LGTM

[WebKit Commit Bot]

Comment on attachment 279140 [details]
Patch

Clearing flags on attachment: 279140

Committed r201039: <http://trac.webkit.org/changeset/201039>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

Keith Miller 2016-05-17 11:44:51 PDT
Created attachment 279140 [details]
Patch

Filip Pizlo 2016-05-17 11:49:28 PDT
Comment on attachment 279140 [details]
Patch
LGTM

r+ in 5 minutes? What is the reason for this haste?

I think we could and should make the test262.yaml file less 
verbose and redundant. (bug157807)

For example, harness/assert.js and harness.sta js are included
by all "runTest262" test. I prefer adding these common harness
files to includeFiles in runTest262 function instead.

Additionally we could remove the ugly relative paths to 
harness files, all of them are in the same harness directory.

[Csaba Osztrogonác]

One more thing to reduce redundancy. There are ~20.000 tests, 
but only 1000 tests with [onlyStrict] or [noStrict] flags.

runTest262 could handle onlyStrict and noStrict flags instead
of duplicate yaml entries for each tests. We only need two 
conditional calls of addRunCommand in runTest262.

[Csaba Osztrogonác]

Comment on attachment 279140 [details]
Patch

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

> Tools/Scripts/import-test262-tests:127
> +            @includeFiles << "donePrintHandle.js"

typo: donePrintHandle.js -> doneprintHandle.js

[Csaba Osztrogonác]

doneprintHandle.js is needed only for async tests, 
this include can be added by runTest262

[Csaba Osztrogonác]

I made a quick measurement. The startup time of run-javascriptcore-tests 
is 30 seconds now, adding test262.yaml increased it to 2 minutes.

I think we should make it faster before running test262 by default.

[Keith Miller]

> I think we could and should make the test262.yaml file less 
> verbose and redundant. (bug157807)
> 
> For example, harness/assert.js and harness.sta js are included
> by all "runTest262" test. I prefer adding these common harness
> files to includeFiles in runTest262 function instead.
> 
> Additionally we could remove the ugly relative paths to 
> harness files, all of them are in the same harness directory.

The reason why I use all the relative paths is that each test's run-script is run from the directory of the test itself. If we didn't precompute the relative path then we would need to do so for each test when we create the run-script. I saw a noticeable decrease in the time it took for run-jsc-stress-tests to start when I switched to relative paths.

> One more thing to reduce redundancy. There are ~20.000 tests, 
> but only 1000 tests with [onlyStrict] or [noStrict] flags.
> 
> runTest262 could handle onlyStrict and noStrict flags instead
> of duplicate yaml entries for each tests. We only need two 
> conditional calls of addRunCommand in runTest262.

There are two entries because we might only fail one of the two modes. It's not related to processing the flags.

[Csaba Osztrogonác]

(In reply to comment #7)
> Comment on attachment 279140 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=279140&action=review
> 
> > Tools/Scripts/import-test262-tests:127
> > +            @includeFiles << "donePrintHandle.js"
> 
> typo: donePrintHandle.js -> doneprintHandle.js

typo fix in bug157902