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

Clean up runner #239

Merged
merged 1 commit into from
Nov 19, 2015
Merged

Clean up runner #239

merged 1 commit into from
Nov 19, 2015

Conversation

vadimdemedes
Copy link
Contributor

Small cleanup of lib/runner.js

})
.catch(noop)
.then(function () {
stats.testCount = tests.only.length ? tests.only.length : stats.testCount;
stats.testCount = hasOnlyTests ? tests.only.length : stats.testCount;
Copy link
Contributor

Choose a reason for hiding this comment

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

What about...

stats.testCount = tests.only.length || stats.testCount;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good one!

Copy link
Contributor

Choose a reason for hiding this comment

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

It's the small things ✨

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fully agree!

@sindresorhus
Copy link
Member

@jamestalmage Node.js 0.10 seems to be failing again on Windows :(

https://ci.appveyor.com/project/sindresorhus/ava/build/job/vrwhv5konnjgylcc

@jamestalmage
Copy link
Contributor

Yes, failure is different now. "Missing plan".

In TAP, the plan line can be emitted at the beginning or end. If it is the end, we already know forked processes often drop a line or two of IO from the end on Windows (especially Node 0.10). It could be something else entirely, but that is my initial suspicion.

@sindresorhus
Copy link
Member

@jamestalmage Yeah, my suspicion too. Might be relevant: nodejs/node#3670

@jamestalmage
Copy link
Contributor

Yes. Looks very relevant. Perhaps we need to avoid explicitly ending the process unless we have to.

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.

4 participants