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

Add support for ES6 and Code Coverage #65

Closed
phillipskevin opened this issue Nov 13, 2015 · 12 comments
Closed

Add support for ES6 and Code Coverage #65

phillipskevin opened this issue Nov 13, 2015 · 12 comments
Assignees

Comments

@phillipskevin
Copy link
Contributor

Some ES6 syntax breaks Istanbul instrumentation.

@phillipskevin
Copy link
Contributor Author

Switching to isparta is simple and allows coverage to work on the project I was having trouble with. Should this be done by default whenever running with coverage, or do we want to allow the user to choose which coverage instrumentation to use? If a project is already running testee with their transpiled files, they might not want to transpile again.

/cc @daffl

@daffl
Copy link
Contributor

daffl commented Nov 16, 2015

We should probably make it configurable. It looks like this one only works with Babel?

@phillipskevin
Copy link
Contributor Author

Right, it just runs babel and then Istanbul. I haven't done too much research on other alternatives to isparta.

@justinbmeyer
Copy link
Member

Can someone walk me through this?

There's steal, testee, instanbul, and babel.

What does what, where do things go wrong?

My assumption on a "perfect" solution would be steal uses babel and an instanbul plugin to babel to transpile all code to be instrumented.

Testee would just run the tests as normal, but be looking out for some signal that there's code coverage information.

@daffl
Copy link
Contributor

daffl commented Aug 2, 2016

What it currently does is instrument the JS files on the server before sending them. This works for any ES5 JavaScript file (also with anything like Browserify, Webpack and server side Babel that builds things first).

The problem is that the instrumentation even with the 1.0-beta Istanbul that supposedly works with ES6 as far as I could get it to work creates output that does not work with Steal when it loads the individual files on the client.

@ccummings
Copy link
Contributor

A new babel plugin exists that generates a coverage variable for es6 code. Not sure if it suffers from the same problem as istanbul today.

@matthewp
Copy link
Contributor

matthewp commented Aug 2, 2016

I'm confused as to how Instanbul instruments on the server. Does it overwrite files? Like if I have a file foo.js it changes the contents of foo.js on my filesystem (and presumably restores it back afterwards)? Is this what happens?

@daffl
Copy link
Contributor

daffl commented Aug 2, 2016

It instruments them on the fly with connect-injector just like when it adds the client script to HTML test files.

@matthewp
Copy link
Contributor

matthewp commented Aug 2, 2016

I see, so if you are using a bundler the source has already been transpiled, but in the case of Steal it is has not.

Would it be possible to allow the user to hook into this pipeline so they could insert their own transpilation prior to the instrumentation step? That way you could possibly transpile just the import statements to commonjs before instanbul gets to it.

@matthewp
Copy link
Contributor

Talking about this problem a bit more, it feels like me the solution would be to use their Babel plugin (and only their Babel plugin) to add the coverage code on the fly, the same way that Testee currently adds the coverage code using the old Istanbul API (which internally uses an old Esprima version).

This would be a minimal change, only changing how coverage is added

Note: this would only run the istanbul babel plugin. It wouldn't transpile class syntax or import/export or anything else.

@phillipskevin phillipskevin self-assigned this Mar 30, 2017
@phillipskevin
Copy link
Contributor Author

I'm going to give this a try.

@phillipskevin
Copy link
Contributor Author

This is closed in v0.4.0. Nice work @andrejewski finishing this off!

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

5 participants