-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
build: make ci test addons in test/addons #2428
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,28 +99,49 @@ test/gc/node_modules/weak/build/Release/weakref.node: $(NODE_EXE) | |
--directory="$(shell pwd)/test/gc/node_modules/weak" \ | ||
--nodedir="$(shell pwd)" | ||
|
||
build-addons: $(NODE_EXE) | ||
rm -rf test/addons/doc-*/ | ||
# Implicitly depends on $(NODE_EXE), see the build-addons rule for rationale. | ||
test/addons/.docbuildstamp: doc/api/addons.markdown | ||
$(RM) -r test/addons/doc-*/ | ||
$(NODE) tools/doc/addon-verify.js | ||
$(foreach dir, \ | ||
$(sort $(dir $(wildcard test/addons/*/*.gyp))), \ | ||
$(NODE) deps/npm/node_modules/node-gyp/bin/node-gyp rebuild \ | ||
--directory="$(shell pwd)/$(dir)" \ | ||
--nodedir="$(shell pwd)" && ) echo "build done" | ||
touch $@ | ||
|
||
ADDONS_BINDING_GYPS := \ | ||
$(filter-out test/addons/doc-*/binding.gyp, \ | ||
$(wildcard test/addons/*/binding.gyp)) | ||
|
||
# Implicitly depends on $(NODE_EXE), see the build-addons rule for rationale. | ||
test/addons/.buildstamp: $(ADDONS_BINDING_GYPS) | test/addons/.docbuildstamp | ||
# Cannot use $(wildcard test/addons/*/) here, it's evaluated before | ||
# embedded addons have been generated from the documentation. | ||
for dirname in test/addons/*/; do \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why switch away from the gmake syntax? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that a hint that the comment isn't clear enough? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aside: it can probably be made to work with .SECONDEXPANSION but I don't think that's going to be easier to read or maintain. Quite the opposite, really. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just couldn't verify the evaluation chain when testing -- perhaps this is the solution to that issue we couldn't pinpoint last time. Lets leave it as-is. |
||
$(NODE) deps/npm/node_modules/node-gyp/bin/node-gyp rebuild \ | ||
--directory="$$PWD/$$dirname" \ | ||
--nodedir="$$PWD"; \ | ||
done | ||
touch $@ | ||
|
||
# .buildstamp and .docbuildstamp need $(NODE_EXE) but cannot depend on it | ||
# directly because it calls make recursively. The parent make cannot know | ||
# if the subprocess touched anything so it pessimistically assumes that | ||
# .buildstamp and .docbuildstamp are out of date and need a rebuild. | ||
# Just goes to show that recursive make really is harmful... | ||
# TODO(bnoordhuis) Force rebuild after gyp or node-gyp update. | ||
build-addons: $(NODE_EXE) test/addons/.buildstamp | ||
|
||
test-gc: all test/gc/node_modules/weak/build/Release/weakref.node | ||
$(PYTHON) tools/test.py --mode=release gc | ||
|
||
test-build: all build-addons | ||
test-build: | all build-addons | ||
|
||
test-all: test-build test/gc/node_modules/weak/build/Release/weakref.node | ||
$(PYTHON) tools/test.py --mode=debug,release | ||
|
||
test-all-valgrind: test-build | ||
$(PYTHON) tools/test.py --mode=debug,release --valgrind | ||
|
||
test-ci: | ||
$(PYTHON) tools/test.py -p tap --logfile test.tap --mode=release message parallel sequential | ||
test-ci: | build-addons | ||
$(PYTHON) tools/test.py -p tap --logfile test.tap --mode=release \ | ||
addons message parallel sequential | ||
|
||
test-release: test-build | ||
$(PYTHON) tools/test.py --mode=release | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
.buildstamp | ||
.docbuildstamp | ||
Makefile | ||
*.Makefile | ||
*.mk | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this target should probably live in PHONY, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that would force a rebuild every time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok