Is there a way to get coverage reports reliably?

[priyank-p]

It looks like current coverage reports return by karma-coverage are weirdly minified and just have require('<the-test-file>'). So you can't really get coverage reports.

[twolfson]

I haven't done much coverage testing myself. Before I dive deep into this, can you provide a sample repo where coverage reports work in another launcher (e.g. Firefox, Chrome) and don't work in karma-electron? It would save me a bunch of time

[priyank-p]

@twolfson I haven't used karma-coverage for browsers yet! But sure I can put a small sample repo.

[priyank-p]

Here is the sample repo i put up on github to see the issue do:

git clone git@github.com:cPhost/coverage-sample-repo.git
npm i
npm test

The coverage html will be generated in coverage directory. The browser report would have correct report while electron one would be wrong.

[twolfson]

Awesome, thanks! That makes my job a whooole lot easier =D I'm busy tonight and uncertain about the weekend so I can promise getting started on this by the end of next week

Thanks again for the test repo =)

https://github.com/cPhost/coverage-sample-repo

[twolfson]

Looking into this now

[twolfson]

I've successfully reproduced the issue. It might be due to our lack of sourcemap support. Going to try messing with some settings to see how the coverage tool reacts

[twolfson]

This test repo saved me a buuuunch of headaches, thanks =D

[twolfson]

A little bit of progress, it looks like we set loadScriptsViaRequire: true. Unfortunately, that removes the would-be content from this file and apparently breaks coverage

However, the report still looks awful due to a lack of sourcemap telling it "hey, ignore this require section". I'm going to size up adding this in...

[twolfson]

tl;dr - Going to take a bit of break. Will try again by the end of next weekend

Okay, I've spent about an hour on this and have a general idea of what needs to be done:

• Warn users about lack of source map support when using loadScriptsViaRequire
• Create/extend source map for other scenario

I have something that should work but I'm not iterating easily with it. I think I need to step back, reapproach source maps with a fresh set of eyes on a proof of concept (hard to know if what I'm doing is right with all these layers of coverage and Electron). Will prob be best to find a source map inspector/similar and use that as template for what we're doing

Should be as simple as creating a file with only a few characters offset on first line (e.g. void 0), then creating a source map for said file, then verifying its source map result is what we want

Going to take a bit of break. Will try again by the end of next weekend

[priyank-p]

Thanks for detailed updates on this (and working on the issue, appreciate it).

I think the use case for this is mostly going to be for a library's or npm packages for electron apps rather than electron apps themselves since it tedious to test the whole app and know what parts you missed.

I did need any sort of coverage needs for send-feedback webcomponent since I had an idea of whats going on and what needed to be tested before each release -> tests. But when working for some utilities like the utils repo on electron-elements it had some complex logic which I need to know it is well tested before each release and is a lot of manual testing to remember.

Once again thanks for working on it, and karma-electron provides a great and easy way to test electron app compared to something like spectrum.

[twolfson]

Going to give this another shot. Starting with a proof of concept with browserify, then browserify + uglifyjs to see what we get in terms of sourcemaps

[twolfson]

Okay, going to talk out loud for a bit. Browserify generates a file for console.log('hi').js with the following content:

(function(){function r(e,n,t){function o(i,f){if(!n[i]){if(!e[i]){var c="function"==typeof require&&require;if(!f&&c)return c(i,!0);if(u)return u(i,!0);var a=new Error("Cannot find module '"+i+"'");throw a.code="MODULE_NOT_FOUND",a}var p=n[i]={exports:{}};e[i][0].call(p.exports,function(r){var n=e[i][1][r];return o(n||r)},p,p.exports,r,e,n,t)}return n[i].exports}for(var u="function"==typeof require&&require,i=0;i<t.length;i++)o(t[i]);return o}return r})()({1:[function(require,module,exports){
console.log('hi');

},{}]},{},[1])
//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIm5vZGVfbW9kdWxlcy9icm93c2VyaWZ5L25vZGVfbW9kdWxlcy9icm93c2VyLXBhY2svX3ByZWx1ZGUuanMiLCJoaS5qcyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQTtBQ0FBO0FBQ0EiLCJmaWxlIjoiZ2VuZXJhdGVkLmpzIiwic291cmNlUm9vdCI6IiIsInNvdXJjZXNDb250ZW50IjpbIihmdW5jdGlvbigpe2Z1bmN0aW9uIHIoZSxuLHQpe2Z1bmN0aW9uIG8oaSxmKXtpZighbltpXSl7aWYoIWVbaV0pe3ZhciBjPVwiZnVuY3Rpb25cIj09dHlwZW9mIHJlcXVpcmUmJnJlcXVpcmU7aWYoIWYmJmMpcmV0dXJuIGMoaSwhMCk7aWYodSlyZXR1cm4gdShpLCEwKTt2YXIgYT1uZXcgRXJyb3IoXCJDYW5ub3QgZmluZCBtb2R1bGUgJ1wiK2krXCInXCIpO3Rocm93IGEuY29kZT1cIk1PRFVMRV9OT1RfRk9VTkRcIixhfXZhciBwPW5baV09e2V4cG9ydHM6e319O2VbaV1bMF0uY2FsbChwLmV4cG9ydHMsZnVuY3Rpb24ocil7dmFyIG49ZVtpXVsxXVtyXTtyZXR1cm4gbyhufHxyKX0scCxwLmV4cG9ydHMscixlLG4sdCl9cmV0dXJuIG5baV0uZXhwb3J0c31mb3IodmFyIHU9XCJmdW5jdGlvblwiPT10eXBlb2YgcmVxdWlyZSYmcmVxdWlyZSxpPTA7aTx0Lmxlbmd0aDtpKyspbyh0W2ldKTtyZXR1cm4gb31yZXR1cm4gcn0pKCkiLCJjb25zb2xlLmxvZygnaGknKTtcbiJdfQ==

That comment at the end is base64 encoded JSON. Running it through base64 -d and formatting via Node.js gets us:

{ version: 3,
  sources: 
   [ 'node_modules/browserify/node_modules/browser-pack/_prelude.js',
     'hi.js' ],
  names: [],
  mappings: 'AAAA;ACAA;AACA',
  file: 'generated.js',
  sourceRoot: '',
  sourcesContent: 
   [ '(function(){function r(e,n,t){function o(i,f){if(!n[i]){if(!e[i]){var c="function"==typeof require&&require;if(!f&&c)return c(i,!0);if(u)return u(i,!0);var a=new Error("Cannot find module \'"+i+"\'");throw a.code="MODULE_NOT_FOUND",a}var p=n[i]={exports:{}};e[i][0].call(p.exports,function(r){var n=e[i][1][r];return o(n||r)},p,p.exports,r,e,n,t)}return n[i].exports}for(var u="function"==typeof require&&require,i=0;i<t.length;i++)o(t[i]);return o}return r})()',
     'console.log(\'hi\');\n' ] }

What we can see here:

• First line of the file is included as sourcesContent[0] pointing to an actual file:
  • Compiled version of https://github.com/browserify/browser-pack/blob/v6.1.0/prelude.js
• Second line of the file is included as sourcesContent[1]
• Not all of the content is accounted for (e.g. IIFE for ({1:[function(require,module,exports){)

I really want to figure out if there's a better way to handle my preprocessor aside from a Mustache template. I feel like there's a better way =(

[twolfson]

https://sokra.github.io/source-map-visualization/#custom is a sanity saver

UglifyJS doesn't seem to combine multiple source maps but browserify does support double encoding which is crazy

I think we should be able to leverage a library like:

• https://www.npmjs.com/package/source-list-map
• https://github.com/thlorenz/combine-source-map

[twolfson]

Great, I have a patch that's working. I'll likely need to fix up our tests too though

Ironically, during manual testing, I think I found out that we don't need this patch to get coverage working. The issue was ordering of preprocessors. Updating the Electron section to the following of the test repo gets everything working. Unfortunately, it's a bit greedy for its glob so I need to look to see if there's negation support for Karma globs (I think there is):

  if (process.env.BROWSER === 'Electron') {
    karmaConfig.preprocessors = {
      '**/*.js': ['coverage', 'electron']
    };
    karmaConfig.client = {
      useIframe: false,
      loadScriptsViaRequire: false
    };
  }

[twolfson]

Yep, there's negation support. Here's the easiest way to use this:

  if (process.env.BROWSER === 'Electron') {
    karmaConfig.preprocessors = {
      'lib/**/*.js': ['coverage', 'electron'],
      '!(lib)/**/*.js': ['electron']
    };
    karmaConfig.client = {
      useIframe: false,
      loadScriptsViaRequire: false
    };
  }

Here's a more complex but easier to maintain version:

    preprocessors: {
      'lib/**/*.js': ['coverage'],
      '!(lib)/**/*.js': []
    },
    // ...
  };

  if (process.env.BROWSER === 'Electron') {
    Object.keys(karmaConfig.preprocessors).forEach(function (preprocessorGlob) {
      karmaConfig.preprocessors[preprocessorGlob].push('electron');
    });
    karmaConfig.client = {
      useIframe: false,
      loadScriptsViaRequire: false
    };
  }

[twolfson]

I'm going to still land the sourcemap fix but this issue was determine to be isolated

[twolfson]

The sourcemap fix has been released in 6.0.0