Closed jocrah closed 2 years ago
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
ah right, forgot about that. i'll take a quick look locally.
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.
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?
At this point (and we can check once CI is completed) i think all the remaining failures are:
node 0.8, 0.10, 3.0, 3.1, and 3.2, failing on test/no_only.js
The latter is my concern.
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 https://github.com/nodejs/node/issues/2581, and fixed i think for latter versions?
hmm, meaning what, that this one test file is triggering a bug, but no other test files have triggered it before?
hmm, oddly enough locally v3.3 fails, but it passes in CI.
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.
ok, figured that last one out; when specifying env
, the PATH has to be explicitly passed down.
Merging #572 (2fb5a66) into master (8594f3b) will increase coverage by
0.32%
. The diff coverage is100.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%) |
:arrow_up: |
lib/test.js | 95.42% <0.00%> (+0.28%) |
:arrow_up: |
lib/results.js | 100.00% <0.00%> (+0.65%) |
:arrow_up: |
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.
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
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?
o okay interesting, yea most likely something around that.
I've switched from spawn to exec; that seems like it'll fix it :-)
okay seen, nice!
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 134
instead of the expected 1
yeah, i locally changed the ok
to a match
, and i see it's the exact node bug you linked.
I went ahead and TODOd the tests for node 0.8, 0.10, and 3.0-3.2.
okay cool, this was fun :smile:, thanks for the review and insights @ljharb
Summary
This pr implements the functionality stated in https://github.com/substack/tape/issues/569.
How it works
By passing a
--no-only
flag while running tape tests (eg.tape **/*test.js --no-only
), anytest.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
totrue
.