gotwarlost / istanbul

Yet another JS code coverage tool that computes statement, line, function and branch coverage with module loader hooks to transparently add coverage when running tests. Supports all JS coverage use cases including unit tests, server side functional tests and browser tests. Built for scale.
Other
8.7k stars 787 forks source link

Nodejs 7.2 async/await #733

Closed gbahamondezc closed 7 years ago

gbahamondezc commented 7 years ago

I'm using NodeJS 7 + gulp-istanbul for my test coverage and i'm getting Unexpected identifier with a async method of a class.

    class SomeClass {
        async someMethod() {
            return await  somePromise();
        }
    }

Is this issue related direct with Istanbul?
If so, there's some way to support this case?

BrandonZacharie commented 7 years ago

I get a bunch or errors that look like

Transformation error; return original code
{ Error: Line 22: Unexpected token =>
    at constructError (node_modules/esprima/esprima.js:2407:21)
    at createError (node_modules/esprima/esprima.js:2426:17)
    at unexpectedTokenError (node_modules/esprima/esprima.js:2500:13)
    at throwUnexpectedToken (node_modules/esprima/esprima.js:2505:15)
    at consumeSemicolon (node_modules/esprima/esprima.js:2620:13)
    ...

Maybe esprima needs to be upgraded or patched.

sabakugaara commented 7 years ago

@gbahamondezc, istanbul shuold be ^1.0.0 and try this:

node --harmony ./node_modules/.bin/istanbul cover ./node_modules/.bin/_mocha -- tests/*
kevinludwig commented 7 years ago

@sabakugaara the most recent version of istanbul is 0.4.5; do you mean gulp-istanbul >= 1.0.0? Also as far as I can tell, the problem is that the parser being used is esprima, and it does not yet support async/await it only claims to support ES2016. So I think the answer here is that esprima needs to support ES2017 and then istanbul needs to be upgraded to a compliant version of esprima, and then gulp-istanbul needs to be bumped to take that new version of istanbul. If you'd like to verify for yourself that this is the case, you can look at the esprima online parser, just type in a simple do nothing async function and it will complain: http://esprima.org/demo/parse.html.

sabakugaara commented 7 years ago

@kevinludwig try this: istanbul 1.1-alpha

kevinludwig commented 7 years ago

npm ERR! No compatible version found: gulp-istanbul@1.1.0-alpha.1 npm ERR! Valid install targets: npm ERR! 1.1.1, 1.1.0, 1.0.0, 0.10.4, 0.10.3, 0.10.2, 0.10.1, 0.10.0, 0.9.0, 0.8.1, 0.8.0, 0.7.0, 0.6.0, 0.5.0, 0.4.0, 0.3.1, 0.3.0, 0.2.2, 0.2.1, 0.2.0, 0.1.1, 0.1.0

sabakugaara commented 7 years ago

maybe gulp-istanbul should update dependencies https://github.com/SBoudrias/gulp-istanbul/blob/master/package.json#L32

kevinludwig commented 7 years ago

don't think that helps. Latest version of istanbul is 0.4.5 and the version specified by gulp-istanbul is ^0.4.0 so it will end up pulling down 0.4.5. The problem is that istanbul has a dependency on esprima and esprima doesn't yet seem to support async/await.

kevinludwig commented 7 years ago

for what its worth, I tried upgrading esprima to 4.0.0-dev (the istanbul dependency) which is currently whats on master in github (for esprima), and the problem persists.

kevinludwig commented 7 years ago

Welp, node 7.6 landed and async/await is no longer behind a harmony flag, but I still can't get coverage for async/await.

patrikx3 commented 7 years ago

yep, async/await is not working :( i think it is the last important language update for a while.

thetutlage commented 7 years ago

Install 1.1.0-alpha.1 from npm and it works. Just did a minute ago

npm i --save-dev istanbul@v1.1.0-alpha.1
patrikx3 commented 7 years ago

works! thanks!

magicdawn commented 7 years ago

use npm i istanbul@next --save-dev, not @latest

gbahamondezc commented 7 years ago

Works, perfect with alpha version.

kevinludwig commented 7 years ago

what would you recommend as the way to get gulp-istanbul working? I have this vague feeling that if I make a PR to that repo saying "just use the alpha version of istanbul" I will get rejected.

kevinludwig commented 7 years ago

also if I just do cd node_modules/gulp-istanbul && npm install istanbul@next, then run a npm test for my project, it blows up inside of gulp-istanbul code, this is the complaint:

    var instrumenter = new opts.instrumenter(fileOpts);
                       ^
TypeError: opts.instrumenter is not a constructor

So did the interface change for istanbul in a way that would break gulp-istanbul?

kevinludwig commented 7 years ago

It's trying to do new istanbul.Instrumenter(fileOpts) just to make that a bit more clear.

kevinludwig commented 7 years ago

@gotwarlost So I've realized that the API has pretty drastically changed between 0.4.5 and 1.1.0-alpha.1, which is why gulp-istanbul is not a trivial version bump upgrade. I'd be willing to give it a shot changing gulp-istanbul to work with istanbul@next but the docs don't seem to be updated at last on the README for the 1.1.0-alpha.1 tag. What are the chances that the README will be updated any time soon to reflect the new API?

ovaar commented 7 years ago

Is there any improvements on this issue?

electerious commented 7 years ago

From the readme:

Deprecation Notice: this version of istanbul is deprecated, we will not be landing pull requests or releasing new versions. But don't worry, the Istanbul 2.0 API is now available and is being actively developed in the new istanbuljs organization.

async/await is supported by default in the new version: https://github.com/istanbuljs/istanbuljs

Took me a while to realize that there's a difference between istanbul and istanbuljs on npm.

bforbis commented 6 years ago

A few questions:

Is istanbul v1.1.0-alpha.1 going to be released as a standard release? I understand that this package is considered deprecated in favor of what is available at https://istanbul.js.org using nyc, but it seems like upgrading to nyc is not necessarily trivial. To get async/await support using nyc, you have to involve babel. Now that async/await is part of the LTS release of node, I'd rather not involve babel into my project if I can avoid it.

kevinludwig commented 6 years ago

Nyc does not require banel with latest node. Its vastly simpler than the gulp plugin.

bforbis commented 6 years ago

Oh, looks like I misunderstood the istanbul website which wasn't clear on whether babel-plugin-istanbul was required. @kevinludwig is correct, this is not required.

bennycode commented 6 years ago

I also had problems using async and await calls in my jasmine tests. Updated istanbul from "0.4.5" to "1.1.0-alpha.1" and it works now like a charm! ✨

Here is my npm "coverage" script:

"coverage": "rimraf coverage && cross-env NODE_ENV=test istanbul cover --report html ./node_modules/jasmine/bin/jasmine.js"