build: make 'make test' run linter and build docs

[rubys]

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

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

[vsemozhetbyt]

CI: https://ci.nodejs.org/job/node-test-pull-request/16084/

[BridgeAR]

Would it not be better to remove the addon parts from this task and keep jstest? Just the cctest would have to move below. The same for the other entries.

[rubys]

This make rule is designed to run a number of steps serially. Depending on jstest would enable make to run these commands in parallel; and in particular, before all is completed. Note that the definition of build-addons does not require $(NODE_EXE) to be present.

build-addons: | $(NODE_EXE) test/addons/.buildstamp

[refack]

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.

I understand that empirically this change achieves the desired result, but I'm not sure I understand why. AFAICT .PHONY targets are for targets that are "commands" and don't actuly output files. But in this case test/addons/.buildstamp and test/addons-napi/.buildstamp are files, while build-addons and build-addons-napi aren't.

/CC @nodejs/build-files

[rubys]

@refack at this point, the @touch $@ statements in these two rules are superfluous. Once a rule is marked as .PHONY, it matters not whether there is a corresponding file or what its timestamp might be.

[richardlau]

build-addons should remain .PHONY as it does not refer to a file.

[richardlau]

build-addons-napi should remain .PHONY as it does not refer to a file.

[richardlau]

If we're going to make this target .PHONY, I would remove the touch so it's no longer a file and probably rename the target so it doesn't look like a file either.

[richardlau]

If we're going to make this target .PHONY, I would remove the touch so it's no longer a file and probably rename the target so it doesn't look like a file either.

[Trott]

Not sure when it changed, but make test used to run linters like the comment says. It would be great to have that behavior restored before Code + Learn in October. It will be an annoyance otherwise.

[maclover7]

ping @rubys

[BridgeAR]

This needs a rebase and the comments have to be looked at.

[Trott]

@rubys Still working on this?

[BridgeAR]

Closing due to long inactivity. @rubys please feel free to reopen or open a new PR in case you would like to continue working on this.