Skip to content
This repository has been archived by the owner on Jan 19, 2021. It is now read-only.

Add option for including all sources #15

Open
Aaronius opened this issue Oct 9, 2015 · 11 comments
Open

Add option for including all sources #15

Aaronius opened this issue Oct 9, 2015 · 11 comments

Comments

@Aaronius
Copy link

Aaronius commented Oct 9, 2015

First off, I have little understanding of the interactions amongst karma, webpack, istanbul-instrumenter-loader, and karma-coverage. This may not be the appropriate project for my request and, if not, maybe you can point me in the right direction.

The end result I'm looking for is to be able to include sources files that are not required by tests into coverage report without making the suggested index.js file, specifically this part: componentsContext.keys().forEach(componentsContext);.

Why don't I want index.js to execute the modules? Well, it's a long story and a complicated one at that so I'll avoid telling it if possible. What it comes down to though is that the modules throw exceptions unless the tests inject certain things into them (currently we're using inject-loader for this). Because they throw exceptions in this case, no coverage report is rendered. If I don't use the example index.js, source files that are not required by tests are not included in the coverage report.

I see inklings of things that may make this possible. For example, I'm aware of the --include-all-sources command-line flag for istanbul but I'm not using the command-line. I'm aware of work on karma-coverage to allow for this as well as this interim workaround. My attempts at making them work in unison with webpack+istanbul-instrumenter-loader have failed. They make the coverage report, but the source files that are not required by tests still don't show up.

Any insight here? Thank you!

@jhicken
Copy link

jhicken commented Oct 9, 2015

Ditto

I also want to note that this would be helpful on https://github.com/deepsweet/isparta-loader.

@Aaronius
Copy link
Author

Aaronius commented Oct 9, 2015

FWIW, I can get around my particular situation by doing:

componentsContext.keys().forEach(function(component) {
  try {
    componentsContext(component);
  } catch (e) {}
});

It would still be great to have the ability to include all source files in the reporter without using the recommended index.js, however.

@nkbt
Copy link

nkbt commented Oct 10, 2015

It seems relevant to #3.
There is a potential (but not correct) solution #12.

We also use inject-loader. We ended up never including extra deps and always mock outer stuff. So we simply avoid this kind of exception because we never load same file twice with different require path.

@jhicken
Copy link

jhicken commented Oct 13, 2015

One thing I will note with the approach to put everything in an index.js and load it there. If you do this all of your files including you untested ones will have some testing instrumentation. It makes me a little bit sad. I know at some point I will be testing all the files however Its nice to see what files actually do not have any tests written for.

It seems like we should be able to pass in a separate glob or pattern to match your source files against. It even looks like istanbul should merge duplicate includes using a collector.

I don't know perhaps 2 loaders would work better for this. So a loader for source files and a loader for instrumentation. If we take that approach. Nothing needs to change with this loader.

I would honestly prefer it all to be in one.

Thoughts anybody?

@nkbt
Copy link

nkbt commented Oct 13, 2015

@jhicken For our project I've built a fairly simple script that collects all js files and creates a basic testing template (that includes module and checks if it is truthy). So we were able to get correct code coverage and at the same time manually decide which files should be removed as not that relevant. It doe not overwrite existing specs and only adds missing ones.

I've put it to the gist if you are fancy trying that way:

https://gist.github.com/nkbt/0d531edd99c1ced81a69

@jhicken
Copy link

jhicken commented Oct 13, 2015

Cool thanks @nkbt I'll give it a peak tomorrow.

@deepsweet
Copy link
Collaborator

#48

@getsnoopy
Copy link

getsnoopy commented Mar 22, 2017

We were able to get this working, albeit inefficiently. We end up capturing the initial coverage map for each file that passes through the loader where baseCoverageMap is a closure variable that maintains the cumulative base coverage map:

var istanbulLibInstrument = require('istanbul-lib-instrument');
var instrumenter = istanbulLibInstrument.createInstrumenter({
  esModules: true,
});
webpackConfig.module.preLoaders.push({
  test: /\.js$/,
  include: function(modulePath) {
    var source;
    var result = {};

    // Determine whether the module should be instrumented
    // Note: Instrument only application sources with Istanbul (we only want to measure how much
    //       of the application code was executed, not the unit test code as well).
    var shouldInstrument = true;

    // If yes, save its initial coverage map
    // Note: This is so that we have a "base" coverage map for all of the modules we instrument
    //       via Webpack. We need this base coverage map to account for all the modules'
    //       coverage, regardless of whether the modules are executed (by way of their
    //       corresponding unit tests), since Istanbul only reports coverage for modules
    //       that have been executed which gives us inaccurate coverage statistics.
    if (shouldInstrument) {
      // Read the module source
      source = fs.readFileSync(modulePath, { encoding: 'utf8' });

      // Pass it to the instrumenter to get the coverage map
      instrumenter.instrumentSync(source, modulePath);
      result[modulePath] = instrumenter.lastFileCoverage();

      // Merge the module's coverage map into the base coverage map
      Object.assign(baseCoverageMap, result);
    }

    return shouldInstrument;
  },
  loader: 'istanbul-instrumenter',
  query: {
    esModules: true,
  },
});

After the modules have been bundled and executed in the browser, we capture the coverage object from there and merge it into baseCoverageMap, and save that out to a coverage.json file that Istanbul then reads which results in the correct coverage statistics.

It's not elegant, and we haven't figured out a way to do all this within the bundle Webpack generates (ideally injects a module which is executed that maintains this baseCoverageMap), but it works.

@Skeeterdrums
Copy link

@getsnoopy do you have a hosted example of that solution? I'm not exactly sure what how you're storing or representing the baseCoverageMap variable.

@pingynator
Copy link

I would like to see an include-exclude-pattern option for files required by tests. So sourcecode gets instrumented automatically when tested, but can also be configured with include and exclude (f.e. exclude node_modules).

And I would like to see a second include-exclude-pattern option for files, which should be instrumented although they are not required by testfiles yet (for example to include certain node_modules).

@getsnoopy
Copy link

@Skeeterdrums Sorry for the (very) late response. I don't have a hosted example, but I can outline the basics. You'll notice we plugged into the include functionality of Webpack, so every file in a certain directory tree will go through that function, and will be instrumented. Since none of them will be executed at that point, all of their coverage maps will be zeroed out. We basically capture this in baseCoverageMap, and merge the actual coverage map generated later during the execution of the tests into it to get a "full" coverage map.

It's hacky, but it works. It's the only way we could think of doing it at the moment.

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

No branches or pull requests

7 participants