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

Exit with status code 137 on node 4 when using with AVA, --all and --require babel hook #181

Closed
thangngoc89 opened this issue Mar 4, 2016 · 7 comments

Comments

@thangngoc89
Copy link

I have a repo setup with AVA and nyc

nyc --all --require babel-core/register ava

AVA option

"require": [
   "babel-core/register"
]

On node 5, all tests passed.

But on node 4, npm@2 doesn't dedupe babel-core/register by default, we will have 2 different versions of babel-core/register : one from AVA and one from project's dependencies.

This will cause test process exit with status code 137

My current solution is disable coverage on node 4.

poke @jamestalmage

@jamestalmage
Copy link
Member

The other issue here relates to the fact that he wanted to use the --all option. Which fails on ES6 files if you try to use AVAs --require flag instead of nycs.

Also, it is preferable to use AVA's require pkgConf to provide a cleaner cli experience across both covered and non-covered test runs. Again, that doesn't work for --all

@novemberborn
Copy link
Contributor

It's starting to seem that --require here is just a cheat. You should let the child process be responsible for setting up the transpiler. nyc can hook into that better now than when --require was introduced. Basically nyc's --require is only useful if you have test code you always run using nyc and you're too lazy to manage the transpiler yourself.

But… then there's --all. I think we're doing that wrong too. We should still find all the files however rather than instrumenting them up front we should just keep track of them. Then once we have the coverage report we can figure out which files were never loaded and synthesize an empty report for those files, based on the raw source.

@MoOx
Copy link

MoOx commented Mar 4, 2016

A workaround for --all is to add empty test files that just require the (not yet) tested file?

@novemberborn
Copy link
Contributor

A workaround for --all is to add empty test files that just require the (not yet) tested file?

Yea, that way AVA can load the files and you won't need the conflicting --require for nyc.

Once AVA supports custom Babel configs you could use https://github.com/novemberborn/babel-plugin-import-glob to require all source files in one go, so you don't forget any.

Until we fix --all here that is 😉

@MoOx
Copy link

MoOx commented Mar 5, 2016

I can already glob with ava **/__tests__/*.js :)

MoOx added a commit to MoOx/phenomic that referenced this issue Mar 5, 2016
@jamestalmage
Copy link
Member

It's starting to seem that --require here is just a cheat.

Also, if you are using Node 4+, you can use the native --require flag:

$ node --require babel-register node_modules/.bin/nyc --all ava

If you need to support Node 0.12 or below, I have created spawn-require, which would be used as follows:

$ spawn-require babel-register nyc --all ava

But… then there's --all. I think we're doing that wrong too... find all the files ... just keep track of them. ... figure out which files were never loaded and synthesize an empty report

👍

@novemberborn
Copy link
Contributor

Closing this issue since it's by design. Tracking improvements in #183.

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

No branches or pull requests

4 participants