search

SBoudrias / gulp-istanbul

Istanbul unit test coverage plugin for gulp.
MIT License
186 stars 87 forks source link

does not work with async/await (node 7.6+) #116

Closed kevinludwig closed 7 years ago

kevinludwig commented 7 years ago

It fails to parse. I'm pretty sure gulp-istanbul needs to be upgraded to use istanbul-api version 1.1.1 (from what I can tell istanbul has been broken out into several sub-packages and the APIs have changed, so its not sufficient to bump the istanbul version as referenced here: https://github.com/gotwarlost/istanbul/issues/733

SBoudrias commented 7 years ago

Stable istanbul doesn't support ES6. You need to provide an instrumenter working with es6, see: https://github.com/SBoudrias/gulp-istanbul#instrumenter

Right now your best stable option is isparta.

kevinludwig commented 7 years ago

Hi, I will be glad if I'm wrong about this but as far as I can tell, using isparta makes no difference. Maybe becuase it only claims to be an instrumenter for es6 code that is run through babel, or maybe because async/await isn't supported by isparta. Either way, if I do this:

gulp.task('cover', () => {
     return gulp.src(['./src/**/*.js''])
         .pipe(istanbul({ instrumenter: isparta.Instrumenter })
         .pipe(istanbul.hookRequire());
});

I get the same error. As a side note, I realize that istanbul stable does not support es6/es7, but if you acutally look at the alpha tag for istanbul (what you'd get if you installed istanbul@next)you'd see that all of the functionality of istanbul has been stripped out of it. The istanbul project itself is only the command line tool, and the actual APIs that gulp-istanbul would need to interface with are in other repos, in particular istanbul-api, which is currently sitting at version 1.1.1.

SBoudrias commented 7 years ago

I don't really have the time to start supporting unstable/alpha version of istanbul. When it'll be released officially, we'll support it. For now, you can always compile your code with babel before instrumenting it (I believe that's what jest is doing). You'll get similar results.

kevinludwig commented 7 years ago

I guess. I mean like I said, the thing that gulp-istanbul would depend on is istanbul-api, istanbul-lib-coverage, istanbul-lib-instrument, istanbul-lib-hook, etc. none of which are alpha. The only thing that is alpha as far as I can see is istanbul which like i said is nothing but a layer over top of those other things to produce a command line tool.

You're right that I can transpile my code using babel but it seems pretty ridiculous to transpile features that natively exist in node just so that I can get a task runner plugin to generate code coverage.

SBoudrias commented 7 years ago

I'm happy to merge to a PR replacing istanbul with a stable version of istanbul-api

kevinludwig commented 7 years ago

I've got a separate version already working (that I did yesterday) but I'd be happy to try and make it work for gulp-istanbul. I got my code working by using gulp-istanbul as a starting point after all...

But there are a few things to consider:

  1. I don't think there's any ability in the new istanbul-api to register a custom coverage reporter. So I don't think that particular feature of gulp-istanbul can be supported.
  2. The instrumenter is now created via a factory function not via new so It seems like the gulp-istanbul API would change a bit if you want to continue supporting custom instrumenters.
  3. And I'm unable to get my changes working with gulp-mocha which means there's something I'm misunderstanding about your implementation. (I had to implement my own version of gulp-mocha that runs mocha in-process).**

** My prior usage of gulp-istanbul together with gulp-mocha was to write the coverage files to a temporary directory and then run gulp-mocha against those files, and then run istanbul.writeReports etc after. And when I ported my code to istanbul-api that was resulting in no coverage being generated (the coverage variable was undefined). The reason I was doing that was because hookRequire doesn't work because of the way gulp-mocha is written--it spawns a new process to run mocha, so hookRequire has no effect on the tests or source code. After looking at your code, its unclear to me how my approach actually worked (but it did), because with mocha in a subprocess the global coverage variable in the gulp process will remain undefined (which is what happens in my code when I implement against istanbul-api).

So anyway if you're OK with (1) and (2) you have any ideas on (3) then I'll proceed, I just don't want to get all the way to a PR and find out you're not OK with the changes.

SBoudrias commented 7 years ago

@kevinludwig gulp-istanbul doesn't work with the new gulp-mocha as it spawns sub processes.