Update test.yml Github Action to report coverage to sonarcloud #8712
Conversation
…d unit test coverage to sonarcloud
WalkthroughThe changes introduce enhancements to the testing workflow in the GitHub Actions configuration. A new job, Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant SonarCloud
Developer->>GitHub Actions: Push code changes
GitHub Actions->>GitHub Actions: Run tests with go test -json
GitHub Actions->>GitHub Actions: Generate coverage report
GitHub Actions->>SonarCloud: Trigger sonarCloudTrigger job
SonarCloud-->>GitHub Actions: Analyze code quality
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
Outside diff range and nitpick comments (2)
.github/workflows/test.yml (2)
106-119: LGTM! Good addition of code coverage reporting.The changes to the
gojob look good. Here are the key improvements:
- Adding the
-jsonflag to thego testcommand (line 106) is beneficial for machine-readable output.- Creating a separate step for code coverage report (lines 108-112) is necessary as
-raceand-covermode=countare incompatible.- Archiving the code coverage results (lines 113-119) is crucial for the new SonarCloud integration.
Consider adding a comment explaining why we're running the tests twice (once with -race and once for coverage). This will help future maintainers understand the reasoning behind these steps.
Tools
actionlint
109-109: shellcheck reported issue in this script: SC2001:style:1:11: See if you can use ${variable//search/replace} instead
(shellcheck)
109-109: shellcheck reported issue in this script: SC2046:warning:1:16: Quote this to prevent word splitting
(shellcheck)
109-109: shellcheck reported issue in this script: SC2005:style:1:16: Useless echo? Instead of 'echo $(cmd)', just use 'cmd'
(shellcheck)
109-109: shellcheck reported issue in this script: SC2086:info:3:157: Double quote to prevent globbing and word splitting
(shellcheck)
109-112: Consider addressing shellcheck warnings for improved script robustness.While not critical, addressing the following shellcheck warnings could improve the script's readability and robustness:
- Use
${variable//search/replace}instead ofsedfor simple string replacements.- Quote the command substitution to prevent word splitting.
- Avoid useless
echoby directly using the command output.- Use double quotes to prevent globbing and word splitting.
Here's an improved version of the script addressing these issues:
- name: Code coverage report creation run: | VERSION=${GITHUB_REF#refs/tags/v} VERSION=${VERSION:-$GITHUB_SHA} TESTS=$(cat "pkgs.txt.part.${{ matrix.part }}") VERSION="$VERSION" SKIP_WASM_WSL_TESTS="false" go test -coverprofile=coverage.out -covermode=count -json -mod=readonly -tags='ledger test_ledger_mock norace' $TESTS > report.jsonThis version:
- Uses parameter expansion instead of
sed.- Removes the unnecessary
echo.- Properly quotes variables to prevent word splitting.
- Still uses
$TESTSunquoted as it's intended to be word-split for multiple test packages.Tools
actionlint
109-109: shellcheck reported issue in this script: SC2001:style:1:11: See if you can use ${variable//search/replace} instead
(shellcheck)
109-109: shellcheck reported issue in this script: SC2046:warning:1:16: Quote this to prevent word splitting
(shellcheck)
109-109: shellcheck reported issue in this script: SC2005:style:1:16: Useless echo? Instead of 'echo $(cmd)', just use 'cmd'
(shellcheck)
109-109: shellcheck reported issue in this script: SC2086:info:3:157: Double quote to prevent globbing and word splitting
(shellcheck)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/test.yml (2 hunks)
Additional context used
actionlint
.github/workflows/test.yml
109-109: shellcheck reported issue in this script: SC2001:style:1:11: See if you can use ${variable//search/replace} instead
(shellcheck)
109-109: shellcheck reported issue in this script: SC2046:warning:1:16: Quote this to prevent word splitting
(shellcheck)
109-109: shellcheck reported issue in this script: SC2005:style:1:16: Useless echo? Instead of 'echo $(cmd)', just use 'cmd'
(shellcheck)
109-109: shellcheck reported issue in this script: SC2086:info:3:157: Double quote to prevent globbing and word splitting
(shellcheck)
Additional comments not posted (1)
.github/workflows/test.yml (1)
Line range hint
1-204: Overall, good improvements to the CI pipeline with code coverage and SonarCloud integration.The changes to this workflow file significantly enhance the CI pipeline by introducing code coverage reporting and SonarCloud integration. These additions will provide better insights into test coverage and code quality, which is valuable for maintaining a robust codebase.
Key improvements:
- Addition of code coverage reporting in the
gojob.- New
sonarCloudTriggerjob for SonarCloud analysis.While there are a few minor suggestions for improvement, the overall structure and implementation of these changes are sound. Once the
projectBaseDirissue is resolved and the script improvements are considered, this will be a solid enhancement to your CI process.
| sonarCloudTrigger: | ||
| needs: go | ||
| name: SonarCloud Trigger | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Clone Repository | ||
| uses: actions/checkout@master | ||
| with: | ||
| fetch-depth: 0 | ||
| - name: Download code coverage results | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: code-coverage-report | ||
| #path: app | ||
| - name: Analyze with SonarCloud | ||
| uses: sonarsource/sonarcloud-github-action@master | ||
| with: | ||
| projectBaseDir: app | ||
| args: > | ||
| -Dsonar.projectKey=osmosis-labs-osmosis | ||
| -Dsonar.organization=osmosis-labs-polaris | ||
| -Dsonar.host.url=https://sonarcloud.io | ||
| -Dsonar.go.coverage.reportPaths=coverage.out | ||
| -Dsonar.go.tests.reportPaths=report.json | ||
| -Dsonar.sources=. | ||
| -Dsonar.tests=. | ||
| -Dsonar.test.inclusions=**/*_test.go,**/testdata/** | ||
| -Dsonar.language=go | ||
| -Dsonar.go.exclusions=**/vendor/**,**/*_mock.go | ||
| -Dsonar.coverage.exclusions=**/mock*/**,**/mock*.go,commands/debug.go | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} |
There was a problem hiding this comment.
💡 Codebase verification
Update SonarCloud projectBaseDir to the repository root.
The app directory does not contain a go.mod file and includes multiple Go source files across various subdirectories. This suggests that app is not a standalone Go module but part of a larger monorepo structure.
Action Required:
- Change
projectBaseDirfromappto.in the SonarCloud configuration to ensure that the entire repository is analyzed correctly.
with:
projectBaseDir: .This adjustment will help SonarCloud accurately assess the project's structure and dependencies.
Analysis chain
Good addition of SonarCloud integration, but check the projectBaseDir setting.
The new sonarCloudTrigger job is well-structured and comprehensive. It correctly depends on the go job, ensuring that code coverage results are available. The SonarCloud configuration covers essential aspects like coverage paths, test paths, and exclusions.
However, there's a potential issue:
The projectBaseDir is set to app (line 189), but it's not clear if this is the correct base directory for your project. This setting determines the root directory for SonarCloud analysis. If your project structure doesn't have an app directory at the root, this could lead to incorrect analysis results.
Please verify if app is the correct base directory. If not, you should adjust this setting to match your project structure, typically using . for the root of the repository.
To confirm the correct project structure, you can run:
This will display the directory structure, helping you determine the correct projectBaseDir setting.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the project structure
ls -R | grep ":$" | sed -e 's/:$//' -e 's/[^-][^\/]*\// /g' -e 's/^/ /' -e 's/-/|/'
Length of output: 8680
Script:
#!/bin/bash
# Check for go.mod in app directory
if [ -f app/go.mod ]; then
echo "go.mod found in app directory."
else
echo "go.mod not found in app directory."
fi
# List Go source files in app directory
echo "Listing Go source files in app:"
rg --files --glob "*.go" app
Length of output: 3406
|
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you! |
Update: #8711
What is the purpose of the change
Update test.yml Github Action to report code and test coverage to sonarcloud
Documentation and Release Note
Unreleasedsection ofCHANGELOG.md? Not needed as doesn't affect codebaseWhere is the change documented?