tape-testing / tape

tap-producing test harness for node and browsers
MIT License
5.77k stars 307 forks source link

[fix] don't consider 'ok' of todo tests in exit code #470

Closed r0mflip closed 5 years ago

r0mflip commented 5 years ago

Fixes #469

result.ok not being considered towards exitcode if result.todo is true

fregante commented 5 years ago

Thank you! Hope this is merged soon

fregante commented 5 years ago

This sadly doesn't fix the issue. The test added does not check the exit code like these tests do https://github.com/substack/tape/blob/master/test/exit.js

ljharb commented 5 years ago

A PR that does would be appreciated.

fregante commented 5 years ago

I looked into the code and I couldn’t find any path that changed the exit code. It seems that the error automatically changes it.

r0mflip commented 5 years ago

I will try to add other tests.

I looked into the code and I couldn’t find any path that changed the exit code. It seems that the error automatically changes it.

I sorry, I don't understand.

fregante commented 5 years ago

There are a few mentions of test._exitCode = 1 but even if I delete them all, the exit code is still 1, so there's something else that sets it and I can't find it.

r0mflip commented 5 years ago

Tried to add tests in #471

r0mflip commented 5 years ago

@bfred-it There are a few mentions of test._exitCode = 1 but even if I delete them all, the exit code is still 1, so there's something else that sets it and I can't find it.

After the fix #470 I don't get any of those problems. Can you please confirm again, I've tested it and I get the right exit code.

fregante commented 5 years ago

I tried the latest on npm and I get those errors, even after deleting node_modules, but I'm running it via tape-run.

I think that tape-run might be the issue, maybe it detects not ok in the output and exits with 1. It doesn't actually follow the exit code.

I'll open an issue in tape-run