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.

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

V8 CI broken by test runner changes #180

Closed
targos opened this issue Oct 18, 2020 · 6 comments
Closed

V8 CI broken by test runner changes #180

targos opened this issue Oct 18, 2020 · 6 comments

Comments

@targos
Copy link
Member

targos commented Oct 18, 2020

See https://ci.nodejs.org/job/node-test-commit-v8-linux/3479/nodes=benchmark-ubuntu1604-intel-64,v8test=v8test/console

Error:

11:14:22 deps/v8/tools/run-tests.py --gn --arch=x64 \
11:14:22 			--mode=release --progress=dots --timeout=120 \
11:14:22 			mjsunit cctest debugger inspector message preparser \
11:14:22 			--junitout /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/benchmark-ubuntu1604-intel-64/v8test/v8test/v8-tap.xml
11:14:22 Usage: run-tests.py [options] [tests]
11:14:22 
11:14:22 run-tests.py: error: no such option: --mode

/cc @nodejs/build

@Trott
Copy link
Member

Trott commented Oct 18, 2020

Here's the relevant bit of script:

# we run the `v8-updates` suite on machines labeled `*benchmark*`
case "$nodes" in 
  *benchmark*)
    NODE_TEST_DIR=/home/iojs/node-tmp NODE_COMMON_PORT=15000 python tools/test.py -j $JOBS -p tap --mode=release --flaky-tests=run v8-updates
    # Flag test as unstable if `perf` is not found
    perf --version || exit 54
  ;;
esac

I can remove the --mode=release if we're pretty sure that's all that needs to happen and this is time sensitive or anything.

@richardlau
Copy link
Member

richardlau commented Oct 18, 2020

Here's the relevant bit of script:

# we run the `v8-updates` suite on machines labeled `*benchmark*`
case "$nodes" in 
  *benchmark*)
    NODE_TEST_DIR=/home/iojs/node-tmp NODE_COMMON_PORT=15000 python tools/test.py -j $JOBS -p tap --mode=release --flaky-tests=run v8-updates
    # Flag test as unstable if `perf` is not found
    perf --version || exit 54
  ;;
esac

I can remove the --mode=release if we're pretty sure that's all that needs to happen and this is time sensitive or anything.

Isn’t that supposed to run our test runner and not V8’s?

@richardlau
Copy link
Member

@targos The job runs the test-v8 target and this is invoking the V8 test runner:
https://github.com/nodejs/node/blob/6141ca318ad52c507fd9fbc49218768b72eb8f54/Makefile#L670-L676
so this should be fixed in the canary branch.

@targos
Copy link
Member Author

targos commented Oct 19, 2020

Thanks @richardlau. This already affects V8 8.7. I added a commit to remove the flag in nodejs/node#35700

@targos targos closed this as completed Oct 19, 2020
@richardlau
Copy link
Member

@targos It looks like V8 8.6 was also affected -- the V8 CI for Node.js 15 (nodejs/node#35014 (comment)) failed.
cc @BethGriggs

@targos
Copy link
Member Author

targos commented Oct 19, 2020

Okay, sorry about that. It passed originally: https://ci.nodejs.org/job/node-test-commit-v8-linux/3416/ but I guess they backported the test runner changes.

Let me open a PR against master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants