Make coverage reports es6 #63

magicmark · 2017-04-13T10:36:22Z

Switched to nyc from istanbul
Moved coverage report levels (need to investigate why they dropped in some cases)
Changed some test imports to import from src rather than lib

(Used to look like this, now they look like this

test:quick and tests-only are kinda useless now but unsure if safe to deprecate (major bump?)

ljharb · 2017-04-13T23:55:24Z

nyc has more accurate coverage than istanbul-cli, so it's ok if they dropped a little bit.

ljharb · 2017-04-13T23:56:38Z

package.json

-    "test:quick": "babel-node node_modules/.bin/_mocha -R tap test/init.js test/*-test.js"
+    "test": "npm run test:coverage && npm run --silent cover:check",
+    "tests-only": "npm run test:quick",
+    "test:coverage": "nyc --silent node_modules/.bin/_mocha -R tap test/init.js test/*-test.js",


while we're at it, can this just use mocha directly, instead of needing the _mocha istanbul hack?

ljharb · 2017-04-13T23:56:56Z

server.js

@@ -1 +1 @@
-module.exports = require('./lib/server.js');
+module.exports = require('./src/server.js');


This will break the package. Please revert it.

ljharb · 2017-04-13T23:57:07Z

test/BatchManager-test.js

@@ -2,7 +2,7 @@ import { assert } from 'chai';
 import sinon from 'sinon-sandbox';

 import { makeJob, COMPONENT_NAME } from './helper';
-import BatchManager from '../lib/utils/BatchManager';
+import BatchManager from '../src/utils/BatchManager';


Similarly, we actually want tests to be run on babel output; not on the raw source.

I believe we can have source maps, even when we're running tests on the babel output. I've set this up with nyc in another package.

magicmark · 2017-05-27T04:07:12Z

@ljharb rebased + made changes. I guess babel-node isn't needed since nyc is using babel-register to transpile tests.

ljharb

Thanks, this is almost ready!

ljharb · 2017-05-27T04:14:06Z

package.json

-    "test:quick": "babel-node node_modules/.bin/_mocha -R tap test/init.js test/*-test.js"
+    "test:coverage": "nyc --silent node_modules/.bin/mocha -R tap test/init.js test/*-test.js",
+    "cover:check": "nyc report && echo code coverage thresholds met, achievement unlocked!",
+    "test:quick": "npm run test:coverage"


the point of the "quick" script is to run without coverage :-) can this line be reverted?

ljharb · 2017-05-27T04:17:03Z

.nycrc

@@ -0,0 +1,17 @@
+{
+  "lines": 91,
+  "functions": 56,


When I ran this locally on node 7, it failed with 55.something - could we drop this to 55 just to avoid the failure later, when I add node 7 to travis?

- Switch to nyc - Update thresholds

magicmark · 2017-07-05T21:49:18Z

@ljharb rebased again

passing with node 6 and 7:

v6.3.1:
=============================== Coverage summary ===============================
Statements   : 91.06% ( 163/179 )
Branches     : 70.31% ( 45/64 )
Functions    : 57.89% ( 44/76 )
Lines        : 90.8% ( 158/174 )
================================================================================

v7.7.3:
=============================== Coverage summary ===============================
Statements   : 91.06% ( 163/179 )
Branches     : 70.31% ( 45/64 )
Functions    : 56.58% ( 43/76 )
Lines        : 90.8% ( 158/174 )
================================================================================

ljharb · 2017-07-05T22:33:03Z

package.json

@@ -5,13 +5,13 @@
  "main": "lib/index.js",
  "scripts": {
    "prepublish": "not-in-publish || safe-publish-latest && npm run build",
-    "build": "rimraf lib && babel src -d lib",
+    "build": "rimraf lib && babel --source-maps inline src -d lib",


Can we avoid using inline source maps, and use regular ones instead?

Hmm, running into some problems here.

The higher quality source maps decreases the coverage which isn't a big deal (I guess this was the point :D), but I can't get nyc to map back to original sources.

I tried both with and without the babel-plugin-istanbul plugin, with every combination of sourceMap, instrument and env vars, but I couldn't get the mapping back from the instrumentation code+es5/es5 respectively :(

I can't find anything in the nyc or babel-plugin-istanbul docs about external source maps - I also notice the documentation seems to recommend using inline source maps (https://github.com/istanbuljs/nyc#support-for-custom-require-hooks-babel-webpack-etc)

I'll open an issue+reproducible example on nyc later, but for now, this is blocking me :(

Have you got any thoughts w/r solving this? (You mentioned you had another repo set up with nyc?)

https://github.com/airbnb/prop-types/blob/master/package.json#L52 is the other repo.

I'm not sure how to set it up.

It would be fine to build with inline source maps only when building for coverage, it just means we have to build twice.

ljharb · 2017-07-05T22:33:28Z

package.json

    "lint": "eslint src test",
    "pretest": "npm run --silent lint",
    "test": "npm run build && npm run test:coverage && npm run --silent cover:check",
    "tests-only": "npm run build && npm run test:quick",
-    "test:coverage": "babel-node node_modules/.bin/istanbul cover --report html node_modules/.bin/_mocha -- -R tap test/init.js test/*-test.js",
-    "cover:check": "istanbul check-coverage && echo code coverage thresholds met, achievement unlocked!",
+    "test:coverage": "nyc --silent node_modules/.bin/mocha -R tap test/init.js test/*-test.js",


can we use nyc --silent "$(which mocha)" … rather than hardcoding node_modules?

magicmark force-pushed the make_coverage_reports_es6 branch from fcc5301 to 8f2c1fc Compare April 13, 2017 10:37

magicmark mentioned this pull request Apr 13, 2017

Adding sourcemaps for coverage reports #61

Open

ljharb suggested changes Apr 13, 2017

View reviewed changes

magicmark force-pushed the make_coverage_reports_es6 branch from 8f2c1fc to 3d63634 Compare May 27, 2017 03:44

ljharb suggested changes May 27, 2017

View reviewed changes

magicmark force-pushed the make_coverage_reports_es6 branch from 3d63634 to f585654 Compare May 28, 2017 16:12

Add sourcemaps to code coverage reports

5229cc0

- Switch to nyc - Update thresholds

magicmark force-pushed the make_coverage_reports_es6 branch from f585654 to 5229cc0 Compare July 5, 2017 21:48

ljharb reviewed Jul 5, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make coverage reports es6 #63

Make coverage reports es6 #63

magicmark commented Apr 13, 2017

ljharb commented Apr 13, 2017

ljharb Apr 13, 2017

ljharb Apr 13, 2017

ljharb Apr 13, 2017

ljharb Apr 13, 2017

magicmark commented May 27, 2017 •

edited

Loading

ljharb left a comment

ljharb May 27, 2017

ljharb May 27, 2017

magicmark commented Jul 5, 2017

ljharb Jul 5, 2017

magicmark Jul 6, 2017 •

edited

Loading

ljharb Jul 6, 2017

ljharb Jul 5, 2017

		@@ -1 +1 @@
		module.exports = require('./lib/server.js');
		module.exports = require('./src/server.js');

Make coverage reports es6 #63

Are you sure you want to change the base?

Make coverage reports es6 #63

Conversation

magicmark commented Apr 13, 2017

ljharb commented Apr 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magicmark commented May 27, 2017 • edited Loading

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magicmark commented Jul 5, 2017

Choose a reason for hiding this comment

magicmark Jul 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magicmark commented May 27, 2017 •

edited

Loading

magicmark Jul 6, 2017 •

edited

Loading