Proof of concept of a more sustainable coverage approach.

[m-a-r-c-e-l-i-n-o]

The popular karma-coverage does not work as expected for me, mainly because Istanbul will try to instrument es6/es7 files and fail. As mentioned here, the workaround would be to use karma-babel-preprocessor, but there are two issues with this approach:

1. This requires the user to make sure "karma-babel-preprocessor" is using the same babel configuration/presets as their jspm setup, as well as additional configuration for source mapping.
2. There are other transpilers/customization without a solid solution.

Here lies a proof of concept for allowing jspm to do all the heavy lifting with practically no additional configuration. This approach has been successfully used in node-jspm-jasmine — I helped author it. I'm not suggesting that this be merged blindly, but do open room for discussion. If there is a better approach, please do let me know!

Notice that the "reporter.js" file, along with its local file dependencies, are nearly identical to the one in the karma-coverage project, with almost all modifications occurring inside here.

[maxwellpeterson-wf]

I think this is looking great, thanks for such a substantial contribution!

@evanweible-wf want to take a look?

[evanweible-wf]

Yeah looks good to me as well!

[m-a-r-c-e-l-i-n-o]

@maxwellpeterson-wf @evanweible-wf Thanks for the positive feedback! I'll look into what needs to be cleaned up and update this pr with a release candidate for you guys to test in the next few days/weeks.

[sergei-maertens]

this is looking great - is there any ETA on a RC?

[evanweible-wf]

@sergei-maertens sorry, I hadn't noticed that @m-a-r-c-e-l-i-n-o had rebased. I'll take another look today!

[sergei-maertens]

After two days of diving into way too much source code I have a working coverage with ES6 + React/JSX set up, so I can continue and there's no hurry, but this does seem cleaner and saner than our current setup :-)

[m-a-r-c-e-l-i-n-o]

@evanweible-wf @sergei-maertens I was waiting (but also had other responsibilities) on this pr to move forward, and it was finally accepted a few days ago. I'll be updating this pr with a much cleaner version by tomorrow, it's pretty much done.

[evanweible-wf]

@m-a-r-c-e-l-i-n-o sounds great!

[m-a-r-c-e-l-i-n-o]

Updated this pr with a decent release candidate. It passed the unit and lint tests, and runs just fine on one of my local projects. I've done no isolated tests, but it should be fine for now.

@maxwellpeterson-wf @evanweible-wf @sergei-maertens Please pull down this pr locally and let me know if any of you have issues. Couple of things to note:

• Since we are using the reporters from "karma-coverage", all the "coverageReporter" settings are valid for "karma-jspm" as well, use it the same way you would with "karma-coverage".
• Include "jspm" in reporters, preprocessors, and frameworks.
• I had to install "karma-coverage" directly from their github repo since npm hasn't been updated yet.
• I had to install "remap-istanbul" directly from a fork in my github since I'm still waiting on this pr.
• If you see a folder on your "html" report called "no-source-map", it means that one or more of your files was transpiled without a sourcemap. You'll need to make sure your jspm sourcemap generator is enabled. (It should be enabled by default.)
• Here's what my karma config file looks like:

module.exports = function(config) {
    config.set({
        basePath: '',
        frameworks: [ 'jspm', 'jasmine' ],
        plugins: [
            'karma-jspm',
            'karma-jasmine',
            'karma-chrome-launcher'
        ],
        jspm: {
            stripExtension: false,
            config: 'jspm.config.js',
            packages: 'jspm_packages',
            loadFiles: [
                'src/client/**/*.js',
                'src/shared/**/*.js'
            ]
        },
        proxies: {
            '/src': '/base/src',
            '/node_modules': '/base/node_modules',
        },
        preprocessors: {
            'src/client/!(test)/*.js': [ 'jspm' ],
            'src/shared/**/*.js': [ 'jspm' ]
        },
        // optionally, configure the reporter
        coverageReporter: {
            type : 'html',
            dir : process.cwd(),
            subdir: 'coverage'
        },
        reporters: [ 'progress', 'jspm' ],
        port: 9876,
        colors: true,
        debug: false,
        logLevel: config.LOG_INFO,
        autoWatch: false,
        browsers: [ 'Chrome' ],
        singleRun: true,
        concurrency: Infinity
    })
}

[sergei-maertens]

I will try it out asap. On Jun 7, 2016 21:18, "Marceli.no" notifications@github.com wrote:

Updated this pr with a decent release candidate. It passed the unit and lint tests, and runs just fine on one of my local projects. I've done no isolated tests, but it should be fine for now.

@maxwellpeterson-wf https://github.com/maxwellpeterson-wf @evanweible-wf https://github.com/evanweible-wf @sergei-maertens https://github.com/sergei-maertens Please pull down this pr locally and let me know if any of you have issues. Couple of things to note:

• Since we are using the reporters from "karma-coverage https://github.com/karma-runner/karma-coverage", all the "coverageReporter" settings are valid for "karma-jspm" as well, use it the same way you would with "karma-coverage".
• Include "jspm" in reporters, preprocessors, and frameworks.
• I had to install "karma-coverage" directly from their github repo since npm hasn't been updated yet.
• I had to install "remap-istanbul https://github.com/SitePen/remap-istanbul" directly from a fork in my github since I'm still waiting on this pr https://github.com/SitePen/remap-istanbul/pull/62.
• If you see a folder on your "html" report called "no-source-map", it means that one or more of your files was transpiled without a sourcemap. You'll need to make sure your jspm sourcemap generator is enabled. (It should be enabled by default.)
• Here's what my karma config file looks like:

module.exports = function( config ) { config.set( { basePath: '', frameworks: [ 'jspm', 'jasmine' ], plugins: [ 'karma-jspm', 'karma-jasmine', 'karma-chrome-launcher', 'karma-coverage' ], jspm: { stripExtension: false, config: "jspm.config.js", browser: "jspm.browser.js", loadFiles: [ 'src/client//!(bundle|reloader).js', 'src/client/test/!(coverage)//_.js', 'src/shared/_/.js', 'app.prod.config.js', 'app.dev.config.js' ] }, reporters: [ 'progress', 'jspm' ], preprocessors: { 'src/shared/*/.js': [ 'jspm' ] }, port: 9876, colors: true, logLevel: config.LOG_INFO, autoWatch: true, browsers: [ 'Chrome' ], singleRun: true, concurrency: Infinity, coverageReporter: { type : 'html', dir : 'src/client/test', subdir: 'coverage' } } ) }

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Workiva/karma-jspm/pull/147#issuecomment-224385602, or mute the thread https://github.com/notifications/unsubscribe/AFQ01j8rYsqUk60R6ZDbF3wR9s22Pt0Wks5qJcQYgaJpZM4ISP1c .

[m-a-r-c-e-l-i-n-o]

Any thoughts on this, guys?

[aviary-wf]

Raven

Number of Findings: 0

[evanweible-wf]

@maxwellpeterson-wf @trentgrover-wf

[maxwellpeterson-wf]

This is looking good. @sergei-maertens did you get a chance to try it out?

[sergei-maertens]

Oh shuck - I completely forgot about this since we're using our fork at the moment. I don't see any problems though. On Jul 11, 2016 4:41 PM, "Max Peterson" notifications@github.com wrote:

This is looking good. @sergei-maertens https://github.com/sergei-maertens did you get a chance to try it out?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Workiva/karma-jspm/pull/147#issuecomment-231754876, or mute the thread https://github.com/notifications/unsubscribe/AFQ01tHfe0f9vbwxXRzYSsYMoaCH-FnDks5qUlYCgaJpZM4ISP1c .

[m-a-r-c-e-l-i-n-o]

@evanweible-wf @maxwellpeterson-wf @trentgrover-wf Hold off on this merge, I'm going to use it in a pretty complex project in the next few days, it'll be a good chance to more thoroughly test and patch this baby, if needed.

[maxwellpeterson-wf]

@m-a-r-c-e-l-i-n-o great, thanks!

[rmconsole-wf]

@m-a-r-c-e-l-i-n-o This pull request has merge conflicts, please resolve.

[cyrixmorten]

@m-a-r-c-e-l-i-n-o Tried out using your branch with your version of remap-istanbul as you described above, and it seems to be almost working.

I get the following error: [reporter-wrapper.coverage]: [TypeError: Cannot read property 'tempDirectory' of undefined]

After uncommenting rimraf.sync(rootConfig.client.jspm.coverage.tempDirectory) in reporter.js, I am able to get a coverage report output.

We have a setup that only does coverage when verifying the whole project, so my suspicion is that the error might occur when coverage is disabled. Haven't dived that deeply into it yet, just wanted to give some feedback :)

[m-a-r-c-e-l-i-n-o]

@cyrixmorten Thank you for the feedback, I appreciate it!

After uncommenting rimraf.sync(rootConfig.client.jspm.coverage.tempDirectory) in reporter.js, I am able to get a coverage report output.

You mean it works after you comment that line out? (As oppose to uncommenting, since it should already be uncommented.) This is the line you are referring to, right?

And I think you are right about this error being thrown only when coverage is disabled, this property would likely not be set if the coverage preprocessor hasn't ran.

[cyrixmorten]

Yeah, I meant that i commented it out :-)

Though I might have drawn an early conclusion on to what extend it was working.. I got the coverage console output, but not the files written to coverage folder.

I realised that I had forgotten to add jspm to preprocessors explaining the lack of output files.

This however unfortunately spawned a new wall of errors in the line of:

13 09 2016 16:37:20.449:ERROR [preprocessor.coverage]: { [Error: Line 1: Unexpected token )] lineNumber: 1, description: 'Unexpected token )', index: 17 }
Error: Line 1: Unexpected token )
    at constructError (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/esprima/esprima.js:2407:21)
    at createError (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/esprima/esprima.js:2426:17)
    at unexpectedTokenError (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/esprima/esprima.js:2500:13)
    at throwUnexpectedToken (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/esprima/esprima.js:2505:15)
    at consumeSemicolon (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/esprima/esprima.js:2620:13)
    at parseExpressionStatement (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/esprima/esprima.js:4223:9)
    at parseStatement (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/esprima/esprima.js:4760:24)
    at parseStatementListItem (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/esprima/esprima.js:3989:16)
    at parseScriptBody (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/esprima/esprima.js:5490:25)
    at parseProgram (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/esprima/esprima.js:5506:16)
    at Object.parse (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/esprima/esprima.js:5690:23)
    at Object.Instrumenter.instrumentSync (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/istanbul/lib/instrumenter.js:459:31)
    at SystemJSNodeLoader.CoveragePreprocessor.systemJS.instantiate (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/karma-jspm/src/preprocessor.js:97:66)
    at /Users/cyrixmorten/Development/Schneider/designer/client/node_modules/jspm/node_modules/systemjs/dist/system.src.js:432:33
    at tryCatchReject (file:///Users/cyrixmorten/Development/Schneider/designer/target-test/client/jspm_packages/system-polyfills.src.js:1188:30)
    at runContinuation1 (file:///Users/cyrixmorten/Development/Schneider/designer/target-test/client/jspm_packages/system-polyfills.src.js:1147:4)
    at Fulfilled.when (file:///Users/cyrixmorten/Development/Schneider/designer/target-test/client/jspm_packages/system-polyfills.src.js:935:4)
    at Pending.run (file:///Users/cyrixmorten/Development/Schneider/designer/target-test/client/jspm_packages/system-polyfills.src.js:826:13)
    at Scheduler._drain (file:///Users/cyrixmorten/Development/Schneider/designer/target-test/client/jspm_packages/system-polyfills.src.js:102:19)
    at Scheduler.drain (file:///Users/cyrixmorten/Development/Schneider/designer/target-test/client/jspm_packages/system-polyfills.src.js:67:9)
    at nextTickCallbackWith0Args (node.js:453:9)
    at process._tickCallback (node.js:382:13)

and

Potentially unhandled rejection [748] TypeError: Cannot read property 'src' of null
    at addToError (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/jspm/node_modules/systemjs/dist/system.src.js:111:80)
    at linkSetFailed (/Users/cyrixmorten/Development/Schneider/designer/client/node_modules/jspm/node_modules/systemjs/dist/system.src.js:700:21)
    at /Users/cyrixmorten/Development/Schneider/designer/client/node_modules/jspm/node_modules/systemjs/dist/system.src.js:500:9
    at tryCatchReject (file:///Users/cyrixmorten/Development/Schneider/designer/target-test/client/jspm_packages/system-polyfills.src.js:1188:30)
    at runContinuation1 (file:///Users/cyrixmorten/Development/Schneider/designer/target-test/client/jspm_packages/system-polyfills.src.js:1147:4)
    at Rejected.when (file:///Users/cyrixmorten/Development/Schneider/designer/target-test/client/jspm_packages/system-polyfills.src.js:968:4)
    at Pending.run (file:///Users/cyrixmorten/Development/Schneider/designer/target-test/client/jspm_packages/system-polyfills.src.js:826:13)
    at Scheduler._drain (file:///Users/cyrixmorten/Development/Schneider/designer/target-test/client/jspm_packages/system-polyfills.src.js:102:19)
    at Scheduler.drain (file:///Users/cyrixmorten/Development/Schneider/designer/target-test/client/jspm_packages/system-polyfills.src.js:67:9)
    at nextTickCallbackWith0Args (node.js:453:9)
    at process._tickCallback (node.js:382:13)

To be fair it is a fairly big project that I try to test and I am new to the whole world of jspm, so may very well be a configuration problem on my end.

Had it working with our previous setup that was based on npm + browserify.

[cyrixmorten]

Eureka!! :-D

Think the problems were entirely on my end. My final discovery was that I had failed to remove all traces of karma-coverage.

After removing it from plugins and reporters, I successfully got a full coverage report with files and everything 👍

As mentioned, we conditionally enable coverage, which now looks like this:

function setupCoverage() {
    if (!options.coverage) {
        console.log(' * coverage: disabled');
        return;
    }
    console.log(' * coverage: enabled');

    // coverage reporter generates the coverage
    defaultConfig.reporters = [ 'progress', 'dots', 'junit', 'jspm'];

    // source files, that you wanna generate coverage for
    // do not include tests or libraries
    // (these files will be instrumented by Istanbul)
    var src = path.join(target.srcFolder, '/**/!(*spec).js');

    defaultConfig.preprocessors[src] = ['jspm'];

    defaultConfig.junitReporter = {
        outputFile : path.join(target.coverageFolder, 'client/unit.xml'),
        suite : ''
    };

    // optionally, configure the reporter 
    defaultConfig.coverageReporter = {
        dir: path.join(target.coverageFolder),
        reporters : [
            { type: 'cobertura', subdir: '.', file: 'cobertura-coverage.xml' }, // (xml format supported by Jenkins)
            { type: 'html', subdir: '.' },
            { type: 'text'},
            { type: 'text-summary'}
        ]
    };
}

With frameworks and plugins as follows:

    frameworks : ['jspm', 'mocha', 'chai-as-promised', 'sinon-chai'],
    plugins: [
        'karma-jspm',
        'karma-chrome-launcher',
        'karma-mocha',
        'karma-chai-as-promised',
        'karma-sinon-chai',
        'karma-junit-reporter',
        'karma-sourcemap-loader',
        'karma-ng-html2js-preprocessor'
    ],

[m-a-r-c-e-l-i-n-o]

@cyrixmorten Yay! Glad you resolved it -- it did look like a configuration issue. So that first issue "TypeError: Cannot read property 'tempDirectory' of undefined" is fairly straight forward to address, and I'll see if I can do it today or tomorrow.

[cyrixmorten]

@m-a-r-c-e-l-i-n-o Yeah, well actually I did not need to alter anything in karma-jspm after the final correction.

It may however be a nice gesture to throw an error if there are karma-coverage references in the configuration since it apparently does not play well together with this library.

Our code is by the way an angular project of ~4k loc with 107 unit/midway tests.

Just to note that it is clearly capable of handling coverage of more than trivial examples.

[m-a-r-c-e-l-i-n-o]

See https://github.com/Workiva/karma-jspm/pull/178 for a simpler, likely more efficient solution.