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

emit test plans #18

Merged
merged 1 commit into from
Nov 21, 2015
Merged

emit test plans #18

merged 1 commit into from
Nov 21, 2015

Conversation

jamestalmage
Copy link
Contributor

Related: scottcorgan/tap-spec#40

@scottcorgan

I've searched high and low, and can not find a way to determine the exit code of the process piping data to stdinn. I am not sure it exists (I know it does in bash shell, but it does not seem to at the OS level).

The way other tap reporters (i.e. faucet) seem to handle this, is that they look for the plan line (which, as I understand it is required), and then compare what they get.

This implements recognition of the plan line:

1...3

And emits it as an event.

This will allow you to correctly exit with your other reporters. Thanks!

scottcorgan added a commit that referenced this pull request Nov 21, 2015
@scottcorgan scottcorgan merged commit 9cbd06b into scottcorgan:master Nov 21, 2015
@scottcorgan
Copy link
Owner

SOOO thankful you did this! Thank you! Haven't had time and this keeps popping up. Published as 1.4.0. Updating tap-spec soon.

@jamestalmage
Copy link
Contributor Author

Awesome, thanks!

//cc: @sindresorhus @vdemedes

@sindresorhus
Copy link

@jamestalmage What does this mean for us now? We're already using the builtin node-tap reporter? Is tap-spec any better?

@jamestalmage
Copy link
Contributor Author

It means we could go back to tape and maybe get back Node 0.10 Windows tests.

I see no need for this if we stick with tap.

@sindresorhus
Copy link

@jamestalmage Oh ok. Let's just stick with what we have right now. It seems to be working fine and I don't want to open the can of worms that is Node.js 0.10 on Windows yet. We can reconsider in the future.

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

Successfully merging this pull request may close these issues.

3 participants