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

how does this sound, for a start to the known issues category #5

Merged
merged 2 commits into from
Dec 24, 2015

Conversation

bcoe
Copy link
Owner

@bcoe bcoe commented Dec 23, 2015

Added links based on @sindresorhus' suggestions, regarding known issues on Windows.

How does this sound as a start @jamestalmage, would love to expand on it based on your feedback.

@jamestalmage
Copy link

Anyone testing stderr/stdout of a forked process on Windows will have nightmares on AppVeyor, especially Node 0.10.

We ended up having to do some really hackie stuff to make AppVeyor work for us:

Our test suite still fails occasionally on Windows. It's much better now that we do minimal testing of our console output. Instead of tests that execute the CLI, we test the API, and keep the CLI to as thin a wrapper as possible.


Also, a very recent strategy that I think is great: Instead of creating a logger class, which logs directly to console, we employ a message generator that returns strings, so this:

test.on('error', function (e) {
  logger.logTestError(e);
});

becomes this:

test.on('error', function (e) {
  console.error(messageGenerator.createTestErrorMessage(e));
});

A little more verbose, but eminently more testable.


@sindresorhus recently published hook-std. If you must test stdout/stderr, use that in the same process instead of forking.

bcoe added a commit that referenced this pull request Dec 24, 2015
how does this sound, for a start to the known issues category
@bcoe bcoe merged commit e740c92 into master Dec 24, 2015
@bcoe bcoe deleted the caveats branch December 24, 2015 20:29
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