-
Notifications
You must be signed in to change notification settings - Fork 50
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
Another approach for code coverage measurement #178
base: master
Are you sure you want to change the base?
Conversation
…strumentation for systemjs/jspm/transpiler projects, provide new reporter using remap-istanbul and modified adapter for attaching instrumentation
RavenNumber of Findings: 0 |
No comments? What should be done to get this pull request merged into the master branch? |
@malaupa Thanks for attempting to provide a better approach! I did explore this route when I crafted the (#147) solution and had to drop it early on do to the lack of sourcemap support in Istanbul. Most of the drawbacks that you listed for my solution had to be done in order to enable correct file mappings on the final html coverage report. In your solution, does the original source files show up in the final html report? And does it do so with the correct mappings? |
@m-a-r-c-e-l-i-n-o Yes and yes. This was the goal. |
@malaupa fantastic.
I couldn't understand how you were generating the correct mapping using Istanbul directly, but looking at the code now, it appears that you are still using my reporter file with @maxwellpeterson-wf @evanweible-wf This solution does indeed seem more efficient than mine, I think that scratching my pr in favor of this one, might be a good choice. |
Attempted to use this PR but unfortunately so far no luck getting coverage. The tests run just fine but the coverage report is empty.
Relevant parts of konf:
|
@cyrixmorten I added reporters: |
Tried removing the preprocessors, updating libraries, switching to jasmine instead of mocha and none of which seems to make the coverage report output come up. Wonder if I am missing a step, but can't seem to figure it out. Also dived into Here is my jspm conf:
|
@cyrixmorten Could you please start karma with |
Think that you are on to something there 👍 Even though I do have source mapping enabled they are not included in karma for some reason. Thank you for taking the time :) |
Just to clarify, by embedded source maps your mean inline using
Or separate
|
|
My thoughts as well, and I do see the base64 data inlined in the js files now but so far no luck on the report yet. However, as mentioned before, it may very well be a quirk somewhere in my setup. |
If I can support you, please ask. I have searched for a long time to get coverage with karma, jspm and babel. The result was to write this pr based on @m-a-r-c-e-l-i-n-o idea. Accordingly I want to share my solution and hopefully it will be merged. |
Finally got some time to look at this again. Following the previous comments, I have made sure that the inline sources are available when debugging the browser instantiated by karma. By adding Yet again I dived into the code of this PR to try and track down why there is no coverage output. I found that in
Hope you have an idea of why that is? :) If I print out
|
It was great to see an approach in #147 to measure code coverage while using jspm with karma. I searched very long for a transpiler agnostic and more simple attached reporter for karma. But the idea in #147 has drawbacks:
Thats why i create another implementation using istanbul directly while test running in the browser:
I think the code is simpler and there is no need for a preprocessor. Feedback is welcome!
The following karma config I used:
I have tested the code against a project with more than 450 tests in Chrome.
Sorry for the whitespace insertion. Was caused by my formatter.