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 786 forks source link

Better integration with other pre-processors #212

Open schulzch opened 10 years ago

schulzch commented 10 years ago

When integrating Brunch, Karma and Istanbul precious information from source maps is lost. I think its a good idea to look for source maps during instrumentation (app.js.map for app.js) and display sources instead of intermediate code.

We use Brunch to compile the application (bare vendor.js and requirejs app.js) and Karma to compile tests, instrument app.js using Istanbul and run tests inside Chrome/Firefox/etc.. Unfortunately the coverage report and source view (developer tools) display a single app.js instead of source files. Random guessing what the source file was is a real pain in the ***, not to mention virtually useless statistics.

This possibly relates to:

gotwarlost commented 10 years ago

Is there a sample project and a build script that will help me understand all the moving parts without having to learn a bunch of tools? I realize that you are asking for something more abstract but I like to get my hands dirty with your use-case before trying to come up with a solution.

justindujardin commented 10 years ago

@gotwarlost see pow-core for an example, and an example report.

It would be much nicer if istanbul could look at my source maps and generate reports that include the original TypeScript files instead of one composite javascript file.

Clone the repo, set coverageDebug = true at the top of karma.config.js, and npm install && npm install bower && ./node_modules/.bin/bower install. Then npm test whenever you want to generate a new report, which will show up in .coverage

+1 for this feature.

jessaustin commented 9 years ago

@justindujardin I agree this would be a nice enhancement for istanbul. This is just a quick note to make installation of pow-core easier. Add a postinstall property to the scripts property of package.json:

  "scripts": {
    "postinstall": "bower install"
  }

Now a simple npm install is all you need!

chinchang commented 9 years ago

+1 Right now I need to run tests on development environment where there is no concatenation and minification so that I get coverage for source files.

schulzch commented 9 years ago

@gotwarlost pow-core matches my use case more or less (its using TypeScript instead of CoffeeScript and Grunt instead of Brunch).

One way to implement this feature would be by using Mozilla's Source Map parser:

  1. Parse source map in instrumenter.js:instrumentSync()
  2. Annotate each uninstrumented AST node with a source line and source column
  3. Modify this.sourceMap in instrumenter.js:instrumentASTSync() so that instrumented code points to source code instead of uninstrumented code

Viable approach?

vvakame commented 9 years ago

:+1:

vvakame commented 9 years ago

@gotwarlost I know good document for SourceMap. I think gulp ( and browserify ) is defacto of streaming processing tools. I think that it is good when you reference the gulp of policy.

https://github.com/floridoo/gulp-sourcemaps/wiki/Plugins-with-gulp-sourcemaps-support http://thlorenz.com/blog/browserify-sourcemaps https://github.com/azu/multi-stage-sourcemap https://github.com/twada/battlefield-sourcemaps

in karma-sourcemap. https://github.com/demerzel3/karma-sourcemap-loader/blob/2c9ee367aa9fcf7cf65bf8c224d5c89b7f151d12/index.js#L9

I want to use SourceMap with TypeScript and karma ( karma-sourcemap -> karma-coverage (it uses istanbul!) ).


this is very useful document if you can read japanese... http://efcl.info/2014/09/03/multi-stage-sourcemap/

fkling commented 9 years ago

:+1: Since transpilers became increasingly popular this year, having support for source maps would be tremendously useful.

lo1tuma commented 9 years ago

:+1:

arielschiavoni commented 9 years ago

+1

dayyan commented 9 years ago

:+1:

GoulwenBZH commented 9 years ago

+1

simonzack commented 9 years ago

:+1:, this really makes it hard with 6to5 or traceur. I cannot switch to istanbul due to this issue.

jvilk commented 9 years ago

:+1: I completely agree. It seems like this could be a post-processing step, since Istanbul already has the relevant JS source location information for covered / non-covered statements. I believe you can map those back to the original source code via the source maps.

jtangelder commented 9 years ago

:+1:

Iorg commented 9 years ago

+1

danvk commented 9 years ago

I wound up writing a script to apply a source map to LCOV data to get coverage data for a recent project. Built-in support from Istanbul would be fantastic.

gotwarlost commented 9 years ago

@danvk - this is great! This gives me a good starting point. Thanks!

brunodunbar commented 9 years ago

This feature is gonna be very helpful, especially when running tests on minified/uglified code.

gotwarlost commented 9 years ago

Working on it in the source-map branch... Code is still very raw and not ready for use.

mysterycommand commented 9 years ago

:+1:

tiriana commented 9 years ago

+1 Using source maps would increase built code coverage significantly. Are there any chances for this to happen? [typescript => js]

wojciak commented 9 years ago

+1 (ts -> js)

topaxi commented 9 years ago

+1 https://github.com/douglasduteil/isparta is doing something like this already for babel, but sadly isn't an option for me anymore

PinkaminaDianePie commented 9 years ago

+1 this feature more important than any other

rattrayalex commented 9 years ago

+1 TS

Any word on progress?

acromarco commented 9 years ago

+1 (ts -> js)

splintor commented 9 years ago

+1 (ts -> js)

lzilioli commented 9 years ago

+1, I'm using babel to compile es6 to es5 and then running the tests against the es5 babel output

loicoudot commented 9 years ago

+1 TypeScript will be very popular soon with AngularJS 2.0

kitsonk commented 9 years ago

@gotwarlost I noticed the source-map branch. I have been working on theintern/intern and dojo/core and being based on TypeScript, being able to map back to source for coverage is important. @bryanforbes and I have been looking at how we can include automated code coverage to our tooling chain. Right now I have hacked together kitsonk/grunt-cover-ts which tries to remap the lcov.info, but then realised that the coverage.json is better and was starting to look at remapping that back to source. I would be glad to help out with the source-map branch but would like to understand how best to contribute.

PinkaminaDianePie commented 9 years ago

@kitsonk some services like coveralls.io uses only lcov.info. So i think you need to remap both lcov.info and coverage.json

bryanforbes commented 9 years ago

@PinkaminaDianePie The process that @kitsonk is describing is simply what he has done in grunt-cover-ts (and what his thoughts were) to transform the coverage information after Istanbul outputs it. Since lcov doesn't have all the info needed to map back to the pre-transpiled source, his next steps were going to be to take use coverage.json to map back and then output whatever coverage format is needed (I would assume using Istanbul). Essentially:

  1. Run tests, generate coverage.json
  2. grunt-cover-ts uses coverage.json + source maps to map back to the TypeScript
  3. grunt-cover-ts then uses a modified coverage object and hands it to Istanbul to generate the appropriate coverage report (lcov, etc.)

The source-map branch adds the idea of a transformer to the Istanbul API which would take the coverage object and manipulate it before handing it to a reporter (such as lcov), thus making grunt-cover-ts unnecessary.

PinkaminaDianePie commented 9 years ago

@bryanforbes ah, ok.

gotwarlost commented 9 years ago

For the (ts -> js) folks, can someone provide a git repo with some hello world code and simple tests that I can use as an example to test some source-map support I recently added on the branch?

lukeapage commented 9 years ago

maybe this is too much because it integrates with grunt-istanbul etc. but it might be useful as its a small project - https://github.com/lukeapage/vindinium-ai

gotwarlost commented 9 years ago

@lukeapage - I tried your repo and used a small subset of your code and tests to validate my ideas. Thanks for the info.

gotwarlost commented 9 years ago

@kitsonk - I was actually able to get "good" reports on your dojo/core library (you need to verify that they are correct) by doing the following:

Result of a grunt test is that you get to see coverage for all your original files (.ts and not .js, I mean) with no "boilerplate coverage"

Could you give it a whirl and let me know if things look correct?

bryanforbes commented 9 years ago

@gotwarlost - @kitsonk and I are working together on coverage for dojo/core and he's on holiday so I tested your changes. This is amazing and it looks correct. I'll have some other colleagues try it out on other projects (dojo/dom, dojo/router, etc.) and let you know if we run into any problems. Thanks so much for doing this!!

gotwarlost commented 9 years ago

@bryanforbes - excellent, thanks for the quick test. Looking forward to more feedback.

bryanforbes commented 9 years ago

For anyone else trying this with Intern, you can do the second step of the instructions posted by @gotwarlost by doing the following:

cd node_modules/intern
npm install gotwarlost/istanbul#source-map
dpogue commented 9 years ago

I'm currently using istanbul as a two-step process:

  1. Compile TypeScript to JS and add istanbul instrumentation using browserify-istanbul.
  2. Run the tests and collect coverage stats using karma-coverage.

Will having those as separate steps cause problems with the source map caching? If you think it should work, I'm happy to spend a bit of time testing it

gotwarlost commented 9 years ago

This use case should simply work as long as you use the source-map branch. Try it out and let me know. Thanks for volunteering to test this.

dpogue commented 9 years ago

It seems to have problems when directories are involved, I'm getting the following error:

Post-processing using source maps
ERROR [coverage]: { [Error: ENOENT, no such file or directory '/project_dir/src/alert/src/alert/alert.ts']
  errno: -2,
  code: 'ENOENT',
  path: '/project_dir/src/alert/src/alert/alert.ts',
  syscall: 'open' }
Error: ENOENT, no such file or directory '/project_dir/src/alert/src/alert/alert.ts'
    at Error (native)
    at Object.fs.openSync (fs.js:500:18)
    at Object.fs.readFileSync (fs.js:352:15)
    at LookupStore.Store.mix.get (/project_dir/node_modules/karma-coverage/node_modules/istanbul/lib/store/fslookup.js:40:19)
    at HtmlReport.Report.mix.writeDetailPage (/project_dir/node_modules/karma-coverage/node_modules/istanbul/lib/report/html.js:406:83)
    at /project_dir/node_modules/karma-coverage/node_modules/istanbul/lib/report/html.js:484:26
    at SyncFileWriter.extend.writeFile (/project_dir/node_modules/karma-coverage/node_modules/istanbul/lib/util/file-writer.js:57:9)
    at FileWriter.extend.writeFile (/project_dir/node_modules/karma-coverage/node_modules/istanbul/lib/util/file-writer.js:147:23)
    at /project_dir/node_modules/karma-coverage/node_modules/istanbul/lib/report/html.js:483:24
    at Array.forEach (native)

All of the sources in the sourcemap are relative to the main project directory (/project_dir), but it seems to be trying to map them as if they were relative to the file being covered.

gotwarlost commented 9 years ago

@dpogue - thanks for the report. Could you give me some more info

dpogue commented 9 years ago

The compiler options for TypeScript are:

target: es5
module: commonjs
declaration: true
noImplicitAny: false
removeComments: false
noLib: false
outDir: dist
sourceMap: true

The sourcemaps are inline. I de-base64'ed the inline sourcemap and the relative paths are correct (relative to the project root).

gotwarlost commented 9 years ago

For the babel + karma folks, I have a stupid simple repo that demonstrates how to get this to work with source maps. Any takers for testing on a real project?

Thanks to @ezequiel who set this up as a simple example.

https://github.com/gotwarlost/babel-coverage

gotwarlost commented 9 years ago

@dpogue what's the version of typescript that you are using?

bryanforbes commented 9 years ago

@gotwarlost - I spoke too soon. One test run works fine (for instance grunt test or grunt test-local), however when a test run's final coverage information is output to JSON and then combined with the coverage information for a second test run, the coverage information for the first run is the only one that survives. To see what I'm talking about, follow these steps:

Once everything is complete, check html-report/src/queue.ts.html. You'll notice that line 173 is not covered. If you change Gruntfile.js back to lcovhtml and run grunt clean:coverage test-local again, you'll see that line 173 is covered by the code. I believe this happens because the post-processed coverage information from the first run is being combined with the pre-processed coverage information of the second run.

gotwarlost commented 9 years ago

@bryanforbes - this is exactly the kind of testing I was looking for. Thanks for the report. I'll look into it this evening.