From 4e4340c81265038510b6f00a9aab32904fe9c8e9 Mon Sep 17 00:00:00 2001
From: Sam Ruby <rubys@intertwingly.net>
Date: Mon, 30 Jul 2018 12:08:27 -0400
Subject: [PATCH] build: make 'make test' run linter and build docs

Before this change, the comment for 'make test' read as follows:

    ## Runs default tests, linters, and builds docs

What it actually did was:

 * run build-addons twice
 * run build-addons-napi twice
 * run cctest once
 * run jstest once
 * not run linters
 * not build docs

Arguably, one could make the comments match the code, or the code match the
comments.  Given that there is a separate make target named `test-only` that
does exactly what `test` did, I went the other way.

Along the way, other fixes were made:

 * `build-addons` and `build-addons-napi` were listed as `.PHONY`, but had
   no commands defined.  I removed the `.PHONY` markers and moved them to
   `test/addons/.buildstamp` and `test/addons-napi/.buildstamp` where they
   would have the desired effect.
 * `tools/doc/node_modules` was listed as `.PHONY` and did not depend on
   `tools/doc/package.json`.  This caused it to unnecessary run every time
   it was referenced.

After these changes, `make test` will:

 * run build-addons once
 * run build-addons-napi once
 * run cctest once
 * run jstest once
 * run linters once
 * build docs once
---
 Makefile | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index c3da8e88bff698..b340fb17f9a84d 100644
--- a/Makefile
+++ b/Makefile
@@ -254,6 +254,10 @@ v8:
 
 .PHONY: jstest
 jstest: build-addons build-addons-napi ## Runs addon tests and JS tests
+	$(MAKE) -s test-js
+
+.PHONY: test-js
+test-js:
 	$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) \
 		--skip-tests=$(CI_SKIP_TESTS) \
 		$(CI_JS_SUITES) \
@@ -267,7 +271,8 @@ test: all ## Runs default tests, linters, and builds docs.
 	$(MAKE) -s build-addons
 	$(MAKE) -s build-addons-napi
 	$(MAKE) -s cctest
-	$(MAKE) -s jstest
+	$(MAKE) -s test-js
+	$(MAKE) -s test-doc
 
 .PHONY: test-only
 test-only: all  ## For a quick test, does not run linter or build docs.
@@ -276,7 +281,7 @@ test-only: all  ## For a quick test, does not run linter or build docs.
 	$(MAKE) build-addons
 	$(MAKE) build-addons-napi
 	$(MAKE) cctest
-	$(MAKE) jstest
+	$(MAKE) test-js
 
 # Used by `make coverage-test`
 test-cov: all
@@ -285,7 +290,7 @@ test-cov: all
 	$(MAKE) build-addons
 	$(MAKE) build-addons-napi
 	# $(MAKE) cctest
-	CI_SKIP_TESTS=core_line_numbers.js $(MAKE) jstest
+	CI_SKIP_TESTS=core_line_numbers.js $(MAKE) test-js
 
 test-parallel: all
 	$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) parallel
@@ -342,6 +347,7 @@ ADDONS_BINDING_SOURCES := \
 	$(filter-out test/addons/??_*/*.cc, $(wildcard test/addons/*/*.cc)) \
 	$(filter-out test/addons/??_*/*.h, $(wildcard test/addons/*/*.h))
 
+.PHONY: test/addons/.buildstamp
 # Implicitly depends on $(NODE_EXE), see the build-addons rule for rationale.
 # Depends on node-gyp package.json so that build-addons is (re)executed when
 # node-gyp is updated as part of an npm update.
@@ -356,7 +362,6 @@ test/addons/.buildstamp: config.gypi \
 		"$$PWD/deps/npm/node_modules/node-gyp/bin/node-gyp.js" "$$PWD/test/addons"
 	touch $@
 
-.PHONY: build-addons
 # .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
@@ -374,6 +379,7 @@ ADDONS_NAPI_BINDING_SOURCES := \
 	$(filter-out test/addons-napi/??_*/*.cc, $(wildcard test/addons-napi/*/*.cc)) \
 	$(filter-out test/addons-napi/??_*/*.h, $(wildcard test/addons-napi/*/*.h))
 
+.PHONY: test/addons-napi/.buildstamp
 # Implicitly depends on $(NODE_EXE), see the build-addons-napi rule for rationale.
 test/addons-napi/.buildstamp: config.gypi \
 	deps/npm/node_modules/node-gyp/package.json tools/build-addons.js \
@@ -387,7 +393,6 @@ test/addons-napi/.buildstamp: config.gypi \
 		"$$PWD/test/addons-napi"
 	touch $@
 
-.PHONY: build-addons-napi
 # .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
@@ -1077,8 +1082,7 @@ lint-md-build: tools/remark-cli/node_modules \
 	tools/doc/node_modules \
 	tools/remark-preset-lint-node/node_modules
 
-.PHONY: tools/doc/node_modules
-tools/doc/node_modules:
+tools/doc/node_modules: tools/doc/package.json
 	@cd tools/doc && $(call available-node,$(run-npm-install))
 
 .PHONY: lint-md