Shouldn't this be

TAP_SNAPSHOT=1 node . run test

or is it OK to run just the following as in this PR (which would run the installed npm version, not npm from the repo's current branch):

TAP_SNAPSHOT=1 npm test

??

I had also suggested

node . run test -- --snapshot

but there was no discussion about using that.

Unfortunately this PR was submitted and merged outside of my working hours, so I had no chance to comment whilst it was still open.

Yes it's technically more correct to do node . in this repo, but for generating snapshots I don't know if it's mission critical. Our node . suggestion in the other examples is a "best practice" suggestion that is mostly about catching bugs when dealing with the run-script code itself. In the snapshot context it's not going to affect things that much, those won't alter based on how the tests are invoked.

If you suggested a PR that changed this it would still be accepted. We really do appreciate the improvements you have been suggesting to all of the docs and processes that are easy to overlook. Thank you.

As far as --snapshot vs TAP_SNAPSHOT=1 I know that the latter is what we use ourselves locally. All other things being equal it's less likely to be mis-copied (if someone were to try to manually re-type it) cause that -- is easy to miss. Not a big distinction (and probably not a consequential one) but that's usually why we do it that way.

@wraithgar

Thanks for your comments, your explanations and your encouragement!

For practical purposes it should be fine to leave it as it is for this release. I'll revisit it for the next release in terms of consistency in the CONTRIBUTING document regarding how to call npm test.