Skip to content

Commit

Permalink
build: make test-doc and lint addon docs
Browse files Browse the repository at this point in the history
- Implements the make test-doc target that build, verify
  and lint docs
- Lint the C++ snippets in addon docs
- When generating addons and running the JS linter,
  use the global node executable if it is not built.
  Therefore one does not have to build node in order to
  run make test-doc.

PR-URL: #16377
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
  • Loading branch information
joyeecheung authored and gibfahn committed Oct 31, 2017
1 parent 63e33ac commit d576e17
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 15 deletions.
56 changes: 42 additions & 14 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ test: all
$(CI_ASYNC_HOOKS) \
$(CI_JS_SUITES) \
$(CI_NATIVE_SUITES) \
doctool known_issues
$(CI_DOC) \
known_issues
endif

# For a quick test, does not run linter or build doc
Expand Down Expand Up @@ -268,7 +269,6 @@ test/gc/build/Release/binding.node: test/gc/binding.cc test/gc/binding.gyp
--directory="$(shell pwd)/test/gc" \
--nodedir="$(shell pwd)"

# Implicitly depends on $(NODE_EXE), see the build-addons rule for rationale.
DOCBUILDSTAMP_PREREQS = tools/doc/addon-verify.js doc/api/addons.md

ifeq ($(OSTYPE),aix)
Expand All @@ -277,7 +277,7 @@ endif

test/addons/.docbuildstamp: $(DOCBUILDSTAMP_PREREQS)
$(RM) -r test/addons/??_*/
$(NODE) $<
[ -x $(NODE) ] && $(NODE) $< || node $<
touch $@

ADDONS_BINDING_GYPS := \
Expand Down Expand Up @@ -313,10 +313,10 @@ test/addons/.buildstamp: config.gypi \
done
touch $@

# .buildstamp and .docbuildstamp need $(NODE_EXE) but cannot depend on it
# .buildstamp needs $(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.
# .buildstamp is out of date and need a rebuild.
# Just goes to show that recursive make really is harmful...
# TODO(bnoordhuis) Force rebuild after gyp update.
build-addons: $(NODE_EXE) test/addons/.buildstamp
Expand Down Expand Up @@ -352,10 +352,10 @@ test/addons-napi/.buildstamp: config.gypi \
done
touch $@

# .buildstamp and .docbuildstamp need $(NODE_EXE) but cannot depend on it
# .buildstamp needs $(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.
# .buildstamp is 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-napi: $(NODE_EXE) test/addons-napi/.buildstamp
Expand Down Expand Up @@ -387,6 +387,7 @@ test-all-valgrind: test-build
CI_NATIVE_SUITES ?= addons addons-napi
CI_ASYNC_HOOKS := async-hooks
CI_JS_SUITES ?= default
CI_DOC := doctool

# Build and test addons without building anything else
test-ci-native: LOGLEVEL := info
Expand All @@ -412,7 +413,8 @@ test-ci: | clear-stalled build-addons build-addons-napi doc-only
out/Release/cctest --gtest_output=tap:cctest.tap
$(PYTHON) tools/test.py $(PARALLEL_ARGS) -p tap --logfile test.tap \
--mode=release --flaky-tests=$(FLAKY_TESTS) \
$(TEST_CI_ARGS) $(CI_ASYNC_HOOKS) $(CI_JS_SUITES) $(CI_NATIVE_SUITES) doctool known_issues
$(TEST_CI_ARGS) $(CI_ASYNC_HOOKS) $(CI_JS_SUITES) $(CI_NATIVE_SUITES) \
$(CI_DOC) known_issues
# Clean up any leftover processes, error if found.
ps awwx | grep Release/node | grep -v grep | cat
@PS_OUT=`ps awwx | grep Release/node | grep -v grep | awk '{print $$1}'`; \
Expand Down Expand Up @@ -448,6 +450,10 @@ test-tick-processor: all
test-hash-seed: all
$(NODE) test/pummel/test-hash-seed.js

test-doc: doc-only
$(MAKE) lint
$(PYTHON) tools/test.py $(CI_DOC)

test-known-issues: all
$(PYTHON) tools/test.py known_issues

Expand Down Expand Up @@ -981,26 +987,38 @@ lint-md: lint-md-build
./*.md doc src lib benchmark tools/doc/ tools/icu/

LINT_JS_TARGETS = benchmark doc lib test tools
LINT_JS_CMD = tools/eslint/bin/eslint.js --cache \
--rulesdir=tools/eslint-rules --ext=.js,.mjs,.md \
$(LINT_JS_TARGETS)

lint-js:
@echo "Running JS linter..."
$(NODE) tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules --ext=.js,.mjs,.md \
$(LINT_JS_TARGETS)
@if [ -x $(NODE) ]; then \
$(NODE) $(LINT_JS_CMD); \
else \
node $(LINT_JS_CMD); \
fi

jslint: lint-js
@echo "Please use lint-js instead of jslint"

lint-js-ci:
@echo "Running JS linter..."
$(NODE) tools/lint-js.js $(PARALLEL_ARGS) -f tap -o test-eslint.tap \
$(LINT_JS_TARGETS)
@if [ -x $(NODE) ]; then \
$(NODE) tools/lint-js.js $(PARALLEL_ARGS) -f tap -o test-eslint.tap \
$(LINT_JS_TARGETS); \
else \
node tools/lint-js.js $(PARALLEL_ARGS) -f tap -o test-eslint.tap \
$(LINT_JS_TARGETS); \
fi

jslint-ci: lint-js-ci
@echo "Please use lint-js-ci instead of jslint-ci"

LINT_CPP_ADDON_DOC_FILES = $(wildcard test/addons/??_*/*.cc test/addons/??_*/*.h)
LINT_CPP_EXCLUDE ?=
LINT_CPP_EXCLUDE += src/node_root_certs.h
LINT_CPP_EXCLUDE += $(wildcard test/addons/??_*/*.cc test/addons/??_*/*.h)
LINT_CPP_EXCLUDE += $(LINT_CPP_ADDON_DOC_FILES)
LINT_CPP_EXCLUDE += $(wildcard test/addons-napi/??_*/*.cc test/addons-napi/??_*/*.h)
# These files were copied more or less verbatim from V8.
LINT_CPP_EXCLUDE += src/tracing/trace_event.h src/tracing/trace_event_common.h
Expand All @@ -1024,11 +1042,19 @@ LINT_CPP_FILES = $(filter-out $(LINT_CPP_EXCLUDE), $(wildcard \
tools/icu/*.h \
))

# Code blocks don't have newline at the end,
# and the actual filename is generated so it won't match header guards
ADDON_DOC_LINT_FLAGS=-whitespace/ending_newline,-build/header_guard

lint-cpp:
@echo "Running C++ linter..."
@$(PYTHON) tools/cpplint.py $(LINT_CPP_FILES)
@$(PYTHON) tools/check-imports.py

lint-addon-docs: test/addons/.docbuildstamp
@echo "Running C++ linter on addon docs..."
@$(PYTHON) tools/cpplint.py --filter=$(ADDON_DOC_LINT_FLAGS) $(LINT_CPP_ADDON_DOC_FILES)

cpplint: lint-cpp
@echo "Please use lint-cpp instead of cpplint"

Expand All @@ -1038,9 +1064,10 @@ lint:
$(MAKE) lint-js || EXIT_STATUS=$$? ; \
$(MAKE) lint-cpp || EXIT_STATUS=$$? ; \
$(MAKE) lint-md || EXIT_STATUS=$$? ; \
$(MAKE) lint-addon-docs || EXIT_STATUS=$$? ; \
exit $$EXIT_STATUS
CONFLICT_RE=^>>>>>>> [0-9A-Fa-f]+|^<<<<<<< [A-Za-z]+
lint-ci: lint-js-ci lint-cpp lint-md
lint-ci: lint-js-ci lint-cpp lint-md lint-addon-docs
@if ! ( grep -IEqrs "$(CONFLICT_RE)" benchmark deps doc lib src test tools ) \
&& ! ( find . -maxdepth 1 -type f | xargs grep -IEqs "$(CONFLICT_RE)" ); then \
exit 0 ; \
Expand Down Expand Up @@ -1120,6 +1147,7 @@ endif
test-ci \
test-ci-js \
test-ci-native \
test-doc \
test-gc \
test-gc-clean \
test-hash-seed \
Expand Down
2 changes: 1 addition & 1 deletion doc/api/addons.md
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,7 @@ static void at_exit_cb1(void* arg) {
Isolate* isolate = static_cast<Isolate*>(arg);
HandleScope scope(isolate);
Local<Object> obj = Object::New(isolate);
assert(!obj.IsEmpty()); // assert VM is still alive
assert(!obj.IsEmpty()); // assert VM is still alive
assert(obj->IsObject());
at_exit_cb1_called++;
}
Expand Down

0 comments on commit d576e17

Please sign in to comment.