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

electron-mocha and istanbul/isparta #19

Closed
jg123 opened this issue Oct 19, 2015 · 33 comments
Closed

electron-mocha and istanbul/isparta #19

jg123 opened this issue Oct 19, 2015 · 33 comments

Comments

@jg123
Copy link

jg123 commented Oct 19, 2015

Any ideas on how to get istanbul or isparta to generate a coverage report based on electron-mocha results. I run the following command and get the following error:

./node_modules/.bin/istanbul cover electron-mocha -- --compilers js:babel/register --recursive


  Test app menu creation
    ✓ should set isQuitMenu to true if quit is clicked


  1 passing (19ms)

No coverage information was collected, exit without writing coverage information
@jg123
Copy link
Author

jg123 commented Oct 19, 2015

Same goes for isparta:

./node_modules/.bin/babel-node ./node_modules/.bin/isparta cover electron-mocha -- --compilers js:babel/register --recursive


  Test app menu creation
    ✓ should set isQuitMenu to true if quit is clicked


  1 passing (20ms)

No coverage information was collected, exit without writing coverage information

@eugirdor
Copy link

I tried both as well and could not figure out how to get it working either. Does anyone know if it's possible or what would need to be done to get it to work?

@inukshuk
Copy link
Collaborator

I might be wrong, but I doubt that it can work this way, because istanbul hooks into Node's require; since electron-mocha forks out to electron the tests are never run as far a istanbul is concerned.

A simple solution that worked for me is to instrument your source code first; then run electron-mocha with a reporter that writes out the coverage report. This works very well, the only annoying thing is that you need to distinguish between your regular source files and the instrumented sources in your tests (I did this by using a custom load function which requires from either src or src-cov depending on an env variable). Other than that it's fine (works with babel/register too).

@orbitbot
Copy link

orbitbot commented Mar 7, 2016

@inukshuk I'm trying to set up a instrumented test run, but am still having problems with successfully generating coverage reports. Can you share your config?

@inukshuk
Copy link
Collaborator

inukshuk commented Mar 7, 2016

Take a look here -- it's pretty straight forward:

  • Run istanbul instrument
  • Run your tests on the instrumented code
  • Run istanbul report on the result

The one thing that is annoying is that you need to require the instrumented source files in your tests; in my case I want to be able to run the tests with or without coverage for speed, so I added a special require that is allows me to switch between the two directories. I also added a custom reporter because I wanted to get coverage and regular test output on Travis CI (otherwise you'd have to run the tests twice).

@orbitbot
Copy link

orbitbot commented Mar 7, 2016

@inukshuk Thanks for the reference! I'll have a look, but at a quick glance I have the feeling this is not going to turn out well... I'm trying to add coverage for non-CommonJS files to be tested in the renderer, and already had to hack electron-mocha to pass source files with globals.

@orbitbot
Copy link

orbitbot commented Mar 7, 2016

Actually, it seems like running the instrumented tests leaves a window.__coverage__ json file in the renderer process, so I managed to achieve what I set out to do by writing a callback in mocha's top-level after hook (ie.outside of any describe, in my case in a separate file) like so:

// hook into mocha global after to write coverage reports if found 
after(function() {
  if (window.__coverage__) {
    console.log('Found coverage report, writing to test/coverage/coverage.json');
    var path = require('path');
    var fs = require('fs');
    fs.writeFileSync(path.resolve(process.cwd(), 'test/coverage/coverage.json'), JSON.stringify(window.__coverage__));
  }
});

@inukshuk
Copy link
Collaborator

inukshuk commented Mar 7, 2016

Yes, that's basically what the reporter I linked above does, too (using istanbul's collector means I can configure the paths using istanbul's config file). You can then combine that data with coverage data from your main process and generate full coverage report.

@orbitbot
Copy link

orbitbot commented Mar 7, 2016

That sounds like a much better approach, I'll have to look at the Istanbul API in more detail. I got slightly confused with the shelljs syntax and didn't read through all the setup you linked to originally. Thanks for all the tips!

@jprichardson
Copy link
Owner

@inukshuk thanks for chiming in and helping :) Closing this. Please ping me if anyone thinks this should be reopened.

@rgbkrk
Copy link
Contributor

rgbkrk commented Apr 19, 2016

I wonder if there's a way to detect that istanbul is trying to run and automatically generate the coverage report for istanbul.

@ngprasad
Copy link

I could not understand what @inukshuk explained. Can someone please post some steps how I can make Istanbul work with electron-mocha? Thanks

@ngprasad
Copy link

ngprasad commented May 5, 2016

@jprichardson I feel this needs to be reopened. Clearly there has not been a resolution to this issue.

@jprichardson jprichardson reopened this May 5, 2016
@jprichardson
Copy link
Owner

jprichardson commented May 5, 2016

@jprichardson I feel this needs to be reopened. Clearly there has not been a resolution to this issue.

Ok, but it's up to @inukshuk to decide with what needs to happen.

@ngprasad
Copy link

ngprasad commented May 5, 2016

Thank you. @inukshuk Here's what I have tried:-

module.exports = function (gulp, plugins) { return function () { gulp.src('./test/unit/*.js') .pipe(plugins.electronMocha.default()) .pipe(plugins.istanbul.writeReports()); } }

I am using gulp-load-plugins to automatically load gulp plugins. But the problem is the coverage report I am getting says everything is 0% even though my tests are running. Can you please let me know what steps I am missing here? I saw your earlier reference about instrumenting the code which I tried but still not able to get it to work. Any help is appreciated. Thanks in advance.

@inukshuk
Copy link
Collaborator

inukshuk commented May 5, 2016

@ngprasad I can only repeat myself, you need to (1) instrument your code, (2) run your tests on the instrumented code, (3) write out the coverage stats (global.__coverage__) when your tests have finished, and (4) optionally combine your coverage reports from the main and and renderer processes.

Alternatively, you can look into how instanbul cover works internally, and write your own script that instruments code on the fly; by --requiring this you could effectively skip steps 1 and 2.

@orbitbot
Copy link

orbitbot commented May 5, 2016

Linked to my npm run based config here: #44 (comment)

If gulp config is an issue, you might want to change the steps to write out files instead of piping to the next transformation/processing to ensure everything works the way you assume.

@ngprasad
Copy link

ngprasad commented May 6, 2016

I was able to instrument my source code but I am not able to figure out how I can use this to run my tests in electron-mocha. I am piping my files from test folder onto electron-mocha. Where should I add the option to use instrumented code for coverage?

@inukshuk
Copy link
Collaborator

inukshuk commented May 6, 2016

@ngprasad it depends on your tests. Instead of loading your source code you need to load the instrumented source code.

If you're testing an Electron app you probably require the code you're testing in your test files; if you're testing a web app you're either doing the same thing or using a compiler / task runner and then load your bundled app with a --require or --preload option. Whatever you do, obviously, you need to load and run your tests against the instrumented code. So, if your source code is in the src folder and your instrumented code is in src-cov -- load the files from src-cov and coverage stats will be collected. This is step 2. Don't forget you also need to actually write out the coverage stats (step 3) -- there are plenty of examples above.

@ngprasad
Copy link

ngprasad commented May 6, 2016

Ok I ran my tests against the instrumented code by requireing them in my test module. After running my tests I logged global.__coverage__ but I am getting undefined and if I try to add it to the istanbul collector like collector.add(global.__coverage__) it gives me an error. Am I missing something here? Is there any other way of running tests on instrumented code? Please note I am still using electron-mocha as test runner. Should I use just mocha?

@inukshuk
Copy link
Collaborator

inukshuk commented May 9, 2016

@ngprasad if you can set up a minimal example that doesn't work I'll be happy to take a look.

@inukshuk
Copy link
Collaborator

inukshuk commented May 9, 2016

I looked a little bit into how istanbul cover works and put together a little POC to show that the basic approach discussed with @rgbkrk in the #44 thread works. Turns out it works fine, but of course it's complicated.

I was hoping to use the --compilers flag for this, but that won't work because setting up the require hooks is partly async and we currently have no way to wait for async requires or compilers (neither does mocha). So the easiest path I see to land this in electron-mocha is to add a --cover option which we handle before running mocha. I'd have the code for this almost ready, but this would be adding a feature to electron-mocha that's tied to istanbul and I'm not sure that's the point of this library.

The other option is to add an command line switch for async requires (and add the coverage code as a separate module or gist / example to the Readme); I think something like this was actually requested so it might be useful to some users... but I imagine this could be a deep Rabbit hole because people will expect it to be compatible various async module loaders out there.

What do you think? Is any of this worth the effort?

@rgbkrk
Copy link
Contributor

rgbkrk commented May 9, 2016

I think it's worth it. My typical on ramp for any project is test then ensure coverage. That means for an electron package I would start here followed by trying to figure out how to do coverage.

@jprichardson
Copy link
Owner

How does Mocha handle this problem?

@ngprasad
Copy link

I was finally able to resolve by running my unit tests on instrumented code as @inukshuk mentioned in the electron process itself. Otherwise the global.__coverge__ object will be undefined. It was really challenging for a beginner like me. It would really helpful if anyone could add istanbul support to this as it is a widely used code coverage reporter and it plays well with mocha too.

@inukshuk
Copy link
Collaborator

inukshuk commented May 11, 2016

@jprichardson Mocha does not have this specific problem, because usually you can run mocha via istanbul. i.e., something along the lines of istanbul cover _mocha -- istanbul hooks into require and instruments your sources on the fly (and writes the coverage report on exit). Even with regular node, istanbul and mocha you'd run into difficulties if you spawn several processes, so my original stance was that, given Electron's multi-process architecture, we just have to instrument your code separately -- no big deal.

However, as this thread demonstrates it would be a nice feature if we could offer a simpler setup and we have collectively figured out a viable approach that works just fine: we can basically mimic what istanbul cover does under the hood in the main or renderer thread, just before running the tests and writing the results after the tests have finished. This works fine, but I'd hesitate to add this to electron-mocha proper, because it would add istanbul as a dependency which is (a) not cool if you don't care about test coverage (b) will potentially add maintenance costs down the road. So it would make sense to just put it into a separate module (electron-mocha-istanbul unless anyone's feeling creative) and provide a 2 line setup for test coverage for those who want it. The problem with this approach, however, is that we'd need two things from electron-mocha / mocha:

  1. run async code and delay execution of the test suite and all --requires until the code has finished
  2. be notified when the tests have finished

The second one already works in the main thread because we can write the coverage report on process.exit; in the renderer process this could be done if we emit our mocha-done IPC event to window also (for instance).

The first one is more annoying though. Mocha has a --delay option which we could expose, too, and which I'm currently using for my POC, but this is not a best solution: it does not postpone --requires and it also seems to run the tests after a given timeout regardless of whether or not run was called (there are also a number of open issues for that flag). It is disappointing that Mocha does not handle --requires inside the Context, because that would make it possible to set up before and after hooks once (it would be silly to have to require your test coverage code in every test file!) -- as result there are lot's of requests in the Mocha issue tracker, there are some hacky solutions like mocha-prepare. If you read all the way through this thread it seems that the maintainers are coming around to agreeing that this is a useful feature, but my impression is that this will come only in case someone writes a proper plugin API.

P.S. happy Electron 1.0 everybody!

@inukshuk
Copy link
Collaborator

@ngprasad glad you figured it out!

@inukshuk
Copy link
Collaborator

I've re-visited this issue and I've figured out that it's actually possible to instrument the code in a synchronous way. This means that we can instrument the code before the tests run using a simple --require option. Furthermore, we can now reliably detect when the tests have finished, by listening for exit for main process tests and unload in the renderer.

This means that it is possible to add a require option which makes electron-mocha work as if it had been called via istanbul cover. If you're curious, see an example here.

It should be possible to turn this into a generic solution (separate npm module) which takes its configuration from .istanbul.yml. The only thing to keep in mind is that you'd probably run the tests twice (with and without --renderer) and then combine them afterwards with istanbul report.

@jprichardson
Copy link
Owner

Nice work @inukshuk!

dougluce added a commit to dougluce/scanner that referenced this issue Jul 8, 2016
Electron-mocha makes things like this a little difficult.  Luckily,
@inukshuk in
jprichardson/electron-mocha#19 (comment)
figures out a way to get around it. It's a bit of effort but does work
in the end.

With any luck an npm module will come out with these clues embedded in
it and the coverage.js can be removed.
dougluce added a commit to dougluce/scanner that referenced this issue Jul 8, 2016
Electron-mocha makes things like this a little difficult.  Luckily,
@inukshuk in
jprichardson/electron-mocha#19 (comment)
figures out a way to get around it. It's a bit of effort but does work
in the end.

With any luck an npm module will come out with these clues embedded in
it and the coverage.js can be removed.
@MarshallOfSound
Copy link
Contributor

For anyone interested in a fully ES6 code compatible coverage setup (without instrumenting the babelified files). Check this out.

MarshallOfSound/Google-Play-Music-Desktop-Player-UNOFFICIAL-@1b2055b

inukshuk added a commit that referenced this issue Jun 16, 2017
@pranavavva
Copy link

Sorry to poke this old thread, but the solution is to run istanbul instrument on the test files, then run mocha on the instumented files?

@inukshuk
Copy link
Collaborator

Basically, yes. Note that if you're using babel already you can now just use the excellent istanbul-plugin to instrument your code. If you do this, you really only need to write out the __coverage__ object after the tests have run. (I write them out for both renderer and main thread tests, then combine them using nyc report from the command line).

@bennycode
Copy link
Contributor

Thanks everyone for sharing your discoveries when adding code coverage to your Electron applications. It helped me to get code coverage for our app which is written in TypeScript. So if you need another example on how to set it up, then this PR might be of help: wireapp/wire-desktop#2215 💐

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

10 participants