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

Ensure overridden reporter methods are called #18

Merged
merged 1 commit into from
Aug 1, 2016

Conversation

bryanforbes
Copy link
Contributor

This fixes the issue reported by @mattlewis92 in the comments in #17.

@delasteve delasteve merged commit bec708f into marcules:master Aug 1, 2016
@delasteve
Copy link
Contributor

Thanks, again, @bryanforbes!

@bryanforbes
Copy link
Contributor Author

No problem. I didn't realize that baseReporterDecorator added methods. Sorry about that!

@mattlewis92
Copy link
Contributor

@bryanforbes thanks for sorting it so quickly! 😄 I've nearly got it working now, the text summary works and the html files are created, but the files that display the individual coverage per file aren't, the error thrown is:

01 08 2016 20:56:14.931:ERROR [karma]: [Error: Unable to find entry for [src/components/calendarDayView.component.ts]]
Error: Unable to find entry for [src/components/calendarDayView.component.ts]
    at MemoryStore.Store.mix.get (/Users/mattlewis/Code/open-source/angular2-calendar/node_modules/istanbul/lib/store/memory.js:38:19)
    at HtmlReport.Report.mix.writeDetailPage (/Users/mattlewis/Code/open-source/angular2-calendar/node_modules/remap-istanbul/node_modules/istanbul/lib/report/html.js:411:67)
    at /Users/mattlewis/Code/open-source/angular2-calendar/node_modules/remap-istanbul/node_modules/istanbul/lib/report/html.js:489:26
    at SyncFileWriter.extend.writeFile (/Users/mattlewis/Code/open-source/angular2-calendar/node_modules/remap-istanbul/node_modules/istanbul/lib/util/file-writer.js:57:9)
    at FileWriter.extend.writeFile (/Users/mattlewis/Code/open-source/angular2-calendar/node_modules/remap-istanbul/node_modules/istanbul/lib/util/file-writer.js:147:23)
    at /Users/mattlewis/Code/open-source/angular2-calendar/node_modules/remap-istanbul/node_modules/istanbul/lib/report/html.js:488:24
    at Array.forEach (native)
    at HtmlReport.Report.mix.writeFiles (/Users/mattlewis/Code/open-source/angular2-calendar/node_modules/remap-istanbul/node_modules/istanbul/lib/report/html.js:482:23)
    at /Users/mattlewis/Code/open-source/angular2-calendar/node_modules/remap-istanbul/node_modules/istanbul/lib/report/html.js:484:22
    at Array.forEach (native)
    at HtmlReport.Report.mix.writeFiles (/Users/mattlewis/Code/open-source/angular2-calendar/node_modules/remap-istanbul/node_modules/istanbul/lib/report/html.js:482:23)
    at HtmlReport.Report.mix.writeReport (/Users/mattlewis/Code/open-source/angular2-calendar/node_modules/remap-istanbul/node_modules/istanbul/lib/report/html.js:566:14)
    at /Users/mattlewis/Code/open-source/angular2-calendar/node_modules/remap-istanbul/lib/writeReport.js:77:22
    at /Users/mattlewis/Code/open-source/angular2-calendar/node_modules/amdefine/amdefine.js:125:34
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)

I'm using webpack and the istanbul-instrument-loader to instrument the code, any ideas? This is the project I'm trying to run it on: https://github.com/mattlewis92/angular2-calendar/tree/upgrade-karma-remap-istanbul

@bryanforbes
Copy link
Contributor Author

@mattlewis92 I'm looking into this right now. Thanks.

@TheLarkInn
Copy link

@mattlewis92 I want to just focus on a new out of the box CLI app. This removes any possibility of edge cases. Could you please test that first. I don't want to have @bryanforbes running around in circles looking for edge cases etc. Especially since the was kind enough to work on this for me. ❤️❤️❤️

@TheLarkInn
Copy link

Oops sorry thus was meant for the CLI repo sorry!!!

@bryanforbes
Copy link
Contributor Author

@mattlewis92 There is currently an issue with istanbul-instrument-loader where it will not add the source map line to an instrumented file if one exists. Can you try using sourcemap-istanbul-instrumenter-loader as a drop-in replacement for istanbul-instrument-loader with the force-sourcemap option to see if this fixes the issue you're seeing?

@mattlewis92
Copy link
Contributor

@bryanforbes that did the trick, thanks so much for your assistance with this, I really appreciate your help 😃

I'll work on a PR later to add some tests to this repo + a full e2e example to help others in the future :octocat:

@EarthCitizen
Copy link

@bryanforbes I am getting the same type of error as @mattlewis92, but if I run remap-istanbul from the command line using the same coverage JSON file as what karma-remap-istanbul is trying to consume, the HTML report is generated just fine.

@mattlewis92
Copy link
Contributor

@EarthCitizen this is the config I'm using (for karma + webpack): mattlewis92/angular-calendar@574afc6

@EarthCitizen
Copy link

Related conversation in #21

@mattlewis92
Copy link
Contributor

The trick is to use the sourcemap-istanbul-instrumenter-loader instead of the normal Istanbul loader with the force-sourcemap option 😀

On 29 Aug 2016, at 22:09, EarthCitizen [email protected] wrote:

Related conversation in #21


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@EarthCitizen
Copy link

I tried that, and it does not fail, but HTML report is empty.

@mattlewis92
Copy link
Contributor

What's your tsconfig?

On 29 Aug 2016, at 22:17, EarthCitizen [email protected] wrote:

I tried that, and it does not fail, but HTML report is empty.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@EarthCitizen
Copy link

{
  "compilerOptions": {
    "target": "ES5",
    "module": "commonjs",
    "removeComments": true,
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "noEmitHelpers": false,
    "inlineSourceMap": true
  },

@EarthCitizen
Copy link

EarthCitizen commented Aug 29, 2016

@mattlewis92 Have a look at the results from my investigation in #21

It really seems to point to karma-remap-istanbul.

@mattlewis92
Copy link
Contributor

Replace the inlineSourceMap option with sourceMap and it should work

On 29 Aug 2016, at 22:32, EarthCitizen [email protected] wrote:

@mattlewis92 Have a look at the results from my investigation in #21


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

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

Successfully merging this pull request may close these issues.

5 participants