Make tests usable with debug release

[skomski]

mode=debug multiply platformTimeout by 5
Add debug VmFlags to GetCommand (--verify-heap --debug-code --enable-slow-asserts)
Fixed tests that rely on zero arguments or hard coded timeouts
Possible to override platformTimeout with TEST_TIMEOUT_MULTIPLIER for longer timeouts for example with an asan debug release

Makes it possible to run tools/test.py -J --mode=debug sequential message parallel.

[bnoordhuis]

Style: space before operator.

[skomski]

@bnoordhuis Updated.

[bnoordhuis]

The TEST_TIMEOUT_MULTIPLIER logic can go now, I think?

[skomski]

I added that for example asan debug builds when the default debug timeout isn't enough.

[bnoordhuis]

Can you mention that in the commit log? Aside: some tests may not propagate it to child processes they start.

[skomski]

Added in commit log.

[bnoordhuis]

@nodejs/build - how much effort is it to set up a version of node-test-pull-request that does everything the same except it's configured with ./configure --debug?

[orangemocha]

We could add a parameter to the Jenkins jobs. I think we would also have to change the makefile so that the parameter can be passed to the run-ci rule.

Just curious: in which scenarios do you foresee needing this? It would be nice to test in debug mode but I am afraid that the increase in run times would make it not very practical for every day use.

[bnoordhuis]

Stress-testing (our interaction with) V8. Debug builds do a large number of extra checks.

I think we would also have to change the makefile so that the parameter can be passed to the run-ci rule.

I think this is all that is needed:

diff --git a/Makefile b/Makefile
index 1d8b89b..2740618 100644
--- a/Makefile
+++ b/Makefile
@@ -6,7 +6,7 @@ DESTDIR ?=
 SIGN ?=
 PREFIX ?= /usr/local
 FLAKY_TESTS ?= run
-TEST_CI_ARGS ?=
+TEST_CI_ARGS ?= --mode=release
 STAGINGSERVER ?= node-www

 OSTYPE := $(shell uname -s | tr '[A-Z]' '[a-z]')
@@ -143,7 +143,7 @@ test-all-valgrind: test-build
        $(PYTHON) tools/test.py --mode=debug,release --valgrind

 test-ci: | build-addons
-       $(PYTHON) tools/test.py -p tap --logfile test.tap --mode=release --flaky-tests=$(FLAKY_TESTS) \
+       $(PYTHON) tools/test.py -p tap --logfile test.tap --flaky-tests=$(FLAKY_TESTS) \
                $(TEST_CI_ARGS) addons message parallel sequential

 test-release: test-build

[orangemocha]

Doesn't configure need a debug switch as well?

[orangemocha]

The makefile change also needs to be done in a way that has a graceful fallback for branches where it's not available, or it needs to be ported to all branches before we can pass the argument in Jenkins.

To be more clear, we are talking about having Jenkins call TEST_CI_ARGS=--mode=$JENKINS_CONFIG make run-ci. If in some branch the makefile doesn't have have the change above, we would end up passing --mode=release twice to the test runner, which might break it.

[bnoordhuis]

Doesn't configure need a debug switch as well?

Yes, but that doesn't need changes to the Makefile. Either make run-ci CONFIG_FLAGS="--debug" or make run-ci BUILDTYPE=Debug should work.

We could reuse BUILDTYPE for the --mode switch as well. That's arguably the most elegant.

[bnoordhuis]

I just realized that one drawback of ./configure --debug is that it builds both release and debug binaries. Waste of CPU cycles. I'll look into it.

[skomski]

Just curious: in which scenarios do you foresee needing this? It would be nice to test in debug mode but I am afraid that the increase in run times would make it not very practical for every day use.

To reduce run times but keep the debug checks we could the use build variable v8_optimized_debug which is currently disabled in node.

[bnoordhuis]

@skomski Caveat emptor from 69581b2:

 build: don't compile debug build with -Og

It's not supported by clang and commit e67542a ("build: disable -Og
when building with clang") is not sufficient because the configure
script no longer writes the 'clang' variable to common.gypi.

I could fix the configure script but I don't care enough actually do
so.  A fixed configure script won't help anyway when the compiler is
overridden through the CXX environment variable at compile time.

[jasnell]

@bnoordhuis @orangemocha ... ping

[orangemocha]

I had prototyped the BUILDTYPE switch in a separate CI job, but it was lost during one of the Jenkins security crises. It's an easy change to make. Does this need anything else from CI?

/cc @jbergstroem, who also worked on supporting debug runs in CI.

[jbergstroem]

I'd like the BUILDTYPE option in CI as well, but debug is slow and require more ram so we need to limit it to fewer buildbots.

[jasnell]

@skomski ... is this something you'd still like to pursue?

[jasnell]

Closing due to lack of forward progress on this. Can reopen and revisit if necessary