ci: enforce 80% statement coverage gate#1286
Open
spoorcc wants to merge 8 commits into
Open
Conversation
Add fail_under = 80 to [tool.coverage.report] in pyproject.toml and run coverage report --fail-under=80 in CI before generating the XML report. Satisfies the OpenSSF silver badge test_statement_coverage80 criterion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JnDFTrUYv86A6HGgZ1qwMa
Single page satisfies three OpenSSF silver badge criteria: governance (decision-making model), roles_responsibilities, and access_continuity. Wired into the Explanation toctree. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JnDFTrUYv86A6HGgZ1qwMa
Explicit credit commitment satisfies the OpenSSF silver badge vulnerability_report_credit criterion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JnDFTrUYv86A6HGgZ1qwMa
Placeholder satisfies the OpenSSF silver badge documentation_roadmap criterion. Content will be extended later. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JnDFTrUYv86A6HGgZ1qwMa
Contributor
|
Warning Review limit reached
More reviews will be available in 18 minutes and 24 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (17)
WalkthroughThe PR enforces an 80% code coverage minimum via ChangesCoverage Configuration, Documentation, and Security Policy
SuperProject Implementation Tests
Reporter Implementation Tests
Command Implementation Tests
Terminal and UI Tests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Tests for check_sub_manifests and _make_recommendation covering the warning logic for submanifest projects not present in the parent manifest. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JnDFTrUYv86A6HGgZ1qwMa
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| # mypy: ignore-errors | ||
| # flake8: noqa | ||
|
|
||
| from unittest.mock import MagicMock, Mock, call, patch |
|
|
||
| from unittest.mock import MagicMock, Mock, call, patch | ||
|
|
||
| import pytest |
New test files covering previously under-tested modules: - commands/diff, freeze, format_patch, update_patch, misc - reporting/check: sarif_reporter, jenkins_reporter, stdout_reporter - project: gitsuperproject, svnsuperproject, svnsubproject - terminal: screen, prompt - superproject and version helpers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JnDFTrUYv86A6HGgZ1qwMa
| # mypy: ignore-errors | ||
| # flake8: noqa | ||
|
|
||
| import json |
| import json | ||
| from unittest.mock import MagicMock, Mock, mock_open, patch | ||
|
|
||
| import pytest |
|
|
||
| import argparse | ||
| from pathlib import Path | ||
| from unittest.mock import MagicMock, Mock, patch |
|
|
||
| import argparse | ||
| from pathlib import Path | ||
| from unittest.mock import MagicMock, Mock, patch |
|
|
||
| import argparse | ||
| from pathlib import Path | ||
| from unittest.mock import MagicMock, Mock, call, patch |
| import pathlib | ||
| from unittest.mock import MagicMock, Mock, patch | ||
|
|
||
| import pytest |
| # mypy: ignore-errors | ||
| # flake8: noqa | ||
|
|
||
| import json |
| # mypy: ignore-errors | ||
| # flake8: noqa | ||
|
|
||
| import json |
Adds one test covering the list_tool_info() loop in environment.py, bringing total coverage to 80.19%. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JnDFTrUYv86A6HGgZ1qwMa
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/test_jenkins_reporter.py (1)
85-89:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTighten
_classassertion to prevent false positives.The current check allows many invalid values because it only checks for one of two substrings. Assert the exact expected
_classvalue (or exact schema shape) so this test actually guards reporter output structure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_jenkins_reporter.py` around lines 85 - 89, The assertion in the test_report_has_correct_class_key function uses loose substring checks that allow invalid values to pass. Replace the current assertion that checks for "issues" or "jenkins" substrings with an exact equality check against the specific expected value for the _class field, ensuring the test properly validates the reporter's output schema rather than allowing partial matches that could be false positives.tests/test_check_reporters.py (1)
1-170: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winSplit mixed reporter tests into module-mirrored files.
tests/test_check_reporters.pymixes tests forjenkins_reporterandcode_climate_reporterin a single non-mirrored filename. Please split/rename into module-aligned files (for example,tests/test_code_climate_reporter.py) to match repository test naming policy and keep ownership clear.As per coding guidelines, "
tests/test_*.py: Usepytestfor unit tests; test files should mirror module names (e.g.,tests/test_manifest.pyfordfetch/manifest.py)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_check_reporters.py` around lines 1 - 170, Split the mixed test file tests/test_check_reporters.py into two separate module-aligned files following the repository naming policy. Create tests/test_jenkins_reporter.py containing all test functions prefixed with test_jenkins (test_jenkins_reporter_init_creates_empty_issues, test_jenkins_add_issue_appends_entry, test_jenkins_add_issue_contains_severity, test_jenkins_add_issue_contains_project_name, test_jenkins_dump_to_file_writes_json) and their associated imports and helper functions. Create tests/test_code_climate_reporter.py containing all test functions prefixed with test_code_climate (test_code_climate_severity_high_maps_to_major, test_code_climate_severity_normal_maps_to_minor, test_code_climate_severity_low_maps_to_info, test_code_climate_add_issue_appends_entry, test_code_climate_add_issue_contains_check_name, test_code_climate_add_issue_severity_value, test_code_climate_dump_to_file_writes_json, test_code_climate_fingerprint_is_deterministic) with their associated imports and helper functions. The shared helper functions _make_manifest, _make_project, and _make_issue should be included in both files or moved to a conftest.py.Source: Coding guidelines
tests/test_commands_misc.py (1)
1-397: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit this mixed test module to match module-mirroring policy.
This file bundles tests for multiple command modules; please split into module-aligned files so test files mirror source modules.
As per coding guidelines, "tests/test_*.py: Use
pytestfor unit tests; test files should mirror module names (e.g.,tests/test_manifest.pyfordfetch/manifest.py)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_commands_misc.py` around lines 1 - 397, The test file bundles tests for multiple command modules (Environment, Init, Validate, FormatPatch, and UpdatePatch) which violates the module-mirroring policy. Split this file into separate test files: create tests/test_environment.py containing test_environment_prints_version, test_environment_logs_newer_version_when_available, test_environment_no_newer_version_notice, test_environment_calls_list_tool_info_for_each_project_type, and test_environment_create_menu; create tests/test_init.py with test_init_creates_manifest_when_absent, test_init_does_not_overwrite_existing_manifest, and test_init_create_menu; create tests/test_validate.py with test_validate_calls_manifest_from_file, test_validate_prints_valid, and test_validate_create_menu; create tests/test_format_patch.py with test_determine_target_patch_type_git, test_determine_target_patch_type_svn, test_determine_target_patch_type_plain, test_format_patch_no_patch_logs_warning, and test_format_patch_runtime_error_raises; and create tests/test_update_patch.py with all remaining test_update_patch_* tests. Each new file should include relevant imports and be named to match its corresponding source module in dfetch/commands/.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_commands_freeze.py`:
- Line 56: The patch target string in the test_commands_freeze.py file for
mocking create_sub_project contains an incorrect attribute chain that does not
exist. Review where create_sub_project is actually imported or referenced in the
dfetch.commands.freeze module and correct the patch target accordingly. The
patch strings on lines 56, 75, 97, and 119 all need to be updated to point to
the correct location where create_sub_project is accessed within the freeze
module, ensuring the mock is applied at the point where the function is actually
used.
In `@tests/test_commands_misc.py`:
- Line 186: The patch target paths in the test cases are unstable and contain
redundant module references that cause AttributeError failures. In the mock
patch calls on lines 186, 212, 254, 280, 309, and 337, replace the patch target
string from the current format to use a direct import path to where
create_sub_project is actually defined or used. The path should reference
dfetch.project.create_sub_project or the appropriate module where this function
is imported within the format_patch module, removing the redundant dfetch
reference in the middle of the path to ensure the patch target is valid and
stable.
In `@tests/test_superproject_and_version.py`:
- Line 1: The test file `tests/test_superproject_and_version.py` combines tests
for two separate modules (NoVcsSuperProject from dfetch/project/superproject.py
and version functionality from dfetch/manifest/version.py), violating the naming
convention. Split this file into two separate test files that mirror their
respective module names: create `tests/test_superproject.py` containing all
tests related to NoVcsSuperProject, and create `tests/test_version.py` (or
appropriate naming for the manifest version module) containing all tests related
to version functionality, then remove the original combined test file.
---
Outside diff comments:
In `@tests/test_check_reporters.py`:
- Around line 1-170: Split the mixed test file tests/test_check_reporters.py
into two separate module-aligned files following the repository naming policy.
Create tests/test_jenkins_reporter.py containing all test functions prefixed
with test_jenkins (test_jenkins_reporter_init_creates_empty_issues,
test_jenkins_add_issue_appends_entry, test_jenkins_add_issue_contains_severity,
test_jenkins_add_issue_contains_project_name,
test_jenkins_dump_to_file_writes_json) and their associated imports and helper
functions. Create tests/test_code_climate_reporter.py containing all test
functions prefixed with test_code_climate
(test_code_climate_severity_high_maps_to_major,
test_code_climate_severity_normal_maps_to_minor,
test_code_climate_severity_low_maps_to_info,
test_code_climate_add_issue_appends_entry,
test_code_climate_add_issue_contains_check_name,
test_code_climate_add_issue_severity_value,
test_code_climate_dump_to_file_writes_json,
test_code_climate_fingerprint_is_deterministic) with their associated imports
and helper functions. The shared helper functions _make_manifest, _make_project,
and _make_issue should be included in both files or moved to a conftest.py.
In `@tests/test_commands_misc.py`:
- Around line 1-397: The test file bundles tests for multiple command modules
(Environment, Init, Validate, FormatPatch, and UpdatePatch) which violates the
module-mirroring policy. Split this file into separate test files: create
tests/test_environment.py containing test_environment_prints_version,
test_environment_logs_newer_version_when_available,
test_environment_no_newer_version_notice,
test_environment_calls_list_tool_info_for_each_project_type, and
test_environment_create_menu; create tests/test_init.py with
test_init_creates_manifest_when_absent,
test_init_does_not_overwrite_existing_manifest, and test_init_create_menu;
create tests/test_validate.py with test_validate_calls_manifest_from_file,
test_validate_prints_valid, and test_validate_create_menu; create
tests/test_format_patch.py with test_determine_target_patch_type_git,
test_determine_target_patch_type_svn, test_determine_target_patch_type_plain,
test_format_patch_no_patch_logs_warning, and
test_format_patch_runtime_error_raises; and create tests/test_update_patch.py
with all remaining test_update_patch_* tests. Each new file should include
relevant imports and be named to match its corresponding source module in
dfetch/commands/.
In `@tests/test_jenkins_reporter.py`:
- Around line 85-89: The assertion in the test_report_has_correct_class_key
function uses loose substring checks that allow invalid values to pass. Replace
the current assertion that checks for "issues" or "jenkins" substrings with an
exact equality check against the specific expected value for the _class field,
ensuring the test properly validates the reporter's output schema rather than
allowing partial matches that could be false positives.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ffaa8672-4536-40f2-960d-8ed1263ee063
📒 Files selected for processing (16)
tests/test_check_reporters.pytests/test_check_stdout_reporter.pytests/test_commands_diff.pytests/test_commands_format_patch.pytests/test_commands_freeze.pytests/test_commands_misc.pytests/test_commands_update_patch.pytests/test_gitsuperproject.pytests/test_jenkins_reporter.pytests/test_sarif_reporter.pytests/test_screen.pytests/test_stdout_reporter.pytests/test_superproject_and_version.pytests/test_svnsubproject.pytests/test_svnsuperproject.pytests/test_terminal_prompt.py
|
|
||
| with patch("dfetch.commands.freeze.create_super_project", return_value=superproject): | ||
| with patch("dfetch.commands.freeze.in_directory"): | ||
| with patch("dfetch.commands.freeze.dfetch.project.create_sub_project") as mock_create: |
Contributor
There was a problem hiding this comment.
Correct patch target for create_sub_project in Freeze tests.
These patch strings likely point to a non-existent attribute chain and can fail before test logic executes.
Proposed fix
- with patch("dfetch.commands.freeze.dfetch.project.create_sub_project") as mock_create:
+ with patch("dfetch.project.create_sub_project") as mock_create:
- with patch("dfetch.commands.freeze.dfetch.project.create_sub_project") as mock_create:
+ with patch("dfetch.project.create_sub_project") as mock_create:
- with patch("dfetch.commands.freeze.dfetch.project.create_sub_project") as mock_create:
+ with patch("dfetch.project.create_sub_project") as mock_create:
- with patch("dfetch.commands.freeze.dfetch.project.create_sub_project") as mock_create:
+ with patch("dfetch.project.create_sub_project") as mock_create:Also applies to: 75-75, 97-97, 119-119
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_commands_freeze.py` at line 56, The patch target string in the
test_commands_freeze.py file for mocking create_sub_project contains an
incorrect attribute chain that does not exist. Review where create_sub_project
is actually imported or referenced in the dfetch.commands.freeze module and
correct the patch target accordingly. The patch strings on lines 56, 75, 97, and
119 all need to be updated to point to the correct location where
create_sub_project is accessed within the freeze module, ensuring the mock is
applied at the point where the function is actually used.
|
|
||
| with patch("dfetch.commands.format_patch.create_super_project", return_value=superproject): | ||
| with patch("dfetch.commands.format_patch.in_directory"): | ||
| with patch("dfetch.commands.format_patch.dfetch.project.create_sub_project") as mock_create: |
Contributor
There was a problem hiding this comment.
Fix patch target paths; current targets likely fail before assertions run.
patch("dfetch.commands.<...>.dfetch.project.create_sub_project") is not a stable import path and can fail with AttributeError, which makes these tests fail for the wrong reason.
Proposed fix
- with patch("dfetch.commands.format_patch.dfetch.project.create_sub_project") as mock_create:
+ with patch("dfetch.project.create_sub_project") as mock_create:
- with patch("dfetch.commands.format_patch.dfetch.project.create_sub_project") as mock_create:
+ with patch("dfetch.project.create_sub_project") as mock_create:
- with patch("dfetch.commands.update_patch.dfetch.project.create_sub_project") as mock_create:
+ with patch("dfetch.project.create_sub_project") as mock_create:
- with patch("dfetch.commands.update_patch.dfetch.project.create_sub_project") as mock_create:
+ with patch("dfetch.project.create_sub_project") as mock_create:
- with patch("dfetch.commands.update_patch.dfetch.project.create_sub_project") as mock_create:
+ with patch("dfetch.project.create_sub_project") as mock_create:
- with patch("dfetch.commands.update_patch.dfetch.project.create_sub_project") as mock_create:
+ with patch("dfetch.project.create_sub_project") as mock_create:Also applies to: 212-212, 254-254, 280-280, 309-309, 337-337
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_commands_misc.py` at line 186, The patch target paths in the test
cases are unstable and contain redundant module references that cause
AttributeError failures. In the mock patch calls on lines 186, 212, 254, 280,
309, and 337, replace the patch target string from the current format to use a
direct import path to where create_sub_project is actually defined or used. The
path should reference dfetch.project.create_sub_project or the appropriate
module where this function is imported within the format_patch module, removing
the redundant dfetch reference in the middle of the path to ensure the patch
target is valid and stable.
| @@ -0,0 +1,107 @@ | |||
| """Tests for dfetch/project/superproject.py (NoVcsSuperProject) and dfetch/manifest/version.py.""" | |||
Contributor
There was a problem hiding this comment.
Split this test file so test filenames mirror module names.
This file mixes tests for two modules (dfetch/project/superproject.py and dfetch/manifest/version.py) under one tests/test_*.py file, which breaks the repository naming convention for tests.
As per coding guidelines, “tests/test_*.py: Use pytest for unit tests; test files should mirror module names (e.g., tests/test_manifest.py for dfetch/manifest.py)”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_superproject_and_version.py` at line 1, The test file
`tests/test_superproject_and_version.py` combines tests for two separate modules
(NoVcsSuperProject from dfetch/project/superproject.py and version functionality
from dfetch/manifest/version.py), violating the naming convention. Split this
file into two separate test files that mirror their respective module names:
create `tests/test_superproject.py` containing all tests related to
NoVcsSuperProject, and create `tests/test_version.py` (or appropriate naming for
the manifest version module) containing all tests related to version
functionality, then remove the original combined test file.
Source: Coding guidelines
spoorcc
force-pushed
the
claude/bestpractices-silver-badge-4zmhea
branch
from
406769a to
fff2b75
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add fail_under = 80 to [tool.coverage.report] in pyproject.toml and
run coverage report --fail-under=80 in CI before generating the XML
report. Satisfies the OpenSSF silver badge test_statement_coverage80
criterion.
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01JnDFTrUYv86A6HGgZ1qwMa
Summary by CodeRabbit
Documentation
Chores
Tests