Skip to content
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.

Sign up for GitHub

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

Jump to bottom

207 failures in v8 tests within node tree #7477

Closed
mhdawson opened this issue Jun 29, 2016 · 18 comments
Closed

207 failures in v8 tests within node tree #7477

mhdawson opened this issue Jun 29, 2016 · 18 comments
Labels
test Issues and PRs related to the tests. v8 engine Issues and PRs related to the V8 dependency.

Comments

@mhdawson
Copy link
Member

mhdawson commented Jun 29, 2016 •
edited
Loading

  • Version: Master
  • Platform: Linux
  • Subsystem: v8

https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark,v8test=v8test/164/console

===
=== 207 tests failed
===

Sampling includes:

--- stderr ---
#
# Fatal error in ../test/cctest/interpreter/test-bytecode-generator.cc, line 1950
# Check failed: (BuildActual(printer, snippets, prologue.c_str()))==(LoadGolden("WideRegisters.golden")).
#

==== C stack trace ===============================

 1: 0x83d849
 2: 0x52c7e6
 3: 0x51eb4a
 4: 0x51ef50
 5: __libc_start_main
 6: 0x4403f9
Command: /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/benchmark/v8test/v8test/deps/v8/out/x64.release/cctest --random-seed=2122748865 --stress-opt --always-opt test-bytecode-generator/WideRegisters --nohard-abort --nodead-code-elimination --nofold-constants
=== cctest/test-bytecode-generator/WithStatement ===
--- stderr ---
#
# Fatal error in ../test/cctest/interpreter/test-bytecode-generator.cc, line 2053
# Check failed: (BuildActual(printer, snippets))==(LoadGolden("WithStatement.golden")).
#

==== C stack trace ===============================

 1: 0x83d849
 2: 0x52d950
 3: 0x51eb4a
 4: 0x51ef50
 5: __libc_start_main
 6: 0x4403f9
Command: /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/benchmark/v8test/v8test/deps/v8/out/x64.release/cctest --random-seed=2122748865 --stress-opt --always-opt test-bytecode-generator/WithStatement --nohard-abort --nodead-code-elimination --nofold-constants

===

and many others
@mhdawson mhdawson added the v8 engine Issues and PRs related to the V8 dependency. label Jun 29, 2016
@mhdawson
Copy link
Member Author

mhdawson commented Jun 29, 2016 •
edited
Loading

Possibly related to #7016 ?

@targos

@targos targos self-assigned this Jun 29, 2016
@targos
Copy link
Member

targos commented Jun 29, 2016

I suspect this is because of the ABI compatibility changes. cc @ofrobots

I'd like to test without the two related commits but I get the following error with make test-v8:

# note: performs full test unless QUICKCHECK is specified
deps/v8/tools/run-tests.py --arch=x64 \
        --mode=release   \
        --no-presubmit \
        --shell-dir=/home/mzasso/git/forks/node/deps/v8/out/x64.release \

>>> Running tests for x64.release
Traceback (most recent call last):
  File "deps/v8/tools/run-tests.py", line 842, in <module>
    sys.exit(Main())
  File "deps/v8/tools/run-tests.py", line 638, in Main
    code = Execute(arch, mode, args, options, suites)
  File "deps/v8/tools/run-tests.py", line 662, in Execute
    raise Exception('Could not find shell_dir: "%s"' % shell_dir)
Exception: Could not find shell_dir: "/home/mzasso/git/forks/node/deps/v8/out/x64.release"
Makefile:236: recipe for target 'test-v8' failed
make: *** [test-v8] Error 1

@mscdex mscdex added the test Issues and PRs related to the tests. label Jun 29, 2016
@mhdawson
Copy link
Member Author

to confirm did you :

  1. git clone https://chromium.googlesource.com/chromium/tools/depot_tools.git
  2. add depto_tools to your path

@targos
Copy link
Member

targos commented Jun 29, 2016

yes, I have depot_tools in my path

@MylesBorins
Copy link
Contributor

@targos is the depo tools in your path an absolute path? I had some weirdness with relative

@targos
Copy link
Member

targos commented Jun 29, 2016

it is absolute. I am currently on OSX and have the exact same error than on Linux.

@ofrobots
Copy link
Contributor

It seems like the test-v8 expects a V8 build in deps/v8/out/$(V8_ARCH).$(BUILDTYPE_LOWER). I suspect there is a prerequisite target that is missing from the makefile.

A normal build of Node doesn't build V8 at the above location.

@targos
Copy link
Member

targos commented Jun 29, 2016

Yeah I just realized that I have to run make v8 first.

targos added a commit to targos/node that referenced this issue Jun 29, 2016
@targos
build: add v8 requirement to test-v8* in Makefile
3ef9961 
The test targets expect that V8 is built in deps/v8/out

Ref: nodejs#7477
@targos targos mentioned this issue Jun 29, 2016
Closed
1 task
@targos
Copy link
Member

targos commented Jun 29, 2016

#7482

@ofrobots
Copy link
Contributor

Running tests against V8 branch-heads/5.1 + cac1709 + d79df45 in a V8 build doesn't show failures for me.

I am building V8 using node's makefile now.

@ofrobots
Copy link
Contributor

I don't see these failures w/ test-v8 either.

@ofrobots
Copy link
Contributor

Err.. actually ignore my last statement about testing with test-v8. My master branch wasn't quite up to date with origin. Building that now.

@targos
Copy link
Member

targos commented Jun 29, 2016

I can reproduce the failures on master. Trying now without the 2 commits.

@ofrobots
Copy link
Contributor

I can reproduce the failures on master as well now. However they continue to fail at the commit 2cc2951 (upgrade to 5.1.281.69, without any other changes from #7016). I suspect this might be related to a difference in environment in how the tests get run by the Node Makefile.

@ofrobots
Copy link
Contributor

I think LoadGolden expects to open a bytecode file. I am guessing it cannot find it in the correct location when run from the Node makefile.

targos added a commit to targos/node that referenced this issue Jun 29, 2016
@targos
deps: fix V8 5.1 tests
282af04 
Restore whitespaces in *.golden files. They were lost when I landed the
V8 5.1 update.

Fixes: nodejs#7477
@targos targos mentioned this issue Jun 29, 2016
Closed
2 tasks
@targos
Copy link
Member

targos commented Jun 29, 2016

#7488

@targos targos closed this as completed in dc17432 Jun 30, 2016
@mhdawson
Copy link
Member Author

Kicking off CI run to validate issue is resolved: https://ci.nodejs.org/job/node-test-commit-v8-linux/169/

@mhdawson
Copy link
Member Author

As expected run was clean thanks @targos.

targos added a commit to targos/node that referenced this issue Jul 10, 2016
@targos
build: add v8 requirement to test-v8* in Makefile
69ef9b1 
The test targets expect that V8 is built in deps/v8/out

Ref: nodejs#7477
PR-URL: nodejs#7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos added a commit to targos/node that referenced this issue Jul 10, 2016
@targos
build: use BUILDTYPE when building V8 in Makefile
22492db 
Without this it would always compile Release and Debug builds.

Ref: nodejs#7477
PR-URL: nodejs#7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos added a commit that referenced this issue Jul 12, 2016
@targos
build: add v8 requirement to test-v8* in Makefile
b68e685 
The test targets expect that V8 is built in deps/v8/out

Ref: #7477
PR-URL: #7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos added a commit that referenced this issue Jul 12, 2016
@targos
build: use BUILDTYPE when building V8 in Makefile
110ce55 
Without this it would always compile Release and Debug builds.

Ref: #7477
PR-URL: #7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 12, 2016
@targos
build: add v8 requirement to test-v8* in Makefile
dfba60e 
The test targets expect that V8 is built in deps/v8/out

Ref: #7477
PR-URL: #7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 12, 2016
@targos
build: use BUILDTYPE when building V8 in Makefile
d7f7795 
Without this it would always compile Release and Debug builds.

Ref: #7477
PR-URL: #7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 12, 2016
@targos
build: add v8 requirement to test-v8* in Makefile
de5d718 
The test targets expect that V8 is built in deps/v8/out

Ref: #7477
PR-URL: #7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 12, 2016
@targos
build: use BUILDTYPE when building V8 in Makefile
5841332 
Without this it would always compile Release and Debug builds.

Ref: #7477
PR-URL: #7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas pushed a commit that referenced this issue Jul 13, 2016
@targos @evanlucas
build: add v8 requirement to test-v8* in Makefile
94a486a 
The test targets expect that V8 is built in deps/v8/out

Ref: #7477
PR-URL: #7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas pushed a commit that referenced this issue Jul 13, 2016
@targos @evanlucas
build: use BUILDTYPE when building V8 in Makefile
4480b14 
Without this it would always compile Release and Debug builds.

Ref: #7477
PR-URL: #7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
@targos
build: add v8 requirement to test-v8* in Makefile
6fce484 
The test targets expect that V8 is built in deps/v8/out

Ref: #7477
PR-URL: #7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
@targos
build: use BUILDTYPE when building V8 in Makefile
728eab1 
Without this it would always compile Release and Debug builds.

Ref: #7477
PR-URL: #7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
@targos
build: add v8 requirement to test-v8* in Makefile
79bd39c 
The test targets expect that V8 is built in deps/v8/out

Ref: #7477
PR-URL: #7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
@targos
build: use BUILDTYPE when building V8 in Makefile
5290c9d 
Without this it would always compile Release and Debug builds.

Ref: #7477
PR-URL: #7482
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos removed their assignment Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
test Issues and PRs related to the tests. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

deps: fix V8 5.1 tests targos/node
5 participants
@mscdex @ofrobots @MylesBorins @targos @mhdawson