implement --no-only flag functionality

[jocrah]

Summary

This pr implements the functionality stated in #569.

How it works

By passing a --no-only flag while running tape tests (eg. tape **/*test.js --no-only), any test.only unintentionally left in any of the tests causes an error to be thrown to make tests fail.

This behaviour can alternatively be achieved by setting the environmental variable NODE_TAPE_NO_ONLY_TEST to true.

[ljharb]

Since test files can be run with either tape test.js or node test.js, my hope would be that the env var would apply to both cases.

[ljharb]

I tried to improve this by passing an option explicitly into tape.run, but because of the way the lazy loader works, i'm not sure how well that'd work.

[jocrah]

I tried to improve this by passing an option explicitly into tape.run, but because of the way the lazy loader works, i'm not sure how well that'd work.

o okay yea understood, also been trying to debug the exit code being 8 for the node version 0.10, read around it and got to know some earlier versions of node used it for uncaught exceptions but not sure what can be done yet at our end

[ljharb]

ah right, forgot about that. i'll take a quick look locally.

[ljharb]

I see that 0.10 and 0.8 both have different kinds of failures, including some exit codes of 127. io.js v3.0-v3.2 seem to have similar failures. I'd prefer not to start making these fail, so let's look more into them.

I've rebased on top of a commit that fixes the 16.9/16.10 failures.

[jocrah]

I see that 0.10 and 0.8 both have different kinds of failures, including some exit codes of 127. io.js v3.0-v3.2 seem to have similar failures. I'd prefer not to start making these fail, so let's look more into them.

okay got it, thanks. Earlier today, tried 0.8 locally and did not get the tests introduced in this pr failing (I guess could depend on the specific patch version).

Please do you mean some other tests failed also with the 127 exit code or these same tests?

[ljharb]

At this point (and we can check once CI is completed) i think all the remaining failures are:

• 0.6 (and i think 0.9), which always fail
• node 0.8, 0.10, 3.0, 3.1, and 3.2, failing on test/no_only.js

The latter is my concern.

[jocrah]

so after trying locally, I see the iojs 3.0, 3.1 and 3.2 failures are related to some stack-buffer overflow error related to nodejs/node#2581, and fixed i think for latter versions?

[ljharb]

hmm, meaning what, that this one test file is triggering a bug, but no other test files have triggered it before?

[ljharb]

hmm, oddly enough locally v3.3 fails, but it passes in CI.

[ljharb]

hmm, when i run the test file by itself, it fails consistently for me locally, but when i run the full suite, it passes. That's concerning.

[ljharb]

ok, figured that last one out; when specifying env, the PATH has to be explicitly passed down.

[codecov-commenter]

Codecov Report

Merging #572 (2fb5a66) into master (8594f3b) will increase coverage by 0.32%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #572      +/-   ##
==========================================
+ Coverage   96.03%   96.36%   +0.32%     
==========================================
  Files           4        4              
  Lines         631      632       +1     
  Branches      147      148       +1     
==========================================
+ Hits          606      609       +3     
+ Misses         25       23       -2     

Impacted Files	Coverage Δ
index.js	98.11% <100.00%> (+0.01%)	⬆️
lib/test.js	95.42% <0.00%> (+0.28%)	⬆️
lib/results.js	100.00% <0.00%> (+0.65%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8594f3b...2fb5a66. Read the comment docs.

[jocrah]

hmm, meaning what, that this one test file is triggering a bug, but no other test files have triggered it before?

yea quite interesting. Looking at the already existing tests, I don't think we've had the use-case of testing the stderr output of any thrown error for those particular node versions (3.0,3.1 and so on)

Locally ( on the master branch), I added a tape test file that had multiple only tests to trigger the existing error we have where there is supposed to be just one only test.

I then tried to run a similar tap test (using the child process spawn) on the tape file using iojs 3.2, it still had the stack error instead of the expected error

[ljharb]

When I use iojs v3.2, and I run tape --no-only test-b.js, i get an exit code of 1 and the expected output - is the bug just around spawn?

[jocrah]

o okay interesting, yea most likely something around that.

[ljharb]

I've switched from spawn to exec; that seems like it'll fix it :-)

[jocrah]

okay seen, nice!

[jocrah]

then I think the main thing left will be the differences in error exit code now. Checking the iojs 3.2 build, I see failure is now related to an exit code of 134instead of the expected 1

[ljharb]

yeah, i locally changed the ok to a match, and i see it's the exact node bug you linked.

[ljharb]

I went ahead and TODOd the tests for node 0.8, 0.10, and 3.0-3.2.

[jocrah]

okay cool, this was fun 😄, thanks for the review and insights @ljharb