Fix: include ollama coverage #17
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 addresses a code coverage regression. The primary goal is to ensure that tests properly cover the src.cli_code directory. Changes include modifications to the .coveragerc file to include more files in the coverage reports, excluding the ollama.py file due to its integration with external dependencies, and updating the run_coverage_ci.sh script to improve the coverage reporting and handle test failures correctly. Additionally, the run_sonar_scan.sh script and sonar-project.properties file were updated to enhance SonarCloud analysis.
Highlights
- Coverage Configuration: The
.coveragercfile is updated to include all Python files withinsrc.cli_codeand its subdirectories for coverage analysis. Theollama.pyfile is excluded due to the difficulty of testing its external API integrations. - CI Script Modifications: The
run_coverage_ci.shscript is modified to exit on test failures, append coverage data from different test groups, and improve overall coverage reporting. - SonarCloud Integration: The
run_sonar_scan.shscript andsonar-project.propertiesfile are updated to pass additional properties to SonarCloud for more accurate analysis, including source directories, test directories, and code exclusions. - Test Failure Handling: The CI environment variable
CI_EXIT_ON_TEST_FAILUREis set to 1 to ensure that the CI pipeline exits if any tests fail, maintaining code quality.
Changelog
Click here to see the changelog
- .coveragerc
- Updated include patterns to cover all Python files under
src.cli_code. - Excluded
ollama.pyfrom coverage due to integration complexities.
- Updated include patterns to cover all Python files under
- pr_description.md
- This file was removed from the pull request.
- scripts/run_coverage_ci.sh
- Set
CI_EXIT_ON_TEST_FAILUREto 1 to exit on test failures. - Added
--cov-appendto pytest command to combine coverage data. - Removed redundant setting of
CI_EXIT_ON_TEST_FAILUREat the end of the script.
- Set
- scripts/run_sonar_scan.sh
- Added properties to pass source directories, test directories, encoding, SCM provider, and coverage report paths to SonarCloud.
- sonar-project.properties
- Added properties for source directories, test directories, encoding, and SCM provider.
- Added
sonar.newCode.referenceBranchto force SonarCloud to see all files as new code. - Added comment explaining the exclusion of
ollama.py.
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.
A coverage fix,
To catch bugs in the mix,
Tests now take flight,
Ensuring code's bright,
A quality code trick.
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 pull request aims to improve code coverage by adjusting include/exclude patterns in .coveragerc, modifying the CI script to enforce test failures, and updating SonarCloud configuration. The changes seem reasonable and contribute to better code quality and analysis.
Summary of Findings
- Coverage Include Patterns: The changes to
.coveragercinclude more generic patterns for including files in coverage reports. While this may increase coverage, it's important to ensure that only relevant code is included and that test performance isn't negatively impacted. - Enforcing Test Failures in CI: The change to
run_coverage_ci.shenforces test failures by settingCI_EXIT_ON_TEST_FAILURE=1. This is a good practice to ensure that failing tests are not ignored in the CI pipeline. - SonarCloud Configuration: The changes to
run_sonar_scan.shandsonar-project.propertiesadd more configuration options for SonarCloud, including source and test directories, encoding, and SCM provider. These changes should improve the accuracy of SonarCloud analysis. - Ollama Model Exclusion: The exclusion of
ollama.pyfrom coverage and SonarCloud analysis due to integration code difficulties is noted. Consider addressing this in the future with better mocking or integration tests. - Temporary SonarCloud Configuration: The temporary setting
sonar.newCode.referenceBranch=origin/maininsonar-project.propertiesis flagged for removal after establishing a baseline. Ensure this is removed in a subsequent PR.
Merge Readiness
The pull request appears to improve code coverage and CI practices. However, it's important to consider the potential impact of the broader coverage include patterns on test performance and relevance. The temporary SonarCloud configuration should be removed in a future PR. I am unable to approve this pull request, and recommend that others review and approve this code before merging. Given the medium severity comment, I recommend that the author address it before merging.
| */src/cli_code/*.py | ||
| */src/cli_code/*/*.py | ||
| */src/cli_code/*/*/*.py |
There was a problem hiding this comment.
Consider if these broad include patterns might negatively impact test performance or include irrelevant code in coverage reports. Is it possible to be more specific about the directories that need coverage?
*/src/cli_code/specific_directory/*.py
*/src/cli_code/another_directory/*/*.py
Code Coverage Report📊 Current Coverage: 31.58% 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. |
|
No description provided.