Remove SonarCloud debugging and align local scan with CI #16
Conversation
…ON_TEST_FAILURE handling
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @ipv1337, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request focuses on improving the SonarCloud integration and code coverage reporting. It removes a debugging flag from the SonarCloud configuration, aligns the local scanner script arguments with the CI workflow, and makes adjustments to coverage reporting and handling of test failures. The PR also excludes the ollama.py file from coverage reports due to its integration with external dependencies.
Highlights
- SonarCloud Configuration: The
-Dsonar.verbose=trueflag has been removed from the GitHub workflow configuration to reduce verbosity during SonarCloud scans. - Local Scan Alignment: The
run_sonar_scan.shscript has been updated to use consistent arguments with the CI workflow, ensuring consistent scanning behavior between local and CI environments. Specifically, it now includes properties forsonar.sources,sonar.tests,sonar.sourceEncoding,sonar.scm.provider, andsonar.coverage.jacoco.xmlReportPaths. - Coverage Reporting: The
.coveragercfile has been modified to include more specific paths for coverage and to excludeollama.pyfrom coverage reports.ollama.pyis excluded because it is difficult to test without mocking the entire Ollama API. - CI Test Handling: The
run_coverage_ci.shscript has been updated to exit on test failures in CI, ensuring code quality. The script now uses theCI_EXIT_ON_TEST_FAILUREvalue set at the beginning of the script to determine whether to exit with an error code on test failures, and the--cov-appendflag is added to pytest. - SonarCloud New Code: The
sonar-project.propertiesfile now includessonar.newCode.referenceBranch=origin/mainto force SonarCloud to see all files as new code for proper coverage metrics. A comment indicates this is temporary and should be removed after an initial coverage baseline is established.
Changelog
Click here to see the changelog
- .coveragerc
- Expanded the
includesection to cover more Python files. - Added
*/src/cli_code/models/ollama.pyto theomitsection, excluding it from coverage reports.
- Expanded the
- pr_description.md
- The entire file has been removed.
- scripts/run_coverage_ci.sh
- Changed
CI_EXIT_ON_TEST_FAILUREto 1 to exit on test failures in CI. - Added
--cov-appendto the pytest command for generating the final coverage report. - Removed the reassignment of
CI_EXIT_ON_TEST_FAILUREat the end of the script.
- Changed
- scripts/run_sonar_scan.sh
- Added properties to the sonar-scanner command to align with CI, including
sonar.sources,sonar.tests,sonar.sourceEncoding,sonar.scm.provider, andsonar.coverage.jacoco.xmlReportPaths.
- Added properties to the sonar-scanner command to align with CI, including
- sonar-project.properties
- Added a comment explaining the exclusion of
ollama.pyfrom coverage. - Added
sonar.newCode.referenceBranch=origin/mainto force SonarCloud to treat all code as new for initial coverage metrics.
- Added a comment explaining the exclusion of
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
The scanner hums, a watchful eye,
On code that strives to reach the sky.
With flags aligned and tests that gleam,
A cleaner build, a coder's dream.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This PR aims to remove debugging flags from SonarCloud scans and align the local scanner script arguments with the CI workflow configuration. The changes look good overall, contributing to a cleaner and more consistent build process.
Summary of Findings
- Coverage Exclusions: The PR excludes
ollama.pyfrom coverage due to integration code with external dependencies. While understandable, consider if mocking or integration tests can be added to improve coverage in the future. - CI Exit on Test Failure: The PR changes the CI to exit on test failures. This is a good practice to ensure code quality and prevent merging code with failing tests.
- SonarCloud Configuration: The PR updates the local
run_sonar_scan.shscript to use consistent arguments with CI, ensuring a more reliable and comparable analysis between local and CI environments. - SonarCloud New Code Metrics: The PR temporarily forces SonarCloud to see all files as new code. Remember to remove this after the initial coverage baseline is established to ensure future PRs are evaluated correctly.
Merge Readiness
The changes in this PR are beneficial for maintaining code quality and aligning local scans with CI. However, the temporary SonarCloud configuration should be removed after the initial coverage baseline is established. I am unable to approve this pull request, and recommend that others review and approve this code before merging.
| # Note: We're using this temporarily to establish accurate baseline coverage | ||
| # TODO: Remove this line after initial coverage baseline is established to ensure | ||
| # future PRs are evaluated correctly for new code coverage | ||
| sonar.newCode.referenceBranch=origin/main |
There was a problem hiding this comment.
This section introduces a temporary configuration to force SonarCloud to treat all files as new code. It's crucial to remove this line after the initial coverage baseline is established to ensure accurate future PR evaluations. Leaving a TODO is good, but consider adding a specific date or issue link for tracking.
|
|
||
| # Set environment variables for CI | ||
| export CI_EXIT_ON_TEST_FAILURE=0 # Don't exit on test failures in CI | ||
| export CI_EXIT_ON_TEST_FAILURE=1 # Exit on test failures to ensure code quality |
There was a problem hiding this comment.
Setting CI_EXIT_ON_TEST_FAILURE to 1 is a good practice to ensure that the CI pipeline fails if any tests fail. This helps prevent merging code with failing tests.
Code Coverage Report📊 Current Coverage: 38.35% Detailed coverage analysis is available in SonarCloud Coverage Change DetailsThis shows code coverage for changes in this PR. To improve coverage, consider adding tests for new or modified code. |
Code Coverage Report📊 Current Coverage: 38.35% Detailed coverage analysis is available in SonarCloud Coverage Change DetailsThis shows code coverage for changes in this PR. To improve coverage, consider adding tests for new or modified code. |
|
This PR removes debugging flags from SonarCloud scans and aligns the local scanner script arguments with CI workflow configuration.\n\nChanges:\n- Removed -Dsonar.verbose=true flag from GitHub workflow configuration\n- Updated local run_sonar_scan.sh script to use consistent arguments with CI