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

Wait for fork to exit and then return results #136

Merged
merged 1 commit into from
Nov 5, 2015
Merged

Conversation

vadimdemedes
Copy link
Contributor

Improves #134.

// @jamestalmage

@jamestalmage
Copy link
Contributor

That should do it.
Once you merge this, I will merge master back into #135 and it should pass.

// define backup results, when there are no tests defined
var testResults = {
stats: {
passCount: 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Does ava already warn if a test file has no tests? (i.e. passCount === 0)?

I would think that should be a failure, otherwise putting:

process.exit(0); 

at the top of your test makes it pass. (contrived I know).

Should we be warning if the process exits with code 0, but never sends a result message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I will revert this one, because it seems unrelated to this PR. I think this question deserves its own space.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would agree, it needs a separate issue/discussion.

I think your code is fine, no need to revert. Lets just create a new issue and follow up on it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@vdemedes #137 created to track that discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will revert it, just for the sake of keeping things clear.

vadimdemedes pushed a commit that referenced this pull request Nov 5, 2015
Wait for fork to exit and then return results
@vadimdemedes vadimdemedes merged commit 6f55d8e into master Nov 5, 2015
@vadimdemedes vadimdemedes deleted the wait-for-forks branch November 5, 2015 22:57
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.

2 participants