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

Getting weird coverage reports #56

Open
ryanzec opened this issue Feb 24, 2016 · 16 comments
Open

Getting weird coverage reports #56

ryanzec opened this issue Feb 24, 2016 · 16 comments

Comments

@ryanzec
Copy link

ryanzec commented Feb 24, 2016

So I am trying to use this to do coverage reports on my es6 code with babel however I am getting weird results. For example, I see this in the html output of:

screen shot 2016-02-24 at 2 49 53 pm

Line 1 is weird as it is expecting to test a if / else / else statement when there is none. Lines 4 and 5 are weird as I don't know what it is talking about as I do render an Application component in my tests. The command I am using to run my tests is:

./node_modules/.bin/babel-node ./node_modules/.bin/babel-istanbul cover ./node_modules/.bin/_mocha

and my mocha.opts file is:

--compilers js:./test/compiler.js
--reporter spec
--timeout 4000
--require test/mocha-generator.js
--require test/setup.js

test/specs/**/*.spec.jsx

Am I missing something are are there false failure showing up?

@jeffrifwald
Copy link
Owner

If you dig into the code generated by babel at the import * as React from 'React'; line, you'll find the mystery branches live in that code. I'm not sure why your coverage report is not ignoring the code generated by babel. What version of babel are you using? I'll investigate further, but I think there are a few possible problems:

  1. Your project's version of babel could be generating very different code than the version used by babel-istanbul. This package is currently using 6.3.x, so I might need to update it to be any version of 6+.
  2. Something weird could be happening in your compilers.js mocha-generator.js, or setup.js files.

@ryanzec
Copy link
Author

ryanzec commented Feb 25, 2016

So the generated code looks like this in case it is useful to you (with 6.5.x version of babel):

'use strict';var _createClass = function () {function defineProperties(target, props) {for (var i = 0; i < props.length; i++) {var descriptor = props[i];descriptor.enumerable = descriptor.enumerable || false;descriptor.configurable = true;if ("value" in descriptor) descriptor.writable = true;Object.defineProperty(target, descriptor.key, descriptor);}}return function (Constructor, protoProps, staticProps) {if (protoProps) defineProperties(Constructor.prototype, protoProps);if (staticProps) defineProperties(Constructor, staticProps);return Constructor;};}();var _react = __webpack_require__(153);var React = _interopRequireWildcard(_react);var _headerComponent = __webpack_require__(254);var _headerComponent2 = _interopRequireDefault(_headerComponent);function _interopRequireDefault(obj) {return obj && obj.__esModule ? obj : { default: obj };}function _interopRequireWildcard(obj) {if (obj && obj.__esModule) {return obj;} else {var newObj = {};if (obj != null) {for (var key in obj) {if (Object.prototype.hasOwnProperty.call(obj, key)) newObj[key] = obj[key];}}newObj.default = obj;return newObj;}}function _classCallCheck(instance, Constructor) {if (!(instance instanceof Constructor)) {throw new TypeError("Cannot call a class as a function");}}function _possibleConstructorReturn(self, call) {if (!self) {throw new ReferenceError("this hasn't been initialised - super() hasn't been called");}return call && (typeof call === "object" || typeof call === "function") ? call : self;}function _inherits(subClass, superClass) {if (typeof superClass !== "function" && superClass !== null) {throw new TypeError("Super expression must either be null or a function, not " + typeof superClass);}subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, enumerable: false, writable: true, configurable: true } });if (superClass) Object.setPrototypeOf ? Object.setPrototypeOf(subClass, superClass) : subClass.__proto__ = superClass;}var 


    Application = function (_React$Component) {_inherits(Application, _React$Component);
      function Application(props) {_classCallCheck(this, Application);return _possibleConstructorReturn(this, Object.getPrototypeOf(Application).call(this, 
        props));}_createClass(Application, [{ key: 'render', value: function render() 


        {
          return (
            React.createElement('div', { className: 'application' }, 
            React.createElement(_headerComponent2.default, null), 
            this.props.children));} }]);return Application;}(React.Component);





    Application.displayName = 'Application';

    module.exports = Application;

I also tried updating the following libraries to the listed version:

babel-cli@6.3.17
babel-core@6.3.26
babel-istanbul@0.6.0
babel-plugin-transform-react-jsx@6.3.13
babel-preset-es2015@6.3.13

and got the same result.

As for my other files, they are

compilers.js

var babel = require('babel-core/register');
var noop = function(){return null;};

//assets the webpack parsers that node can't
require.extensions['.html'] = noop;
require.extensions['.scss'] = noop;
require.extensions['.jpg'] = noop;
require.extensions['.jpeg'] = noop;
require.extensions['.png'] = noop;

mocha-generators.js

var co = require('co');
var mocha = require('mocha');
var Runnable = mocha.Runnable;
var run = Runnable.prototype.run;

Runnable.prototype.run = function(fn) {
  if(this.fn.constructor.name === 'GeneratorFunction') {
    this.fn = co(this.fn);
    this.sync = !(this.async = true);
  }

  return run.call(this, fn);
};

setup.js

//NOTE: setup expect as global
import {expect} from 'chai';

global.expect = expect;

//NOTE: setup JSDOM for test running
import * as jsdom from 'jsdom';

var exposedProperties = ['window', 'navigator', 'document'];

global.document = jsdom.jsdom('');
global.window = document.defaultView;

Object.keys(document.defaultView).forEach((property) => {
  if (typeof global[property] === 'undefined') {
    exposedProperties.push(property);
    global[property] = document.defaultView[property];
  }
});

global.navigator = {
  userAgent: 'node.js'
};

//NOTE: need to include the i18n testing file
global.window.i18n = require('./i18n-node.js');

//NOTE: include all files that should have coverage numbers on them in-case there are files the specs don't trigger inclusion
import globArray from 'glob-array';

var filesForCoverage = [
  process.cwd() + '/web/app/**/*.js',
  process.cwd() + '/web/app/**/*.jsx',

  //NOTE: pages are tested through ui tests, not unit tests
  '!' + process.cwd() + '/web/app/pages/**/*.js',
  '!' + process.cwd() + '/web/app/pages/**/*.jsx',

  //NOTE: mocks are not that important to coverage test
  '!' + process.cwd() + '/web/app/mock/**/*.js',

  //NOTE: 3rd party libraries that there is no point in testing
  '!' + process.cwd() + '/web/app/misc/ua-parser.js',

  //router based stuff seems to cause issue with needing the history location for react-router so just skip those file
  '!' + process.cwd() + '/web/app/application.jsx',
  '!' + process.cwd() + '/web/app/router.jsx',
  '!' + process.cwd() + '/web/app/routes.jsx'
];

globArray.sync(filesForCoverage).forEach(function(filePath) {
  require(filePath);
});

Let me know if you see anything weird that might be causing this issue in these files or if I can provide any other information to help you debug this issue.

@jeffrifwald
Copy link
Owner

I am suspecting something strange could be happening in your filesForCoverage setup step. Perhaps try omitting that part to see if it helps.

I'll go ahead and update the babel dependency also, just to make sure it isn't related.

@ryanzec
Copy link
Author

ryanzec commented Mar 1, 2016

Still the same issue with that block of code relating to filesForCoverage removed. I mean since it just includes files it should not cause any issues (not to mention I don't think there is any other way of make sure that a file is included in the report incase the unit tests don't trigger the inclusion of the all the files).

@jeffrifwald
Copy link
Owner

@ryanzec The preferred method of making sure all files are covered is using a common root setting and using the include-all-sources flag either on the command line or in your .istanbul.yml file. I just wanted to make sure that wasn't causing the issue since require magic often does cause a lot of trouble with Istanbul.

I have published 0.6.1, so you should try to install from scratch (you want to make sure old versions of babel are not being used) and see if that fixes your issue.

@jeffrifwald
Copy link
Owner

Also, one other thing you can try is import React from 'React'; instead of import * as React from 'React';. The star import produces a lot more code than the require import.

@ryanzec
Copy link
Author

ryanzec commented Mar 2, 2016

It was the import * as React that was causing the issue. Weird that other locations use that way of importing did not report the same issues.

I do agree that it is generally bad to do import * for the reasons you mentioned but I was unaware I could import react and react-dom in that way. I have updated by code and guess I will close this issue for now. If I find it again, I will re-open.

@ryanzec ryanzec closed this as completed Mar 2, 2016
@ryanzec
Copy link
Author

ryanzec commented Mar 6, 2016

Ok, getting this issue again. This seems to be an issue when using import * as ..., the new use case is:

screen shot 2016-03-05 at 8 16 16 pm

and the prevent-double-click.constants.js is:

export const ENABLE = 'PreventDoubleClick::enable';
export const DISABLE = 'PreventDoubleClick::disable';

This is a case where I think using the import * as ... is valid.

@ryanzec ryanzec reopened this Mar 6, 2016
@ryanzec
Copy link
Author

ryanzec commented Mar 6, 2016

More weirdness in the results:

screen shot 2016-03-06 at 6 33 24 am

I am not sure why it is telling me that the red highlight statement is not covered when it shows that the class is created twice.

The only code coverage number that seems to be reliable is the lines covered.

@jeffrifwald
Copy link
Owner

Your only option here is to look at the generated code and try to figure out why it is doing what it is doing. Babel is literally creating statements that are not being run, so Istanbul is doing its job. This project has no control over how Istanbul covers or how Babel compiles. With that being said, you can try a few things:

  1. Skip the statements that are causing trouble.
  2. Use the runtime plugin (this could solve some, but not all your problems)
  3. Bug the Babel maintainers (or contribute yourself) to make the compiled code less verbose.

@szilveszter9
Copy link

I've had the same issue using babel-node ./node_modules/.bin/babel-istanbul
but pure istanbul@1.0.0-alpha.2 using babel-node ./node_modules/istanbul/lib/cli.js works fine

@ryanzec
Copy link
Author

ryanzec commented Mar 29, 2016

I guess I will use the 1.x.x branch of the regular istanbul and see how using that goes.

@silkentrance
Copy link

silkentrance commented Apr 28, 2016

@ryanzec do you by any chance use retainLines : true in your babelrc and in the context of instrumentation of your sources?

Nonetheless, I also get weird coverage results. The report shows 100% branch/function/line whatever coverage, but when digging into the generated HTML report it shows that multiple lines/branches are not being covered at all.

This happens with babel@6.7.7 (babel-core 6.7.7)

image

My guess is that it is the ternary operator that is causing this in this case. And, after refactoring of that ternary, I get the following, more sane report

image

Looking at the generated code, though

            this._isStrict = typeof strict == 'undefined' ? true : strict;

shows that nothing special is happening here. Perhaps istanbul should be upgraded again 👯

@jeffrifwald
Copy link
Owner

@silkentrance @ryanzec I actually have not been able to get completely correct coverage from babel-istanbul, isparta, or istanbul's aphla. I believe there is just some inherent weirdness with the source maps generated by babel. Both babel-istanbul and isparta work by skipping coverage in the instrumentation layer based on source maps. Istanbul's alpha works in the reporting layer, but also uses source maps. My hunch is that some of the transformers do not completely play well with source mapping. That is, they probably don't clean up after themselves very well. I've pinpointed babel-plugin-transform-es2015-typeof-symbol as one that causes trouble for me.

TLDR: Any implementation that uses source maps for coverage are at the mercy of the quality of the source map.

@silkentrance
Copy link

@jmcriffey yeah, however, and in my case I think that it maybe has something to do with the HTML reporter, though. Since, when I go through my test cases and sources I think that the counters to the left are correct, but that the HTML reporter somehow gets it all wrong. We will have to wait for an istanbul update, I think.

@NeoPhi
Copy link

NeoPhi commented May 23, 2016

While it is a newer project https://github.com/dtinth/babel-plugin-__coverage__ was able to produce accurate HTML reports for us.

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