fix: emit error and prevent re-test

[joshwiens]

Please check if the PR fulfills these requirements

• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

Currently karma-webpack is swallowing fatal compilation errors which has a substantial negative effect on the development experience.

What is the new behavior?

• karma-webpack: compiler.plugin will throw on any fatal compilation error.
• karma-webpack: compiler.plugin will prevent re-running tests on the above condition.
• karma-webpack: addEntry will not propagate errors for failed module imports.

Does this PR introduce a breaking change?

• Yes
• No

If this PR contains a breaking change, please describe the following...

• Impact:
• Migration path for existing applications:
• Github Issue(s) this is regarding:

This needs to be tested with @angular/cli before it's merged.

Relates to: #49
Relates to: #66

[bebraw]

As long as stats.compilation is defined, this is ok. Better with tests but you might have a plan for that. 👍

[joshwiens]

Long story in regards to the tests. Now that karma-webpack is maintained by @webpack-contrib/org-maintainers this is going to all get sorted out in short order with the defaults upgrade.

[joshwiens]

@filipesilva - This should cover both of the plugins currently needed by @angular/cli. I'll get it merged & published after a few tests in one of my cli based apps.

[filipesilva]

Awesome, thanks so much for taking on this issue! I'll integrate it into the CLI after it's merged and released.

[filipesilva]

Let me test this on the CLI as well, as per your original comment.

[filipesilva]

This is the original commit where I used variations of the plugins in #49 and #66 to fix the error behaviour: angular/angular-cli@8ab3954

The two cases we test for in the CLI are:

• on watch mode, rebuilds with errors will not rerun tests and instead wait to recompile
• on single run mode, builds with errors do not hang and exit with an error code

I tried the changes on this PR by applying them over the released version 2.0.3 and removing the custom plugins I had added:

• on watch mode, rebuilds with errors do not rerun tests and exit the process with an error
• on single run mode, builds with errors do not hang and exit with an error code

So I would say that the watch mode change is not ideal, since it requires users to re-run Karma on every error. The single-run mode is what we did though.

[joshwiens]

Give me 30 minutes & i'll fix the watch mode annoyance and properly cover those use cases.

watch mode change is not ideal ... it's down right obnoxious.

[joshwiens]

@filipesilva - Got dragged away. After mulling this over for a while and in the spirit of not breaking apps that depend on karma-webpack or everyones CLI apps ( to include my own ) the right answer here imo is to just make this respect the --bail option like it should.

• Wouldn't break the litany of apps that have plugins applied to handle the bail problem.
• Change would be benign for the CLI on release ( given the version range in the cli )
• Would require a small PR into @angular/cli to update https://github.com/angular/angular-cli/blob/master/packages/%40angular/cli/plugins/karma.ts#L108 to just apply bail: true in single run mode and leave watch mode unaffected.

Assuming you don't mind a small PR into the CLI, i'll go the route above.

[filipesilva]

@d3viant0ne apologies for taking so long to get back to your message. I agree with the approach you've outlined, it's the best option overall.

I was actually worried what direction it might take because of CLI version range, thank you for taking that into consideration 👍

[the-spyke]

Any news? Just spent almost an hour investigating a strange error in Karma. The root cause was in missing package in node_modules.