Stop hiding source of syntax errors

[sotojuan]

This helps out with #308. It does the following things:

• Make the changes noted here
• Move babel hook test to CLI (needed after making change above)
• Add an API test for nonexistent files

However, there's one issue. The fork.js test that calls a nonexistent file ("rejects on error and streams output") doesn't seem to get a "file does not exist error" (it just gets "error code 1 returned") and worse, a stack of it shows up in the test results. The testing continues fine though, but I am not sure how to contain it or test it.

Originally the test had an .on('uncaughtException') clause but after making the babel.js changes, it is not called at all so I removed it.

Any suggestions and help is appreciated!

[jamestalmage]

Are you talking about this test here? If this test passes, that seems fine to me. We are getting a failure. Users of the CLI or API will get "can't find any files to test", like they have. Anybody using lib/fork from outside AVA is already asking for trouble.

We need to investigate why it fails on Node 0.10.

[sotojuan]

We need to investigate why it fails on Node 0.10

It seems to fail in the regex test for the error message, so maybe the error message changed from 0.10 to others. I can give it a try with nvm.

If this test passes, that seems fine to me.

Yep it works and passes but I was just curious about the fact that a stack is printed to the screen during that error. Maybe it's not a big deal.

[jamestalmage]

That is because we have to let Node print it to stderr natively. We could clean up the output of our own tests by allowing fork to take stdio options like child_process.fork does (and pipe them to something else besides stderr for that particular test). That's a future PR, IMO.

[jamestalmage]

worse, a stack of it shows up in the test results.

Yep, that is anticipated. Basically due to some V8 issues, it is impossible to extract the filename and/or line number from caught SyntaxErrors in JavaScript. Node includes a hack that will print that information to stderr, but it relies on native C++ code. This is why we are changing the position of where we start listening for uncaughtException, it allows the error to be handled natively by Node, so we see the correct filename and line-number. Ideally we wouldn't have to do it this way, but we are picking the lesser of two evils.

[sotojuan]

Sounds good!

Seems like my suspicion was right. Node 0.10 gives an error code of 8 on no file fount, while other Node versions give an error code of 0, which is why my test was failing. It's a quick regex change luckily.

[jamestalmage]

use taps t.match(string, regexp) in all new tests. It gives us better error messages. See #256

[jamestalmage]

t.match

[jamestalmage]

looks like you missed 1 t.match

[jamestalmage]

Node 0.10 gives an error code of 8 on no file fount, while other Node versions give an error code of 0

0? Really? I would have expected 1

[jamestalmage]

It looks like it was 1 - that's good. As long as it is non-zero.

[sotojuan]

Yeah sorry I mistyped. Well either way I got that fixed. The AppVeyor builds all passed—not sure why the check isn't there yet.

[jamestalmage]

AppVeyor takes about 20 minutes per commit to complete, you pushed a few commits in rapid succession, so it was way behind. I cancelled all of them. Go ahead and fix that last t.match and push again, and we should be good.

[jamestalmage]

LGTM

[jamestalmage]

Thanks @sotojuan!

[sotojuan]

I was playing around with/learning about syncing forks and accidentally pushed, so I guess I'll need your LGTM back @jamestalmage. Sorry about that _.

I'm gonna go ahead and fix some inconsistencies in my tests (I think t.ok(err) should always be first).

[sindresorhus]

LGTM

[sindresorhus]

Great work @sotojuan :) 🍰

[vadimdemedes]

Leonardo says thank you!