-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Include working coverage script #2997
Conversation
Are you gonna bundle babel with rollup? How big bundle with this change? |
Babel is definitely not bundled and it is only a transitive devDependency via istanbul-lib-instrument and nyc anway. There should be not change for users of Rollup, both in Rollup's install size and the size of generated bundles. We should just make sure that we do not accidentally publish the instrumented Rollup bundle. |
So to make it clear, this is only for Rollup's own coverage. If Rollup consumers need coverage, they need to set up something similar to what has been done in this PR. |
Ah, great! Thanks for answer |
This addresses #227. That issue is worth reviewing as there's some good info in it. |
Codecov Report
@@ Coverage Diff @@
## master #2997 +/- ##
=========================================
Coverage ? 87.51%
=========================================
Files ? 165
Lines ? 5718
Branches ? 1728
=========================================
Hits ? 5004
Misses ? 438
Partials ? 276 Continue to review full report at Codecov.
|
…cessary for nyc (and we could have had this much earlier...)
@shellscape Since this is working really well now, I'll put it on master to be able to have coverage diff with master for other PRs. In the end it turns out the special instrumentation plugin was not really necessary as nyc is working really well with sourcemaps now, all we needed were additional sourcemaps for bin/rollup and a proper include/exclude .nycrc file. We could consider adding a coverage limit for PRs in the future though for now I'm happy with this being a tool for reference to spot untested code paths. |
"ci:lint": "npm run lint:nofix && npm run security", | ||
"ci:test": "npm run build:test && npm run build:bootstrap && npm run test:all", | ||
"ci:test_12": "npm run build:test && npm run build:bootstrap && npm run test:only && npm run test:typescript", | ||
"ci:coverage": "npm run build:test && nyc --reporter lcovonly mocha && codecov", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(late to the party, but I'll submit a PR for this)
We can eliminate the codecov
dependency by adding this to the circle ci config on the last node version test:
- run:
name: Submit coverage data to codecov.
command: bash <(curl -s https://codecov.io/bash)
when: on_success
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Resolves #227
Description
This includes a script to run tests with coverage.
TODO: