Activate Git action that enforces test coverage #786
Conversation
|
Warning Rate limit exceeded@lifi-action-bot has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 29 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughA new GitHub Actions workflow named Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .github/workflows/enforceTestCoverage.yml (1 hunks)
- .github/workflows_deactivated/enforceTestCoverage.yml (2 hunks)
Additional context used
actionlint
.github/workflows/enforceTestCoverage.yml
59-59: shellcheck reported issue in this script: SC2086:info:58:8: Double quote to prevent globbing and word splitting
(shellcheck)
59-59: shellcheck reported issue in this script: SC2129:style:69:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
59-59: shellcheck reported issue in this script: SC2086:info:69:54: Double quote to prevent globbing and word splitting
(shellcheck)
59-59: shellcheck reported issue in this script: SC2086:info:70:62: Double quote to prevent globbing and word splitting
(shellcheck)
59-59: shellcheck reported issue in this script: SC2086:info:71:58: Double quote to prevent globbing and word splitting
(shellcheck)
59-59: shellcheck reported issue in this script: SC2086:info:72:58: Double quote to prevent globbing and word splitting
(shellcheck)
Additional comments not posted (12)
.github/workflows_deactivated/enforceTestCoverage.yml (7)
1-1: LGTM!The workflow name change accurately reflects the new focus on enforcing a minimum test coverage threshold.
8-9: LGTM!The workflow trigger change to respond to pull request events aligns the coverage enforcement with the pull request lifecycle, which is a good practice.
12-12: LGTM!The job name change accurately reflects the new emphasis on enforcing minimum coverage requirements.
14-15: LGTM!The conditional statement to run the job only when the pull request is not in a draft state enhances the workflow's efficiency by preventing unnecessary checks on draft pull requests.
28-28: LGTM!The minimum test coverage percentage change to 75% with a plan to gradually increase it to 100% by the end of 2024 reflects a strategic decision to lower the immediate threshold while still committing to future improvements.
37-38: Verify the reason for the change in package manager.The dependency installation step has been changed from using npm to yarn. Please verify the reason for this change and ensure that it aligns with the project's package management preferences.
41-43: LGTM!The Foundry toolchain version update to v1.2.0 suggests an upgrade to leverage new features or improvements.
.github/workflows/enforceTestCoverage.yml (5)
1-10: LGTM!The workflow name and trigger events are appropriate for the purpose of enforcing test coverage on pull requests.
11-28: LGTM!The job configuration looks good:
- The job name is appropriate.
- It runs on the latest Ubuntu.
- It only runs when the PR is ready for review.
- The permissions are set appropriately.
- The required environment variables are set.
29-46: LGTM!The checkout and setup steps look good:
- Using the latest versions of the checkout and setup-node actions.
- Installing dev dependencies using Yarn.
- Installing the nightly version of Foundry using the latest version of the foundry-toolchain action.
- Installing Foundry dependencies using
forge install.
48-56: LGTM!The coverage report generation step looks good:
- Generates the coverage report in the standard lcov format using
forge coverage.- Filters the coverage report using a TypeScript script to only include relevant folders.
- Saves the filtered report to
lcov-filtered.info.
133-141: LGTM!The step that comments the coverage summary on the pull request looks good:
- Uses the
mshick/add-pr-comment@v2action to add a comment.- Includes the coverage summary stored in the GitHub environment variables in the comment.
Test Coverage ReportLine Coverage: 74.43% (1575 / 2116 lines) |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/enforceTestCoverage.yml (1 hunks)
Additional context used
actionlint
.github/workflows/enforceTestCoverage.yml
58-58: shellcheck reported issue in this script: SC2086:info:58:8: Double quote to prevent globbing and word splitting
(shellcheck)
58-58: shellcheck reported issue in this script: SC2129:style:69:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
58-58: shellcheck reported issue in this script: SC2086:info:69:54: Double quote to prevent globbing and word splitting
(shellcheck)
58-58: shellcheck reported issue in this script: SC2086:info:70:62: Double quote to prevent globbing and word splitting
(shellcheck)
58-58: shellcheck reported issue in this script: SC2086:info:71:58: Double quote to prevent globbing and word splitting
(shellcheck)
58-58: shellcheck reported issue in this script: SC2086:info:72:58: Double quote to prevent globbing and word splitting
(shellcheck)
Additional comments not posted (7)
.github/workflows/enforceTestCoverage.yml (7)
1-6: The workflow name and description look good!The workflow name clearly conveys the purpose of enforcing a minimum test coverage threshold. The description provides additional context, including the current coverage threshold, the plan to increase it over time, and the focus on 'lines' coverage.
7-15: The workflow trigger conditions are set up correctly.The workflow is triggered on the appropriate pull request events (opened, synchronized, reopened) and only runs when the pull request is not in draft state. This ensures that the test coverage check is performed at the right time during the code review process.
17-27: The workflow permissions and environment variables are properly set.The required permissions for the workflow are correctly specified. The sensitive information (Ethereum node URIs) is securely stored using secrets and accessed through environment variables. The minimum test coverage threshold is also defined as an environment variable, allowing for easy adjustment if needed.
28-45: The setup steps in the workflow are comprehensive and well-defined.The workflow properly sets up the environment by checking out the repository, installing Node.js, Foundry, and the required dependencies. Using specific versions for Node.js and Foundry ensures consistency across runs. The setup steps are complete and in the correct order.
47-55: The coverage report generation step is well-structured and includes the necessary commands.The
forge coveragecommand is used with the appropriate flags to generate the lcov report. The generated report is then filtered using a TypeScript script to include only the relevant source code from the 'src/' folder, excluding the 'test/' and 'script/' folders. This filtering step ensures that the coverage metrics are based on the actual production code.
57-130: The coverage summary generation step looks good, but there are a few issues reported by actionlint that should be addressed.The overall logic of the step is correct:
- Reads the filtered lcov file line by line.
- Extracts the total lines, functions, and branches found and hit using regex matching.
- Calculates the coverage percentages for lines, functions, and branches using bc with high precision.
- Formats the coverage percentages with two decimal places and alignment.
- Checks the line coverage percentage against the minimum threshold.
- Outputs the coverage summary and result.
- Stores the coverage reports in GitHub environment variables.
However, actionlint reported the following issues in the shell script:
- SC2086: Double quote to prevent globbing and word splitting.
- SC2129: Consider using { cmd1; cmd2; } >> file instead of individual redirects.
To fix these issues, apply the following changes:
- echo "LINE_COVERAGE_REPORT=$LINE_COVERAGE_REPORT" >> $GITHUB_ENV - echo "FUNCTION_COVERAGE_REPORT=$FUNCTION_COVERAGE_REPORT" >> $GITHUB_ENV - echo "BRANCH_COVERAGE_REPORT=$BRANCH_COVERAGE_REPORT" >> $GITHUB_ENV - echo "RESULT_COVERAGE_REPORT=$RESULT_COVERAGE_REPORT" >> $GITHUB_ENV + { + echo "LINE_COVERAGE_REPORT=$LINE_COVERAGE_REPORT" + echo "FUNCTION_COVERAGE_REPORT=$FUNCTION_COVERAGE_REPORT" + echo "BRANCH_COVERAGE_REPORT=$BRANCH_COVERAGE_REPORT" + echo "RESULT_COVERAGE_REPORT=$RESULT_COVERAGE_REPORT" + } >> "$GITHUB_ENV"Tools
actionlint
58-58: shellcheck reported issue in this script: SC2086:info:58:8: Double quote to prevent globbing and word splitting
(shellcheck)
58-58: shellcheck reported issue in this script: SC2129:style:69:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
58-58: shellcheck reported issue in this script: SC2086:info:69:54: Double quote to prevent globbing and word splitting
(shellcheck)
58-58: shellcheck reported issue in this script: SC2086:info:70:62: Double quote to prevent globbing and word splitting
(shellcheck)
58-58: shellcheck reported issue in this script: SC2086:info:71:58: Double quote to prevent globbing and word splitting
(shellcheck)
58-58: shellcheck reported issue in this script: SC2086:info:72:58: Double quote to prevent globbing and word splitting
(shellcheck)
132-141: The step for commenting the coverage summary in the pull request is properly configured.The
mshick/add-pr-commentaction is used correctly to add a comment to the pull request with the coverage summary. The authentication is securely handled using a personal access token stored as a secret. The message of the comment includes the relevant coverage reports stored in the GitHub environment variables, providing a clear overview of the test coverage results.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/enforceTestCoverage.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/enforceTestCoverage.yml
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/enforceTestCoverage.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/enforceTestCoverage.yml
Which Jira task belongs to this PR?
Why did I implement it this way?
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)