build: run test-hash-seed at the end of test-v8

[targos]

The v8 and test-hash-seed targets cannot be run in parallel because they
need different copies of the deps/v8 directory.

Ref: #14004 (comment)

I'm open to a better solution but I don't think test-hash-seed shoud be run here, even if we make it sequential (V8 has to be built twice).

@ofrobots @nodejs/v8

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

[refack]

Sounds right to me. IMHO test-hash-seed is a node test (and is run under pummel) not a V8 test...

[ofrobots]

The original motivation was to make sure this gets run whenever we update V8 (i.e. the time we usually run test-v8 in the CI). IMO, test-hash-seed is too slow to be run in the normal CI. Let me see if there might be another way to do this..

[ofrobots]

Perhaps something like:

--- a/Makefile
+++ b/Makefile
@@ -433,13 +433,15 @@ test-async-hooks:


 ifneq ("","$(wildcard deps/v8/tools/run-tests.py)")
-test-v8: v8 test-hash-seed
+test-v8: v8
 #      note: performs full test unless QUICKCHECK is specified
        deps/v8/tools/run-tests.py --arch=$(V8_ARCH) \
         --mode=$(BUILDTYPE_LOWER) $(V8_TEST_OPTIONS) $(QUICKCHECK_ARG) \
         --no-presubmit \
         --shell-dir=$(PWD)/deps/v8/out/$(V8_ARCH).$(BUILDTYPE_LOWER) \
         $(TAP_V8)
+        @echo Testing hash seed
+        $(MAKE) test-hash-seed

[targos]

CI with that change: https://ci.nodejs.org/job/node-test-commit-v8-linux/783/

outdated

[targos]

CI is green. Can we fast-track this fix to unbreak it?

[targos]

BTW I understood why test-hash-seed was there and it makes sense. I was reluctant because of the double V8 build but the job took only 30 minutes to run so that's fine.

[refack]

$(MAKE) test-hash-seed

Just to be sure, spawning a sub-make propagates the exit code?

[targos]

@refack With this:

test-a: test-b
        echo "test-a"
        $(MAKE) test-c

test-b:
        echo "test-b"

test-c:
        node -e "process.exit(1)"

$ make test-a
echo "test-b"
test-b
echo "test-a"
test-a
make test-c
node -e "process.exit(1)"
make[1]: *** [Makefile:442: test-c] Error 1
make: *** [Makefile:436: test-a] Error 2

[mhdawson]

The alternative is to make a new nightly job that just runs the hashseed test on its own. That would remove the overhead for those doing individual regression runs instead of the nightlies. We then might want to create better nightly job which runs both the hashseed and v8 test jobs.

In any case I'm +1 to landing this now to get the test back to green and the improve.

[mhdawson]

I also think we should expedite landing this since the jobs are completely broken as they currently are.

[addaleax]

Landed in 016d81c