-
Notifications
You must be signed in to change notification settings - Fork 171
#SBCOSS-452 Updated the Code-Quality-check workflow for search service. #1109
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
#SBCOSS-452 Updated the Code-Quality-check workflow for search service. #1109
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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
This PR enhances the existing Code-Quality-check workflow by adding dedicated CI steps for the search-service. It introduces a new test job, integrates SonarCloud analysis for the search-service, and updates the PR comment to include its quality gate results.
- Added
test-searchjob to run and collect search-service unit tests - Added
sonar-analysis-searchjob to perform SonarCloud analysis on search-service - Updated
comment-prjob to include Search Service Analysis link
Comments suppressed due to low confidence (2)
.github/workflows/Code-Quality-check.yml:242
- The path points to an XML report, but the default
jacoco:reportgoal generates HTML by default. Ensure your Jacoco plugin is configured to produce an XML report (e.g.,<xml>true</xml>in your POM) or use thejacoco:report-xmlgoal so SonarCloud can parse coverage.
-Dsonar.coverage.jacoco.xmlReportPaths=search-api/search-service/target/site/jacoco/jacoco.xml
.github/workflows/Code-Quality-check.yml:216
- [nitpick] The search tests run on JDK 11 but the SonarCloud analysis for search is set up with JDK 17. It’s best to align on a consistent JDK version across both jobs or document why different versions are needed.
- name: Set up JDK 17
| - name: Run Search Tests | ||
| working-directory: search-api/search-service/ | ||
| run: | | ||
| mvn clean test org.jacoco:jacoco-maven-plugin:0.8.8:prepare-agent test org.jacoco:jacoco-maven-plugin:0.8.8:report \ |
Copilot
AI
Jun 9, 2025
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.
The Maven command invokes the test goal twice (before and after the Jacoco prepare-agent). This will run your tests twice and slow down the workflow. Consider using mvn clean org.jacoco:jacoco-maven-plugin:0.8.8:prepare-agent test org.jacoco:jacoco-maven-plugin:0.8.8:report instead.
| mvn clean test org.jacoco:jacoco-maven-plugin:0.8.8:prepare-agent test org.jacoco:jacoco-maven-plugin:0.8.8:report \ | |
| mvn clean org.jacoco:jacoco-maven-plugin:0.8.8:prepare-agent test org.jacoco:jacoco-maven-plugin:0.8.8:report \ |
| cache: 'maven' | ||
|
|
||
| - name: Run Search Tests | ||
| working-directory: search-api/search-service/ |
Copilot
AI
Jun 9, 2025
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.
[nitpick] Trailing slash in working-directory is not needed and differs from other job definitions. Consider removing it for consistency (search-api/search-service).
| working-directory: search-api/search-service/ | |
| working-directory: search-api/search-service |
ab87fc9
into
Sunbird-Knowlg:code-quality
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Type of change
Please choose appropriate options.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes in the below checkboxes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: