-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
Is there a way to get coverage reports reliably? #30
Comments
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 |
@twolfson I haven't used |
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 |
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 =) |
Looking into this now |
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 |
This test repo saved me a buuuunch of headaches, thanks =D |
A little bit of progress, it looks like we set 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... |
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:
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. Going to take a bit of break. Will try again by the end of next weekend |
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. |
Going to give this another shot. Starting with a proof of concept with |
Okay, going to talk out loud for a bit. Browserify generates a file for
That comment at the end is base64 encoded JSON. Running it through
What we can see here:
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 =( |
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: |
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 if (process.env.BROWSER === 'Electron') {
karmaConfig.preprocessors = {
'**/*.js': ['coverage', 'electron']
};
karmaConfig.client = {
useIframe: false,
loadScriptsViaRequire: false
};
} |
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
};
} |
I'm going to still land the sourcemap fix but this issue was determine to be isolated |
The sourcemap fix has been released in |
It looks like current coverage reports return by
karma-coverage
are weirdly minified and just haverequire('<the-test-file>')
. So you can't really get coverage reports.The text was updated successfully, but these errors were encountered: