-
-
Notifications
You must be signed in to change notification settings - Fork 5
setup(coverage): add per-package test coverage reporting via codecov #82
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
Adds per-package test coverage reporting via Codecov by updating the CI workflow and introducing a Codecov configuration.
- Introduces a new CI step to upload coverage reports to Codecov.
- Adds
.github/codecov.yml
to define coverage thresholds, flags, and comment behavior.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
.github/workflows/ci.yml | Added “Upload coverage” step using codecov/codecov-action |
.github/codecov.yml | New Codecov config with per-package flags and status settings |
with: | ||
directory: ./recipes | ||
files: ./coverage.lcov |
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.
with: | |
directory: ./recipes | |
files: ./coverage.lcov | |
with: | |
token: ${{ secrets.CODECOV_TOKEN }} | |
directory: ./recipes | |
files: ./coverage.lcov |
you I'll need to add token in the repo secret (action) |
can we have badge for each recipes ? |
This is blocked by node's handling of the CLI → root pjson → recipe pjson In order for This could be fixed in node with either:
|
other solution (done on |
That's what this currently does, just via |
oh right |
"start": "node --no-warnings --experimental-import-meta-resolve --experimental-strip-types ./src/workflow.ts", | ||
"test": "node --no-warnings --experimental-import-meta-resolve --experimental-test-module-mocks --experimental-test-snapshots --experimental-strip-types --import='../../test/snapshots.ts' --test --experimental-test-coverage --test-coverage-include='src/**/*' --test-coverage-exclude='**/*.test.ts' './**/*.test.ts'" | ||
"start": "node --no-warnings --experimental-import-meta-resolve ./src/workflow.ts", | ||
"test": "node --no-warnings --experimental-import-meta-resolve --experimental-test-module-mocks --experimental-test-snapshots --import='../../test/snapshots.ts' --test-coverage-include='src/**/*' --test-coverage-exclude='**/*.test.ts' --test './**/*.test.ts'" |
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.
"test": "node --no-warnings --experimental-import-meta-resolve --experimental-test-module-mocks --experimental-test-snapshots --import='../../test/snapshots.ts' --test-coverage-include='src/**/*' --test-coverage-exclude='**/*.test.ts' --test './**/*.test.ts'" | |
"test": "node --no-warnings --experimental-import-meta-resolve --experimental-test-module-mocks --experimental-test-snapshots --import='../../test/snapshots.ts' --test-reporter=spec --test-reporter-destination=stdout --test './**/*.test.ts'", | |
"test:coverage": "node --no-warnings --experimental-import-meta-resolve --experimental-test-module-mocks --experimental-test-snapshots --import='../../test/snapshots.ts' --test-reporter=lcov --test-reporter-destination=./coverage.lcov --test-reporter=spec --test-reporter-destination=stdout --test-coverage-include='src/**/*' --test-coverage-exclude='**/*.test.ts' --test './**/*.test.ts'" |
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.
and run test:coverage
on CI
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.
Yeah, we could brute-force it like this. But that doesn't allow recipe authors to set extra flags / enable other features they may need.
I think better to just wait a couple weeks for the config options PR to land.
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.
both can work. I like to "hard-code" lcov output (I had done that on all of my packages) but other way can work
Resolves #80