V8 CI broken by test runner changes #180

Activity

1. Trott commented on Oct 18, 2020

   

   Trott

   on Oct 18, 2020

   Member

   More actions

   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.
2. richardlau commented on Oct 18, 2020

   

   richardlau

   on Oct 18, 2020

   Last edited by richardlau

   Member

   More actions

   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?
3. richardlau commented on Oct 18, 2020

   

   richardlau

   on Oct 18, 2020

   Member

   More actions

   @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.

   👍React with 👍1Reacted by Michaël Zasso
4. targos commented on Oct 19, 2020

   

   targos

   on Oct 19, 2020

   MemberAuthor

   More actions

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

   closed this as completedon Oct 19, 2020
6. richardlau commented on Oct 19, 2020

   

   richardlau

   on Oct 19, 2020

   Member

   More actions

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

   

   targos

   on Oct 19, 2020

   MemberAuthor

   More actions

   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.

   👍React with 👍1Reacted by Richard Lau