Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.3k
[v4.x backport] test: run v8 tests from node tree#7451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[v4.x backport] test: run v8 tests from node tree #7451
Uh oh!
There was an error while loading. Please reload this page.
Conversation
mscdex commented Jun 28, 2016
Are the log files necessary? |
targos commented Jun 28, 2016
The log files are necessary. They were filtered out by a rule in our |
targos commented Jun 28, 2016
Reference for the failures: https://bugs.chromium.org/p/v8/issues/detail?id=2899 See https://codereview.chromium.org/1402373002 for a solution. |
bnoordhuis commented Jun 28, 2016
Hah, I was just about to post that v8/v8@9c927d0 should fix the test failures; the issue is with the default system locale. |
bnoordhuis commented Jun 28, 2016
LGTM BTW |
targos commented Jun 28, 2016
LGTM too |
MylesBorins commented Jun 28, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
I've added a backport of v8/v8@9c927d0 V8 ci: https://ci.nodejs.org/job/node-test-commit-v8-linux/159/ |
8b991d8 to 5f92123CompareMylesBorins commented Jun 28, 2016
@targos@bnoordhuis PTAL |
deps/v8/tools/run-tests.py Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the indentation is wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
5f92123 to 6f6a2d7Comparebnoordhuis commented Jun 28, 2016
LGTM |
1 similar comment
targos commented Jun 28, 2016
LGTM |
mhdawson commented Jun 28, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
From what I can see its not running quite right in the CI. The output of the jobs look a bit different. What that on purpose (the full output of the tests being run versus the dots ?) |
mhdawson commented Jun 28, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Looking more closely I think the answer to my question is no, as the result is that the job shows as failed even though the tests appear to have passed. In particular the tap file with the results is not being generated: + cp deps/v8/v8-tap.xml v8-tap.xml cp: cannot stat 'deps/v8/v8-tap.xml': No such file or directory |
jasnell commented Jun 28, 2016
LGTM |
MylesBorins commented Jun 28, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
One more run in CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/161/ @mhdawson I noticed that as well re: tap output. What is strange is that the first run of this change seemed to produce the tap output (this is where the failures came up). Can you think of any reason the tap output would not be produced? |
MylesBorins commented Jun 28, 2016
I've opened #7460 which should solve the problem with the tap file. Once that has landed I'll add it to the backport |
Ported by exinfinitum from a PR by jasnell: see nodejs/node-v0.x-archive#14185 Allows the running of v8 tests on node's packaged v8 source code. Note that the limited win32 support added by jasnell has NOT been ported, and so these tests are currently UNIX ONLY. Note that gclient depot tools (see https://commondatastorage.googleapis.com/ chrome-infra-docs/flat/depot_tools/docs/html/ depot_tools_tutorial.html#_setting_up) and subversion are required to run tests. To perform tests, run the following commands: make v8 DESTCPU=(ARCH) make test-v8 DESTCPU=(ARCH) where (ARCH) is your CPU architecture, e.g. x64, ia32. DESTCPU MUST be specified for this to work properly. Can also do tests on debug build by using "make test-v8 DESTCPU=(ARCH) BUILDTYPE=Debug", or perform intl or benchmark tests via make test-v8-intl or test-v8-benchmarks respectively. Note that by default, quickcheck and TAP output are disabled, and i18n is enabled. To activate these options, use options"QUICKCHECK=True" and "ENABLE_V8_TAP=True" respectively. Use "DISABLE_V8_I18N" to disable i18n. Use V8_BUILD_OPTIONS to allow custom user-defined flags to be appended onto "make v8". Any tests performed after changes to the packaged v8 file will require recompiling of v8, which can be done using "make v8 DESTCPU=(ARCH)". Finally, two additional files necessary for one of the v8 tests have been added to the v8 folder. PR-URL: nodejs#4704 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: targos - Michaël Zasso <[email protected]>
6f6a2d7 to 1496fecCompareMylesBorins commented Jun 29, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
I've now included a backport of the fix from upstream... running CI one more time. Will land in v4.x-staging in green. ci: https://ci.nodejs.org/job/node-test-commit-v8-linux/167/ |
Original commit message: [test] Set default locale in test runner BUG=v8:4437,v8:2899,chromium:604310 LOG=n Review URL: https://codereview.chromium.org/1402373002 Cr-Commit-Position: refs/heads/master@{nodejs#35614} PR-URL: nodejs#7451 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
Currently we do not specific an absolute path for the tap output of the V8 test suite. This is proving to be unreliable across release lines. By prepending `$(PWD)` to each path we can guarantee it will always be in the root folder. PR-URL: nodejs#7460 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
1496fec to 72de1ceCompareMylesBorins commented Jun 29, 2016
landed in 48949fb...72de1ce |
Original commit message: [test] Set default locale in test runner BUG=v8:4437,v8:2899,chromium:604310 LOG=n Review URL: https://codereview.chromium.org/1402373002 Cr-Commit-Position: refs/heads/master@{#35614} PR-URL: #7451 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
Notable Changes: This list is not yet complete. Please comment in the thread with commits you think should be included. Descriptions will also be updated in a later release candidate. Semver Minor: * buffer: * backport new buffer constructor APIs to v4.x (Сковорода Никита Андреевич) #7562 * ignore negative allocation lengths (Anna Henningsen) #7562 * backport --zero-fill-buffers cli option (James M Snell) #5745 * build: * add Intel Vtune profiling support (Chunyang Dai) #5527 * repl: * copying tabs shouldn't trigger completion (Eugene Obrezkov) #5958 * src: * add node::FreeEnvironment public API (Cheng Zhao) #3098 * test: * run v8 tests from node tree (Bryon Leung) #4704 * V8: * backport 9c927d0f01 from V8 upstream (Myles Borins) #7451 * cherry-pick 68e89fb from v8's upstream (Fedor Indutny) #3779 Semver Patch: * **libuv**: * upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé) #6796 * upgrade libuv to 1.9.0 (Saúl Ibarra Corretgé) #5994
Original commit message: deps: backport 9c927d0f01 from V8 upstream Original commit message: [test] Set default locale in test runner BUG=v8:4437,v8:2899,chromium:604310 LOG=n Review URL: https://codereview.chromium.org/1402373002 Cr-Commit-Position: refs/heads/master@{#35614} PR-URL: nodejs/node#7451 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
tools
Description of change
This is a backport of cd720f8 from #4704 which enables running the V8 test suite on
v4.xci: https://ci.nodejs.org/job/node-test-commit-v8-linux/157/
(there are currently 4 failures, not sure what they are related to)
/cc @nodejs/lts @nodejs/V8 @ofrobots@mhdawson