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

Fix test globbing on Windows #279

Merged
merged 1 commit into from
Aug 21, 2017
Merged

Conversation

Throne3d
Copy link
Collaborator

@Throne3d Throne3d commented Jul 29, 2017

Windows doesn't support the inline evaluation $(find..) and also doesn't support the find command. nyc can handle globbing itself, as can it appears _mocha (i.e. this command works both without and with the -- given just before the globbing), so it doesn't seem the find command is necessary.

This allows npm test to function on Windows using PowerShell (and presumably also command prompt). For some reason the coverage report is missing data, but it misses data without this change also.

Assuming this doesn't break Travis it should be good to go.

(Windows actually does execute the tests on master, because nyc or mocha receives no valid parameters and so I think defaults to the test directory, but it exits with an error code. This fixes that, but does not fix the lack of coverage data on Windows.)

Windows doesn't support the inline evaluation `$(find..)` and also
doesn't support the find command. `nyc` can handle this itself, as can
it appears `_mocha` (i.e. this command works both without and with the
`--` given just before the globbing), so it doesn't seem the find
command is necessary.

This allows `npm test` to function on Windows using PowerShell. For some
reason the coverage report is missing data, but it misses data without
this change also.
@coveralls
Copy link

coveralls commented Jul 29, 2017

Coverage Status

Coverage remained the same at 99.254% when pulling 92d5b43 on Throne3d:fix/test-windows into ddd9c54 on reactiflux:master.

@@ -27,7 +27,7 @@
"build": "babel lib --out-dir dist",
"prepublish": "npm run build",
"lint": "eslint . --ignore-path .gitignore",
"coverage": "nyc --require babel-core/register _mocha -- $(find test -name '*.test.js')",
"coverage": "nyc --require babel-core/register _mocha -- test/*.test.js",
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this will recurse down the test directory. Doesn't really matter for now, but if we ever add sub-directories with tests it should probably be something like test/**/*.test.js.

@ekmartin ekmartin merged commit 1ff6141 into reactiflux:master Aug 21, 2017
@Throne3d Throne3d deleted the fix/test-windows branch August 21, 2017 13:17
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.

None yet

3 participants