Add Code Coverage Report #131
Conversation
WalkthroughThis pull request enhances the GitHub Actions workflow for the Dart project by implementing comprehensive coverage analysis and reporting. A new Changes
Sequence DiagramsequenceDiagram
participant Workflow as GitHub Actions
participant Test as Test Job
participant Coverage as Coverage Report Job
participant Docs as Deploy Docs Job
Test->>Test: Run tests with coverage
Test-->>Coverage: Trigger coverage processing
Coverage->>Coverage: Generate LCOV report
Coverage->>Coverage: Create coverage badge
Coverage-->>Docs: Provide coverage artifacts
Docs->>Docs: Deploy documentation with coverage
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
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
🧹 Nitpick comments (6)
.github/workflows/dart.yml (5)
99-106: Add quotes to prevent word splitting in shell commands.The artifact name construction could be improved to handle special characters safely.
- run: echo "ARTIFACT_NAME=coverage-$(echo '${{ matrix.test-path }}' | tr '/' '-')" >> $GITHUB_ENV + run: echo "ARTIFACT_NAME=coverage-$(echo "${{ matrix.test-path }}" | tr '/' '-')" >> $GITHUB_ENV🧰 Tools
🪛 actionlint (1.7.4)
100-100: shellcheck reported issue in this script: SC2086:info:1:81: Double quote to prevent globbing and word splitting
(shellcheck)
103-103: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
122-126: Use null-safe file handling in find command.The current find command might break with filenames containing spaces or special characters.
- find coverage-artifacts -name lcov.info -exec echo -a {} \; | xargs lcov -o coverage/lcov.info + find coverage-artifacts -name lcov.info -print0 | xargs -0 lcov -o coverage/lcov.info🧰 Tools
🪛 actionlint (1.7.4)
123-123: shellcheck reported issue in this script: SC2038:warning:2:1: Use 'find .. -print0 | xargs -0 ..' or 'find .. -exec .. +' to allow non-alphanumeric filenames
(shellcheck)
130-136: Add quotes to prevent word splitting in coverage calculation.The coverage calculation and badge generation could be more robust.
- COVERAGE=$(lcov --summary coverage/lcov.info | grep "lines......" | cut -d ' ' -f 4 | cut -d '%' -f 1) + COVERAGE="$(lcov --summary coverage/lcov.info | grep "lines......" | cut -d ' ' -f 4 | cut -d '%' -f 1)" - echo "{\"schemaVersion\": 1, \"label\": \"coverage\", \"message\": \"${COVERAGE}%\", \"color\": \"$(if [ $(echo "$COVERAGE >= 80" | bc -l) -eq 1 ]; then echo 'success'; elif [ $(echo "$COVERAGE >= 60" | bc -l) -eq 1 ]; then echo 'yellow'; else echo 'red'; fi)\"}" > badges/coverage.json + echo "{\"schemaVersion\": 1, \"label\": \"coverage\", \"message\": \"${COVERAGE}%\", \"color\": \"$(if [ "$(echo "$COVERAGE >= 80" | bc -l)" -eq 1 ]; then echo 'success'; elif [ "$(echo "$COVERAGE >= 60" | bc -l)" -eq 1 ]; then echo 'yellow'; else echo 'red'; fi)\"}" > badges/coverage.json🧰 Tools
🪛 actionlint (1.7.4)
131-131: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
131-131: shellcheck reported issue in this script: SC2046:warning:4:106: Quote this to prevent word splitting
(shellcheck)
131-131: shellcheck reported issue in this script: SC2046:warning:4:177: Quote this to prevent word splitting
(shellcheck)
156-156: Fix formatting issues.There are some minor formatting issues in the workflow file:
- Line 156: Remove trailing whitespace
- Line 198: Add newline at end of file
Also applies to: 198-198
🧰 Tools
🪛 yamllint (1.35.1)
[error] 156-156: trailing spaces
(trailing-spaces)
103-103: Update GitHub Actions to latest versions.Several actions are using older versions. Consider updating to the latest versions:
actions/upload-artifact@v3→@v4actions/checkout@v3→@v4actions/download-artifact@v3→@v4peaceiris/actions-gh-pages@v3→@v4Also applies to: 112-112, 115-115, 138-138, 153-153, 176-176, 191-191
🧰 Tools
🪛 actionlint (1.7.4)
103-103: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
README.md (1)
35-35: Add hyphens to compound words.For consistency, add hyphens to the compound term "object-oriented":
-"object oriented architecture" +"object-oriented architecture" -"object oriented approach" +"object-oriented approach"Also applies to: 102-102
🧰 Tools
🪛 LanguageTool
[misspelling] ~35-~35: This word is normally spelled with a hyphen.
Context: ...ruptions. Also, the WalletKit-C had an "object oriented architecture" that was poorly supported...(EN_COMPOUNDS_OBJECT_ORIENTED)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/dart.yml(2 hunks)README.md(5 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/dart.yml
100-100: shellcheck reported issue in this script: SC2086:info:1:81: Double quote to prevent globbing and word splitting
(shellcheck)
103-103: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
112-112: 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)
115-115: the runner of "actions/download-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
123-123: shellcheck reported issue in this script: SC2038:warning:2:1: Use 'find .. -print0 | xargs -0 ..' or 'find .. -exec .. +' to allow non-alphanumeric filenames
(shellcheck)
131-131: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
131-131: shellcheck reported issue in this script: SC2046:warning:4:106: Quote this to prevent word splitting
(shellcheck)
131-131: shellcheck reported issue in this script: SC2046:warning:4:177: Quote this to prevent word splitting
(shellcheck)
138-138: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
153-153: 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)
176-176: the runner of "actions/download-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
191-191: the runner of "peaceiris/actions-gh-pages@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/dart.yml
[error] 156-156: trailing spaces
(trailing-spaces)
[error] 198-198: no new line character at the end of file
(new-line-at-end-of-file)
🪛 LanguageTool
README.md
[misspelling] ~35-~35: This word is normally spelled with a hyphen.
Context: ...ruptions. Also, the WalletKit-C had an "object oriented architecture" that was poorly supported...
(EN_COMPOUNDS_OBJECT_ORIENTED)
[uncategorized] ~39-~39: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...er did not support modern null-safe Dart and it was difficult to expand for multiple...
(COMMA_COMPOUND_SENTENCE)
[misspelling] ~102-~102: This word is normally spelled with a hyphen.
Context: ...at sense, we deviate from a traditional object oriented approach. In other words, while we aim...
(EN_COMPOUNDS_OBJECT_ORIENTED)
🔇 Additional comments (2)
.github/workflows/dart.yml (1)
91-96: LGTM! Coverage collection properly integrated.The test job has been correctly modified to collect coverage data using Dart's built-in coverage tools.
README.md (1)
1-3: LGTM! Coverage badge properly added.The coverage badge is well-integrated with the existing badges and properly links to the coverage report.
| protoc -I. | ||
| --dart_out=grpc:/home/thomas/src/walletkit-dart/lib/src/crypto/tron/rpc | ||
| core/Tron.proto core/Discover.proto core/TronInventoryItems.proto | ||
| core/contract/common.proto core/contract/account_contract.proto | ||
| core/contract/asset_issue_contract.proto core/contract/balance_contract.proto | ||
| core/contract/exchange_contract.proto core/contract/market_contract.proto | ||
| core/contract/proposal_contract.proto core/contract/shield_contract.proto | ||
| core/contract/smart_contract.proto core/contract/storage_contract.proto | ||
| core/contract/witness_contract.proto api/api.proto |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove or properly document protoc commands.
The protoc commands at the end of the README appear to be development notes and include an absolute path. Consider either:
- Removing these commands if they're not meant for users
- Moving them to a development documentation file
- Converting them into a proper documentation section with context and relative paths
….io compatibility
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/dart.yml (1)
176-189: Add error handling for directory operations.Consider adding error checking for the directory operations.
- name: Prepare docs directory run: | + set -e # Exit on error rm -rf docs mkdir -p docs cp -R doc/api/* docs/ mkdir -p docs/coverage docs/badges cp -R coverage-report/* docs/coverage/ cp -R coverage/badges/* docs/badges/🧰 Tools
🪛 actionlint (1.7.4)
177-177: the runner of "actions/download-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/dart.yml(2 hunks)README.md(5 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/dart.yml
100-100: shellcheck reported issue in this script: SC2086:info:1:81: Double quote to prevent globbing and word splitting
(shellcheck)
103-103: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
112-112: 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)
115-115: the runner of "actions/download-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
123-123: shellcheck reported issue in this script: SC2038:warning:2:1: Use 'find .. -print0 | xargs -0 ..' or 'find .. -exec .. +' to allow non-alphanumeric filenames
(shellcheck)
131-131: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
131-131: shellcheck reported issue in this script: SC2046:warning:5:106: Quote this to prevent word splitting
(shellcheck)
131-131: shellcheck reported issue in this script: SC2046:warning:5:181: Quote this to prevent word splitting
(shellcheck)
139-139: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
154-154: 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)
177-177: the runner of "actions/download-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
192-192: the runner of "peaceiris/actions-gh-pages@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/dart.yml
[error] 157-157: trailing spaces
(trailing-spaces)
[error] 199-199: no new line character at the end of file
(new-line-at-end-of-file)
🪛 LanguageTool
README.md
[misspelling] ~35-~35: This word is normally spelled with a hyphen.
Context: ...ruptions. Also, the WalletKit-C had an "object oriented architecture" that was poorly supported...
(EN_COMPOUNDS_OBJECT_ORIENTED)
[uncategorized] ~39-~39: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...er did not support modern null-safe Dart and it was difficult to expand for multiple...
(COMMA_COMPOUND_SENTENCE)
[misspelling] ~102-~102: This word is normally spelled with a hyphen.
Context: ...at sense, we deviate from a traditional object oriented approach. In other words, while we aim...
(EN_COMPOUNDS_OBJECT_ORIENTED)
🔇 Additional comments (6)
.github/workflows/dart.yml (3)
91-96: LGTM! Coverage reporting is properly configured.The test job has been updated to generate coverage data using the Dart coverage package.
130-137: LGTM! Coverage badge configuration is well implemented.The coverage badge color logic (red < 60%, yellow 60-80%, green > 80%) provides good visual feedback.
🧰 Tools
🪛 actionlint (1.7.4)
131-131: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
131-131: shellcheck reported issue in this script: SC2046:warning:5:106: Quote this to prevent word splitting
(shellcheck)
131-131: shellcheck reported issue in this script: SC2046:warning:5:181: Quote this to prevent word splitting
(shellcheck)
144-151: LGTM! Job dependencies and permissions are correctly configured.The deploy job properly depends on the coverage-report job and has the necessary permissions.
README.md (3)
1-3: LGTM! Coverage badge is properly implemented.The coverage badge is well-integrated with the existing badges and links to the coverage report.
Line range hint
6-105: LGTM! Formatting changes improve readability.The text reformatting enhances readability while preserving the original content.
🧰 Tools
🪛 LanguageTool
[misspelling] ~35-~35: This word is normally spelled with a hyphen.
Context: ...ruptions. Also, the WalletKit-C had an "object oriented architecture" that was poorly supported...(EN_COMPOUNDS_OBJECT_ORIENTED)
[uncategorized] ~39-~39: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...er did not support modern null-safe Dart and it was difficult to expand for multiple...(COMMA_COMPOUND_SENTENCE)
146-154: Remove or properly document protoc commands.The protoc commands appear to be development notes and include an absolute path. Consider either:
- Removing these commands if they're not meant for users
- Moving them to a development documentation file
- Converting them into a proper documentation section with context and relative paths
| commit_message: "docs: update documentation and coverage" | ||
| enable_jekyll: false | ||
| full_commit_message: "docs: update API documentation" | ||
| full_commit_message: "docs: update API documentation and coverage report" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add newline at end of file.
Add a newline character at the end of the file to follow POSIX standards.
full_commit_message: "docs: update API documentation and coverage report"
+📝 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.
| commit_message: "docs: update documentation and coverage" | |
| enable_jekyll: false | |
| full_commit_message: "docs: update API documentation" | |
| full_commit_message: "docs: update API documentation and coverage report" | |
| commit_message: "docs: update documentation and coverage" | |
| enable_jekyll: false | |
| full_commit_message: "docs: update API documentation and coverage report" | |
🧰 Tools
🪛 yamllint (1.35.1)
[error] 199-199: no new line character at the end of file
(new-line-at-end-of-file)
| - name: Set artifact name | ||
| run: echo "ARTIFACT_NAME=coverage-$(echo '${{ matrix.test-path }}' | tr '/' '-')" >> $GITHUB_ENV | ||
|
|
||
| - name: Upload coverage artifacts | ||
| uses: actions/upload-artifact@v3 | ||
| with: | ||
| name: ${{ env.ARTIFACT_NAME }} | ||
| path: coverage/lcov.info |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix shell script quoting.
The artifact name construction should use quotes to prevent word splitting.
- run: echo "ARTIFACT_NAME=coverage-$(echo '${{ matrix.test-path }}' | tr '/' '-')" >> $GITHUB_ENV
+ run: echo "ARTIFACT_NAME=coverage-$(echo '${{ matrix.test-path }}' | tr '/' '-')" >> "$GITHUB_ENV"📝 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: Set artifact name | |
| run: echo "ARTIFACT_NAME=coverage-$(echo '${{ matrix.test-path }}' | tr '/' '-')" >> $GITHUB_ENV | |
| - name: Upload coverage artifacts | |
| uses: actions/upload-artifact@v3 | |
| with: | |
| name: ${{ env.ARTIFACT_NAME }} | |
| path: coverage/lcov.info | |
| - name: Set artifact name | |
| run: echo "ARTIFACT_NAME=coverage-$(echo '${{ matrix.test-path }}' | tr '/' '-')" >> "$GITHUB_ENV" | |
| - name: Upload coverage artifacts | |
| uses: actions/upload-artifact@v3 | |
| with: | |
| name: ${{ env.ARTIFACT_NAME }} | |
| path: coverage/lcov.info |
🧰 Tools
🪛 actionlint (1.7.4)
100-100: shellcheck reported issue in this script: SC2086:info:1:81: Double quote to prevent globbing and word splitting
(shellcheck)
103-103: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
| - name: Merge coverage reports | ||
| run: | | ||
| mkdir -p coverage | ||
| find coverage-artifacts -name lcov.info -exec echo -a {} \; | xargs lcov -o coverage/lcov.info | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use null-terminated file handling for safety.
The current find/xargs command may break with filenames containing spaces or special characters.
- find coverage-artifacts -name lcov.info -exec echo -a {} \; | xargs lcov -o coverage/lcov.info
+ find coverage-artifacts -name lcov.info -print0 | xargs -0 lcov -o coverage/lcov.info📝 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: Merge coverage reports | |
| run: | | |
| mkdir -p coverage | |
| find coverage-artifacts -name lcov.info -exec echo -a {} \; | xargs lcov -o coverage/lcov.info | |
| - name: Merge coverage reports | |
| run: | | |
| mkdir -p coverage | |
| find coverage-artifacts -name lcov.info -print0 | xargs -0 lcov -o coverage/lcov.info |
🧰 Tools
🪛 actionlint (1.7.4)
123-123: shellcheck reported issue in this script: SC2038:warning:2:1: Use 'find .. -print0 | xargs -0 ..' or 'find .. -exec .. +' to allow non-alphanumeric filenames
(shellcheck)
Summary by CodeRabbit
CI/CD Improvements
Documentation