[test] Process to locally develop and upstream test262 tests

[test] Process to locally develop and upstream test262 tests

This patch provides improved infrastructure for developing test262 tests
together with V8. It has three parts:
- The test262 test runner is updated to look for local versions of tests
  in the /test/test262/local-tests directory, which mirrors
  /test/test262/data. Additional tests can be added there and are run
  together with tests from upstream. Upstream tests can be locally
  updated by using the same name in local-tests; if a same-named test
  exists, then only the local version will be run. The local-tests
  directory is in the V8 repository, unlike the contents of the data
  directory, so tests can be added in the same patch as something else.
- The tool /test/test262/upstream-local-tests.sh is added to create
  a patch against the test262 respository based on a patch which changes
  the local-tests directory.
- The tool /test/test262/prune-local-tests.sh is added to remove
  redundant local tests on a test262 roll.

See design doc:
https://docs.google.com/document/d/16bj7AIDgZLv4WOsUEzQ5NzcEN9_xo095e88Pz8FC5rA/edit

Review-Url: https://codereview.chromium.org/2611793002
Cr-Commit-Position: refs/heads/master@{#42117}
Committed: https://chromium.googlesource.com/v8/v8/+/586deecbb182cf471d390da59fe8143fcfc120c3

[Dan Ehrenberg, 2017-01-03 19:25:38]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-03 19:25:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-03 19:28:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-03 19:28:38]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/18550)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/18520)
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/20252)

[Dan Ehrenberg, 2017-01-04 19:50:36]

Description was changed from

==========
WIP: [test262] Process to locally develop and upstream test262 tests

This patch provides improved infrastructure for developing test262 tests
together with V8. It has three parts:
- The test262 test runner is updated to look for local versions of tests
  in the /test/test262/local-tests directory, which mirrors
  /test/test262/data. Additional tests can be added there and are run
  together with tests from upstream. Upstream tests can be locally
  updated by using the same name in local-tests; if a same-named test
  exists, then only the local version will be run. The local-tests
  directory is in the V8 repository, unlike the contents of the data
  directory, so tests can be added in the same patch as something else.
- The tool /test/test262/upstream-local-tests.sh is added to create
  a patch against the test262 respository based on a patch which changes
  the local-tests directory.
- The tool /test/test262/prune-local-tests.sh is added to remove
  redundant local tests on a test262 roll.
==========

to

==========
WIP: [test262] Process to locally develop and upstream test262 tests

This patch provides improved infrastructure for developing test262 tests
together with V8. It has three parts:
- The test262 test runner is updated to look for local versions of tests
  in the /test/test262/local-tests directory, which mirrors
  /test/test262/data. Additional tests can be added there and are run
  together with tests from upstream. Upstream tests can be locally
  updated by using the same name in local-tests; if a same-named test
  exists, then only the local version will be run. The local-tests
  directory is in the V8 repository, unlike the contents of the data
  directory, so tests can be added in the same patch as something else.
- The tool /test/test262/upstream-local-tests.sh is added to create
  a patch against the test262 respository based on a patch which changes
  the local-tests directory.
- The tool /test/test262/prune-local-tests.sh is added to remove
  redundant local tests on a test262 roll.

See design doc:
https://docs.google.com/document/d/16bj7AIDgZLv4WOsUEzQ5NzcEN9_xo095e88Pz8FC5...
==========

[Dan Ehrenberg, 2017-01-04 19:59:13]

Description was changed from

==========
WIP: [test262] Process to locally develop and upstream test262 tests

This patch provides improved infrastructure for developing test262 tests
together with V8. It has three parts:
- The test262 test runner is updated to look for local versions of tests
  in the /test/test262/local-tests directory, which mirrors
  /test/test262/data. Additional tests can be added there and are run
  together with tests from upstream. Upstream tests can be locally
  updated by using the same name in local-tests; if a same-named test
  exists, then only the local version will be run. The local-tests
  directory is in the V8 repository, unlike the contents of the data
  directory, so tests can be added in the same patch as something else.
- The tool /test/test262/upstream-local-tests.sh is added to create
  a patch against the test262 respository based on a patch which changes
  the local-tests directory.
- The tool /test/test262/prune-local-tests.sh is added to remove
  redundant local tests on a test262 roll.

See design doc:
https://docs.google.com/document/d/16bj7AIDgZLv4WOsUEzQ5NzcEN9_xo095e88Pz8FC5...
==========

to

==========
[test262] Process to locally develop and upstream test262 tests

This patch provides improved infrastructure for developing test262 tests
together with V8. It has three parts:
- The test262 test runner is updated to look for local versions of tests
  in the /test/test262/local-tests directory, which mirrors
  /test/test262/data. Additional tests can be added there and are run
  together with tests from upstream. Upstream tests can be locally
  updated by using the same name in local-tests; if a same-named test
  exists, then only the local version will be run. The local-tests
  directory is in the V8 repository, unlike the contents of the data
  directory, so tests can be added in the same patch as something else.
- The tool /test/test262/upstream-local-tests.sh is added to create
  a patch against the test262 respository based on a patch which changes
  the local-tests directory.
- The tool /test/test262/prune-local-tests.sh is added to remove
  redundant local tests on a test262 roll.

See design doc:
https://docs.google.com/document/d/16bj7AIDgZLv4WOsUEzQ5NzcEN9_xo095e88Pz8FC5...
==========

[Dan Ehrenberg, 2017-01-04 19:59:13]

littledan@chromium.org changed reviewers:
+ adamk@chromium.org, machenbach@google.com

[Dan Ehrenberg, 2017-01-04 22:31:30]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 22:31:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-04 22:55:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-04 22:55:04]

Dry run: This issue passed the CQ dry run.

[Michael Achenbach, 2017-01-05 14:06:31]

machenbach@chromium.org changed reviewers:
+ machenbach@chromium.org

[Michael Achenbach, 2017-01-05 14:06:32]

lgtm

rubberstamped the .sh files.

https://codereview.chromium.org/2611793002/diff/40001/test/test262/prune-loca...
File test/test262/prune-local-tests.sh (right):

https://codereview.chromium.org/2611793002/diff/40001/test/test262/prune-loca...
test/test262/prune-local-tests.sh:1: # usage: test/test262/prune-local-tests.sh
Might need copyright header?

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py
File test/test262/testcfg.py (left):

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py...
test/test262/testcfg.py:183: filename = os.path.join(self.testroot,
testcase.path + ".js")
This seemed to have worked because python seems tolerant. Strictly, the
testcase.path has only forward slashes also on windows. But I guess it's fine as
it worked before.

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py
File test/test262/testcfg.py (right):

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py...
test/test262/testcfg.py:133: # Match the (...) in
'/path/to/v8/test/test262/subdir/test/(...).js'
Just to clarify: subdir is one of data,local-tests

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py...
test/test262/testcfg.py:135:
r'.*[\\/]test[\\/]test262[\\/][^\\/]+[\\/]test[\\/](.*)\.js',
Maybe keep the regexp in a toplevel constant for readability.

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py...
test/test262/testcfg.py:138: if dirname.startswith(self.testroot) and \
nit: Rather use () around the whole condition instead of \

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py...
test/test262/testcfg.py:139: os.path.exists(os.path.join(self.localtestroot,
relpath+'.js')):
Do these "exists" checks give a measurable slowdown? E.g. on windows? We now
have this extra os call for every test file.

If there is any issue with that, we could also walk the tests in localtestroot
separately and then simply compare in memory which ones we found. I assume the
number of tests in localtestroot is significantly smaller than the tests in
data.

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py...
test/test262/testcfg.py:139: os.path.exists(os.path.join(self.localtestroot,
relpath+'.js')):
nit: spaces around +

or how about adding the .js to the group above? Wouldn't the regexp work like
this:
test[\\/](.*\.js)'

of course then you need the code relpath[:-3] below again, like on the left
side... as you wish.

[Dan Ehrenberg, 2017-01-05 16:21:07]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 16:21:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dan Ehrenberg, 2017-01-05 16:24:16]

Would anyone be interested in taking a closer look at the .sh files? Also, I
added a couple Intl tests for the new semantics, rather than using a
placeholder, and I'd be interested if anyone wanted to take a look.

https://codereview.chromium.org/2611793002/diff/40001/test/test262/prune-loca...
File test/test262/prune-local-tests.sh (right):

https://codereview.chromium.org/2611793002/diff/40001/test/test262/prune-loca...
test/test262/prune-local-tests.sh:1: # usage: test/test262/prune-local-tests.sh
On 2017/01/05 14:06:32, Michael Achenbach (OOO) wrote:
> Might need copyright header?

Done.

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py
File test/test262/testcfg.py (left):

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py...
test/test262/testcfg.py:183: filename = os.path.join(self.testroot,
testcase.path + ".js")
On 2017/01/05 14:06:32, Michael Achenbach (OOO) wrote:
> This seemed to have worked because python seems tolerant. Strictly, the
> testcase.path has only forward slashes also on windows. But I guess it's fine
as
> it worked before.

I'll venture a guess that this is because Windows tolerates pathnames with / in
them, and we might be out of luck on Mac OS 9...

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py
File test/test262/testcfg.py (right):

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py...
test/test262/testcfg.py:133: # Match the (...) in
'/path/to/v8/test/test262/subdir/test/(...).js'
On 2017/01/05 14:06:32, Michael Achenbach (OOO) wrote:
> Just to clarify: subdir is one of data,local-tests

Right, added this in a comment.

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py...
test/test262/testcfg.py:135:
r'.*[\\/]test[\\/]test262[\\/][^\\/]+[\\/]test[\\/](.*)\.js',
On 2017/01/05 14:06:32, Michael Achenbach (OOO) wrote:
> Maybe keep the regexp in a toplevel constant for readability.

Done.

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py...
test/test262/testcfg.py:138: if dirname.startswith(self.testroot) and \
On 2017/01/05 14:06:32, Michael Achenbach (OOO) wrote:
> nit: Rather use () around the whole condition instead of \

This code no longer exists.

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py...
test/test262/testcfg.py:139: os.path.exists(os.path.join(self.localtestroot,
relpath+'.js')):
On 2017/01/05 14:06:32, Michael Achenbach (OOO) wrote:
> nit: spaces around +
> 
> or how about adding the .js to the group above? Wouldn't the regexp work like
> this:
> test[\\/](.*\.js)'
> 
> of course then you need the code relpath[:-3] below again, like on the left
> side... as you wish.

This doesn't either.

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py...
test/test262/testcfg.py:139: os.path.exists(os.path.join(self.localtestroot,
relpath+'.js')):
On 2017/01/05 14:06:32, Michael Achenbach (OOO) wrote:
> Do these "exists" checks give a measurable slowdown? E.g. on windows? We now
> have this extra os call for every test file.
> 
> If there is any issue with that, we could also walk the tests in localtestroot
> separately and then simply compare in memory which ones we found. I assume the
> number of tests in localtestroot is significantly smaller than the tests in
> data.

Turns out all we actually have to do is prune duplicates, so I did that using a
set instead. I don't have a great setup to test performance across platforms; if
you could let me know if you see any regressions, that'd be great.

[commit-bot: I haz the power, 2017-01-05 16:45:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-05 16:45:51]

Dry run: This issue passed the CQ dry run.

[Dan Ehrenberg, 2017-01-05 17:05:22]

Description was changed from

==========
[test262] Process to locally develop and upstream test262 tests

This patch provides improved infrastructure for developing test262 tests
together with V8. It has three parts:
- The test262 test runner is updated to look for local versions of tests
  in the /test/test262/local-tests directory, which mirrors
  /test/test262/data. Additional tests can be added there and are run
  together with tests from upstream. Upstream tests can be locally
  updated by using the same name in local-tests; if a same-named test
  exists, then only the local version will be run. The local-tests
  directory is in the V8 repository, unlike the contents of the data
  directory, so tests can be added in the same patch as something else.
- The tool /test/test262/upstream-local-tests.sh is added to create
  a patch against the test262 respository based on a patch which changes
  the local-tests directory.
- The tool /test/test262/prune-local-tests.sh is added to remove
  redundant local tests on a test262 roll.

See design doc:
https://docs.google.com/document/d/16bj7AIDgZLv4WOsUEzQ5NzcEN9_xo095e88Pz8FC5...
==========

to

==========
[test] Process to locally develop and upstream test262 tests

This patch provides improved infrastructure for developing test262 tests
together with V8. It has three parts:
- The test262 test runner is updated to look for local versions of tests
  in the /test/test262/local-tests directory, which mirrors
  /test/test262/data. Additional tests can be added there and are run
  together with tests from upstream. Upstream tests can be locally
  updated by using the same name in local-tests; if a same-named test
  exists, then only the local version will be run. The local-tests
  directory is in the V8 repository, unlike the contents of the data
  directory, so tests can be added in the same patch as something else.
- The tool /test/test262/upstream-local-tests.sh is added to create
  a patch against the test262 respository based on a patch which changes
  the local-tests directory.
- The tool /test/test262/prune-local-tests.sh is added to remove
  redundant local tests on a test262 roll.

See design doc:
https://docs.google.com/document/d/16bj7AIDgZLv4WOsUEzQ5NzcEN9_xo095e88Pz8FC5...
==========

[Michael Achenbach, 2017-01-06 09:26:02]

lgtm

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py
File test/test262/testcfg.py (right):

https://codereview.chromium.org/2611793002/diff/40001/test/test262/testcfg.py...
test/test262/testcfg.py:139: os.path.exists(os.path.join(self.localtestroot,
relpath+'.js')):
On 2017/01/05 16:24:16, Dan Ehrenberg wrote:
> On 2017/01/05 14:06:32, Michael Achenbach (OOO) wrote:
> > Do these "exists" checks give a measurable slowdown? E.g. on windows? We now
> > have this extra os call for every test file.
> > 
> > If there is any issue with that, we could also walk the tests in
localtestroot
> > separately and then simply compare in memory which ones we found. I assume
the
> > number of tests in localtestroot is significantly smaller than the tests in
> > data.
> 
> Turns out all we actually have to do is prune duplicates, so I did that using
a
> set instead. I don't have a great setup to test performance across platforms;
if
> you could let me know if you see any regressions, that'd be great.

Yes, much nicer! The exists check is already encapsulated in the getpath method.
If we get any perf regression we could precalculate the existing local tests
once and then check those against each other test, but lets not go so far now.

https://codereview.chromium.org/2611793002/diff/60001/test/test262/testcfg.py
File test/test262/testcfg.py (right):

https://codereview.chromium.org/2611793002/diff/60001/test/test262/testcfg.py...
test/test262/testcfg.py:108: RELPATH_REGEXP = re.compile(
nit: Maybe move it even more top-level, right next to TEST_262_LOCAL_TESTS_PATH,
etc. This whole file has test262 context. Then you don't need to qualify it
below.

[Dan Ehrenberg, 2017-01-06 09:58:10]

https://codereview.chromium.org/2611793002/diff/60001/test/test262/testcfg.py
File test/test262/testcfg.py (right):

https://codereview.chromium.org/2611793002/diff/60001/test/test262/testcfg.py...
test/test262/testcfg.py:108: RELPATH_REGEXP = re.compile(
On 2017/01/06 09:26:02, Michael Achenbach (OOO) wrote:
> nit: Maybe move it even more top-level, right next to
TEST_262_LOCAL_TESTS_PATH,
> etc. This whole file has test262 context. Then you don't need to qualify it
> below.

Done.

[adamk, 2017-01-06 18:52:15]

lgtm, let's land this for now and we can iterate if we find issues with the
specifics of the setup.

[Dan Ehrenberg, 2017-01-07 02:22:52]

The CQ bit was checked by littledan@chromium.org

[Dan Ehrenberg, 2017-01-07 02:22:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
machenbach@chromium.org
Link to the patchset: https://codereview.chromium.org/2611793002/#ps80001
(title: "Cleanup nit")

[commit-bot: I haz the power, 2017-01-07 02:23:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-07 02:44:49]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1483755772681050,
"parent_rev": "b5606fe07e1f6422594145c2fd5291ba24ee6626", "commit_rev":
"586deecbb182cf471d390da59fe8143fcfc120c3"}

[commit-bot: I haz the power, 2017-01-07 02:44:53]

Description was changed from

==========
[test] Process to locally develop and upstream test262 tests

This patch provides improved infrastructure for developing test262 tests
together with V8. It has three parts:
- The test262 test runner is updated to look for local versions of tests
  in the /test/test262/local-tests directory, which mirrors
  /test/test262/data. Additional tests can be added there and are run
  together with tests from upstream. Upstream tests can be locally
  updated by using the same name in local-tests; if a same-named test
  exists, then only the local version will be run. The local-tests
  directory is in the V8 repository, unlike the contents of the data
  directory, so tests can be added in the same patch as something else.
- The tool /test/test262/upstream-local-tests.sh is added to create
  a patch against the test262 respository based on a patch which changes
  the local-tests directory.
- The tool /test/test262/prune-local-tests.sh is added to remove
  redundant local tests on a test262 roll.

See design doc:
https://docs.google.com/document/d/16bj7AIDgZLv4WOsUEzQ5NzcEN9_xo095e88Pz8FC5...
==========

to

==========
[test] Process to locally develop and upstream test262 tests

This patch provides improved infrastructure for developing test262 tests
together with V8. It has three parts:
- The test262 test runner is updated to look for local versions of tests
  in the /test/test262/local-tests directory, which mirrors
  /test/test262/data. Additional tests can be added there and are run
  together with tests from upstream. Upstream tests can be locally
  updated by using the same name in local-tests; if a same-named test
  exists, then only the local version will be run. The local-tests
  directory is in the V8 repository, unlike the contents of the data
  directory, so tests can be added in the same patch as something else.
- The tool /test/test262/upstream-local-tests.sh is added to create
  a patch against the test262 respository based on a patch which changes
  the local-tests directory.
- The tool /test/test262/prune-local-tests.sh is added to remove
  redundant local tests on a test262 roll.

See design doc:
https://docs.google.com/document/d/16bj7AIDgZLv4WOsUEzQ5NzcEN9_xo095e88Pz8FC5...

Review-Url: https://codereview.chromium.org/2611793002
Cr-Commit-Position: refs/heads/master@{#42117}
Committed:
https://chromium.googlesource.com/v8/v8/+/586deecbb182cf471d390da59fe8143fcfc...
==========

[commit-bot: I haz the power, 2017-01-07 02:44:54]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/v8/v8/+/586deecbb182cf471d390da59fe8143fcfc...