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

Regression v2.0.1 -> v2.0.2: Exit code not set when thresholds aren't met #418

Closed
matz3 opened this issue Jul 21, 2020 · 7 comments
Closed
Labels

Comments

@matz3
Copy link

matz3 commented Jul 21, 2020

As already mentioned in my comment, there is a regression caused by #403 which prevents the exit code from being set in time, which causes the process not to fail in case the coverage threshold is not met.

I've created a small example to show-case the current behavior and also did some PoC on how it might be solved:
https://github.com/matz3/issue-karma-coverage-threshold-exit-code

Here are the changes to karma and karma-coverage

With v2.0.1 everything seems to work fine.

@alan-agius4
Copy link

alan-agius4 commented Jul 22, 2020

Just to make a note, currently we the Angular CLI team are trying the migrate and use this package and we also encountered this issue.

anthony-redFox added a commit to anthony-redFox/karma-coverage that referenced this issue Jul 22, 2020
…er#418

onComplete are async method which should bw complete in onExit method
@anthony-redFox
Copy link
Collaborator

anthony-redFox commented Jul 22, 2020

I created PR, please take a look. If you ok with fix, please let me know

anthony-redFox added a commit to anthony-redFox/karma-coverage that referenced this issue Jul 22, 2020
…er#418

onComplete are async method which should bw complete in onExit method
anthony-redFox added a commit to anthony-redFox/karma-coverage that referenced this issue Jul 22, 2020
…er#418

onComplete are async method which should bw complete in onExit method
@matz3
Copy link
Author

matz3 commented Jul 23, 2020

@anthony-redFox I could successfully validate your PR as well. But it still needs the 'exit' event change in karma. I've create a PR from my branch linked above: karma-runner/karma#3541

@matz3
Copy link
Author

matz3 commented Jul 23, 2020

@anthony-redFox I added some comments to your PR. I think our PRs both do pretty much the same thing.
My focus was to change as less code as necessary to wait for the promise on the exit event.

anthony-redFox added a commit that referenced this issue Jul 24, 2020
fix(report): waiting promise resolve in onExist method fix #418
karmarunnerbot pushed a commit that referenced this issue Jul 24, 2020
## [2.0.3](v2.0.2...v2.0.3) (2020-07-24)

### Bug Fixes

* **report:** waiting promise resolve in onExist method fix [#418](#418) ([c93f061](c93f061))
@karmarunnerbot
Copy link
Member

🎉 This issue has been resolved in version 2.0.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@matz3
Copy link
Author

matz3 commented Jul 28, 2020

@anthony-redFox thanks for doing a release. But I'm not sure whether this makes sense without karma-runner/karma#3541 being accepted/merged.

Would you mind re-opening this issue, as this actual problem for the consumer is not solved yet.

@matz3
Copy link
Author

matz3 commented Sep 1, 2020

Solved with karma v5.2.0 (in combination with karma-coverage v2.0.3)

openui5bot pushed a commit to SAP/openui5 that referenced this issue Sep 11, 2020
…rage

karma-coverage to solve regression with karma-coverage v2.0.2:
karma-runner/karma-coverage#418

Use new karma-ui5 option "failOnEmptyTestPage".

Exclude thirdparty code from coverage preprocessing.
We don't want to collect coverage scores from thirdparty code.
Also, with karma-coverage v2.0.2+ there is an issue with instrumenting
sinon-4.js, as it contains a very large inline source map that causes
a heap out of memory error in @babel/core.

Change-Id: I8b74ce6fc6b149d0acc49af2275aa84320346562
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants