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

Reported coverage only increases and never decreases without terminating gulp #40

Open
analog-nico opened this issue Oct 2, 2014 · 6 comments
Labels

Comments

@analog-nico
Copy link

I wrote several unit tests and get the coverage report (html) as expected. I have a watch task and therefore execute the unit tests again and again. When I add another test the reported coverage goes up as expected. However, when I comment out a test the reported coverage does not go down again. For an accurate coverage report I have to terminate gulp and start my watch task again.

My otherwise good working code is:

'use strict';

var gulp = require('gulp');
var runSequence = require('run-sequence');
var jshint = require('gulp-jshint');
var jshintStylish = require('jshint-stylish');
var istanbul = require('gulp-istanbul');
var mocha = require('gulp-mocha');
var chalk = require('chalk');

var chai = require("chai");
chai.use(require("chai-as-promised"));
global.expect = chai.expect;


var paths = {
    common: {
        jsFiles: './lib/common/**/*.js'
    },
    crawler: {
        jsFiles: './lib/crawler/**/*.js',
        specFiles: './test/crawler/spec/**/*.js',
        fixtureFiles: './test/crawler/fixtures/**/*.js'
    },
    webserver: {
        jsFiles: './lib/webserver/**/*.js'
    }
};

gulp.task('watch-crawler', ['validate-crawler'], function () {

    return gulp.watch([
        paths.crawler.jsFiles,
        paths.common.jsFiles,
        paths.crawler.specFiles,
        paths.crawler.fixtureFiles
    ], [
        'validate-crawler'
    ]);

});

gulp.task('validate-crawler', function (done) {
    runSequence('lint-crawler', 'test-crawler', done);
});

gulp.task('lint-crawler', function () {

    return gulp.src([paths.common.jsFiles, paths.crawler.jsFiles, paths.crawler.fixtureFiles])
        .pipe(jshint())
        .pipe(jshint.reporter(jshintStylish))
        .pipe(jshint.reporter('fail'));

});

gulp.task('clean-coverage', function (done) {
    (require('rimraf'))('./coverage', done);
});

gulp.task('test-crawler', ['clean-coverage'], function (done) {

    gulp.src([
            paths.crawler.jsFiles,
            paths.common.jsFiles
        ])
        .pipe(istanbul())
        .on('finish', function () {

            gulp.src([
                    paths.crawler.specFiles
                ])
                .pipe(mocha())
                .on('error', function () {
                    console.log(chalk.bold.bgRed(' TESTS FAILED '));
                    done();
                })
                .pipe(istanbul.writeReports({
                    reporters: ['lcov']
                }))
                .on('end', done);

        });

});

Did I configure something wrong? Or what else might be the cause?

@analog-nico
Copy link
Author

I just got really weird failures when editing my instrumented code. After restarting gulp and not changing the code it is working again. It seems that my code is instrumented multiple times. Maybe that is a hint...

@SBoudrias
Copy link
Owner

You probably shouldn't rerun the instrumentation on each watch event triggers if you're running everything in the same process.

@analog-nico
Copy link
Author

Good idea! However, this did not fix it.

Weird. I never got this issue before in other configurations. (I just verified it again.)

  • Gulp + Karma including Istanbul (gulpfile.js)
  • Grunt + grunt-jasmine-node-coverage which uses Istanbul (gruntfile.js)

Do you have any other idea? (I am diving into the internals now.)

@analog-nico
Copy link
Author

I found the cause. :)

  • The instrumentation works fine. Even if I instrument it repeatedly. (Which I have to do since I may add additional files while my watch task is running.)
  • The cause lies within the COVERAGE_VARIABLE in the code of gulp-instanbul: The variable is set once the module is loaded and remains the same for all runs in the watch cycle. Since global[opts.coverageVariable] still contains the coverage data of the previous cycle the coverage data of the next cycle is added on top!

My current workaround is to provide my own coverageVariable which is different for every watch cycle:

gulp.task('test-crawler', ['clean-coverage'], function (done) {

    var coverageVariable = '$$cov_' + new Date().getTime() + '$$';

    gulp.src([
            paths.crawler.jsFiles,
            paths.common.jsFiles
        ])
        .pipe(istanbul({
            coverageVariable: coverageVariable
        }))
        .on('finish', function () {

            gulp.src([
                    paths.crawler.specFiles
                ])
                .pipe(mocha())
                .on('error', function () {
                    console.log(chalk.bold.bgRed(' TESTS FAILED '));
                    done();
                })
                .pipe(istanbul.writeReports({
                    reporters: ['lcov'],
                    coverageVariable: coverageVariable
                }))
                .on('end', done);

        });

});

@SBoudrias It would be great if you could find a nicer solution that you can bake into gulp-instanbul. I guess many people fall into this trap. (Hopefully they notice and not just think their coverage is higher than it actually is.) My current workaround also has the disadvantage that the global namespace is polluted when my watch task is running for a long time. (My RAM would be big enough but it would be nice to know that this - you could call it memory leak - is fixed.)

@SBoudrias SBoudrias added the bug label Oct 3, 2014
@SBoudrias
Copy link
Owner

Thanks for digging the bug. I'll see what I can do about it.

Creating a new var for each run would be a good first step. I'm just unsure about deleting it as this plugin don't know when the user is done with the instrumented code.

@analog-nico
Copy link
Author

Mmh, good point. As I understand you baked into gulp-instanbul the ability to run multiple test runs (e.g. one for unit tests in the browser, one for the server libs, and one for end to end tests in the browser) and finally do a report that sums up the coverage of all test runs? That is powerful! +1

I would say there are two groups of users:

  • Someone like in my case who calls the instrumentation once, executes a test runner once, and writes reports once.
  • Someone more advanced who does at least one of those things more than once in a single watch cycle.

Maybe there is a solution that allows the first group not having to do anything and the second group e.g. passes additional options. The second group is certainly more advanced and can be burdened with the task to get the options right. (Namely setting coverageVariable as needed and maybe making a clear call once they are done.)
I just realize that issue #39 is somehow related. I would count @traviswimer into the group of people who should not be burdened with getting the options right. (He has the same use case as I do but runs two of those tasks in parallel.)

So I guess it is indeed a good starting point to create a new coverageVariable for each run. However, this is certainly tricky. Is it for example possible to ask gulp in which task the call to gulp-instanbul was made? In the simple use cases all calls are made in the same task I would say. And if you create a new coverageVariable right before you call instrumenter.instrument(...) and use it in subsequent calls for e.g. reporting in the same gulp task this might solve most use cases.

Just my 5 cents. The more I think about it the more I realize how tricky it is. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants