Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from May 25, 2019

Conversation

r0mflip
Copy link
Contributor

@r0mflip r0mflip commented May 23, 2019

Fixes #469

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

@fregante
Copy link
Contributor

Thank you! Hope this is merged soon

Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ljharb ljharb changed the title Fixes #469 [fix] don't consider 'ok' of todo tests in exit code May 25, 2019
@ljharb ljharb merged commit 15b2dfc into tape-testing:master May 25, 2019
ljharb added a commit that referenced this pull request May 25, 2019
 - [fix] don't consider 'ok' of todo tests in exit code (#470)
 - [refactor] Removed never-read inErrorState from index.js (#462)
 - [deps] update `glob`, `resolve`
 - [docs] Minor punctuation/highlighting improvement (#468)
@fregante
Copy link
Contributor

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
Copy link
Collaborator

ljharb commented May 26, 2019

A PR that does would be appreciated.

@fregante
Copy link
Contributor

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
Copy link
Contributor Author

r0mflip commented May 26, 2019

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
Copy link
Contributor

fregante commented May 26, 2019

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
Copy link
Contributor Author

r0mflip commented May 26, 2019

Tried to add tests in #471

@r0mflip
Copy link
Contributor Author

r0mflip commented May 26, 2019

@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
Copy link
Contributor

fregante commented May 26, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

{todo: true} passes the test but cli exits with 1
3 participants