Skip to content

test: refactor 'which' tests to AVA#580

Merged
nfischer merged 1 commit intomasterfrom
test-ava-which
Nov 27, 2016
Merged

test: refactor 'which' tests to AVA#580
nfischer merged 1 commit intomasterfrom
test-ava-which

Conversation

@nfischer
Copy link
Copy Markdown
Member

@freitagbr ready for review. This should be good to go.

@nfischer nfischer changed the title test: refactor which tests to AVA test: refactor 'which' tests to AVA Nov 24, 2016

// TODO(nate): make sure this does not have a false negative if 'git' is missing
test('basic usage', t => {
const git = shell.which('git');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not use node? Surely if this test is running on a system, that system has node installed...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Node can be installed as nodejs on Linux. It's always installed as node.exe on Windows, though. We can count on git to be always be installed on a dev's box. That's the rationale. We could probably change the next one to git as well if we wanted to be consistent

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's a good point, let's keep it git then.

@freitagbr
Copy link
Copy Markdown
Contributor

LGTM

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants