feat: add github workflow for test coverage #1034
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Summary by CodeRabbit
WalkthroughThe changes introduce automated code coverage reporting for an Ember application. A new GitHub Actions workflow is added to run tests and post coverage reports on pull requests. The Ember build configuration is updated to include code coverage instrumentation using a Babel plugin compatible with Embroider. The Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub
participant GitHub Actions
participant Node.js/Ember App
participant lcov-reporter-action
Developer->>GitHub: Open Pull Request
GitHub->>GitHub Actions: Trigger coverage workflow
GitHub Actions->>Node.js/Ember App: Checkout & setup environment
GitHub Actions->>Node.js/Ember App: Install dependencies & run tests with coverage
Node.js/Ember App->>Node.js/Ember App: Collect coverage data
Node.js/Ember App->>GitHub Actions: Output lcov.info
GitHub Actions->>lcov-reporter-action: Post coverage report as PR comment
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying www-rds with
|
| Latest commit: |
8a4b7ff
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d992f4c8.www-rds.pages.dev |
| Branch Preview URL: | https://feat-add-test-coverage-bot.www-rds.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
.github/workflows/coverage.yml(1 hunks)ember-cli-build.js(1 hunks)package.json(2 hunks)tests/test-helper.js(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/coverage.yml
14-14: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
23-23: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
.github/workflows/coverage.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 5-5: too many spaces inside brackets
(brackets)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (5)
tests/test-helper.js (2)
8-11: Correct implementation of code coverage imports.The imports for
forceModulesToBeLoadedandsendCoverageare properly added from the test support module of ember-cli-code-coverage.
18-21: Well-implemented QUnit.done handler for coverage reporting.The async handler correctly ensures all modules are loaded and properly awaits the coverage data transmission after tests complete. This integration follows best practices for code coverage in Ember applications.
ember-cli-build.js (1)
15-21: Correct Babel configuration for Embroider-compatible coverage.The Babel plugin configuration correctly integrates ember-cli-code-coverage with the Embroider build system by using the
embroider: trueoption. This ensures proper instrumentation while maintaining compatibility with the modern build pipeline.package.json (2)
30-30: Appropriate test coverage script added.The
test:coveragescript correctly sets the COVERAGE environment variable to enable code coverage during test runs.
69-69: Correct addition of ember-cli-code-coverage dependency.Version ^3.1.0 is appropriate and uses the caret for compatible updates.
| uses: romeovs/[email protected] | ||
| with: | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| lcov-file: ./coverage/lcov.info |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding a badge to README.
The coverage report is posted as a comment, but you might want to also add a coverage badge to your README for visibility.
After this workflow is established, you could add the following to your README.md:
[](https://codecov.io/gh/Real-Dev-Squad/website-www)This requires setting up integration with a service like Codecov, which could be a future enhancement.
| - name: Run tests with coverage | ||
| run: pnpm run test:coverage || true |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider failing the workflow on low coverage.
The current implementation runs tests with coverage and continues even if tests fail (|| true). While this ensures the workflow completes and reports coverage, consider adding a step to enforce a minimum coverage threshold.
You could add a step after posting the coverage comment to check the coverage percentage and fail if it's below a threshold:
- name: Check coverage threshold
run: |
COVERAGE=$(cat ./coverage/lcov.info | grep -oP 'LF:\K[0-9]+' | awk '{covered+=$1} END {print covered}')
TOTAL=$(cat ./coverage/lcov.info | grep -oP 'LH:\K[0-9]+' | awk '{total+=$1} END {print total}')
PERCENTAGE=$(awk "BEGIN { print ($TOTAL/$COVERAGE) * 100 }")
echo "Current coverage: $PERCENTAGE%"
if (( $(echo "$PERCENTAGE < 70" | bc -l) )); then
echo "Coverage below threshold of 70%"
exit 1
fi| contents: read | ||
| pull-requests: write | ||
| steps: | ||
| - uses: actions/checkout@v3 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Update GitHub Actions checkout to latest version.
The runner for the checkout action is outdated.
- - uses: actions/checkout@v3
+ - uses: actions/checkout@v4📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v3 | |
| - uses: actions/checkout@v4 |
🧰 Tools
🪛 actionlint (1.7.4)
14-14: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
| on: | ||
| pull_request: | ||
| branches: [ main, master, develop ] |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
YAML syntax issues in workflow trigger configuration.
There are spacing issues in the bracket notation of the branches array.
on:
pull_request:
- branches: [ main, master, develop ]
+ branches: [main, master, develop]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| on: | |
| pull_request: | |
| branches: [ main, master, develop ] | |
| on: | |
| pull_request: | |
| branches: [main, master, develop] |
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 5-5: too many spaces inside brackets
(brackets)
| run_install: false | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v3 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Update GitHub Actions setup-node to latest version.
The runner for the setup-node action is outdated.
- - uses: actions/setup-node@v3
+ - uses: actions/setup-node@v4📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uses: actions/setup-node@v3 | |
| - uses: actions/setup-node@v4 |
🧰 Tools
🪛 actionlint (1.7.4)
23-23: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
| - name: Install dependencies | ||
| run: pnpm install |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider caching node_modules for faster workflows.
While pnpm caching is configured, you could further optimize by caching node_modules.
- name: Install dependencies
run: pnpm install
+
+ - name: Cache node_modules
+ uses: actions/cache@v4
+ with:
+ path: node_modules
+ key: ${{ runner.os }}-node-modules-${{ hashFiles('**/pnpm-lock.yaml') }}
+ restore-keys: |
+ ${{ runner.os }}-node-modules-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install dependencies | |
| run: pnpm install | |
| - name: Install dependencies | |
| run: pnpm install | |
| - name: Cache node_modules | |
| uses: actions/cache@v4 | |
| with: | |
| path: node_modules | |
| key: ${{ runner.os }}-node-modules-${{ hashFiles('**/pnpm-lock.yaml') }} | |
| restore-keys: | | |
| ${{ runner.os }}-node-modules- |
tejaskh3
force-pushed
the
feat-add-test-coverage-bot
branch
from
9bd14e3 to
8a4b7ff
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| run: pnpm install | ||
|
|
||
| - name: Run tests with coverage | ||
| run: pnpm run test:coverage || true |
There was a problem hiding this comment.
The || true suffix will make this step always exit with a success code, even when tests fail. This could hide test failures in the workflow. Consider either:
- Removing
|| trueto ensure test failures are properly reported - Using GitHub's
continue-on-error: trueat the step level for better error handling while still allowing the coverage reporting to proceed
This approach would maintain test integrity while still achieving the coverage reporting goal.
| run: pnpm run test:coverage || true | |
| run: pnpm run test:coverage | |
| continue-on-error: true |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Date:
change date hereDeveloper Name:
developer name hereIssue Ticket Number:-
#923Description:
Add description of the PR here
Is Under Feature Flag
Database changes
Breaking changes (If your feature is breaking/missing something please mention pending tickets)
Is Development Tested?
Tested in staging?
Add relevant Screenshot below ( e.g test coverage etc. )