| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
Style: space before operator.
Sorry, something went wrong.
There was a problem hiding this comment.
The TEST_TIMEOUT_MULTIPLIER logic can go now, I think?
Sorry, something went wrong.
There was a problem hiding this comment.
I added that for example asan debug builds when the default debug timeout isn't enough.
Sorry, something went wrong.
There was a problem hiding this comment.
Can you mention that in the commit log? Aside: some tests may not propagate it to child processes they start.
Sorry, something went wrong.
There was a problem hiding this comment.
Added in commit log.
Sorry, something went wrong.
|
@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? |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
Stress-testing (our interaction with) V8. Debug builds do a large number of extra checks.
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 |
Sorry, something went wrong.
|
Doesn't configure need a debug switch as well? |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
To reduce run times but keep the debug checks we could the use build variable v8_optimized_debug which is currently disabled in node. |
Sorry, something went wrong.
|
@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.
|
Sorry, something went wrong.
Added for example for asan debug builds when the default debug timeout isn't enough.
|
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. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
@skomski ... is this something you'd still like to pursue? |
Sorry, something went wrong.
|
Closing due to lack of forward progress on this. Can reopen and revisit if necessary |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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.