Repository navigation
feat(audit): add python-test-suite-audit skill - #2
Merged
Merged
Conversation
…tic scanners Read-only suite health audit: tautology, weak assertion, mock echo, private coupling, error depth, isolation. Quick static-only default, sampled deep probes opt-in. Bundles stdlib-only audit_assertions and plan_audit_scope helpers, severity scorecard template, eval fixtures, and audit script coverage. Updates catalog, docs, CI, and quality contracts.
Reviewer's GuideAdds and fully integrates a read-only Flow diagram for the read-only Python test-suite auditflowchart LR
A[Existing Python test suite] --> B[Quick static audit]
B --> C{Execution confirmation}
C -->|No| D[Static findings]
C -->|Yes| E[Bounded mutation or order probes]
E --> F[Confirmed findings]
D --> G[Critical Major Minor scorecard]
F --> G
G --> H[Evidence report]
Flow diagram for audit evidence and severity reportingflowchart LR
A[Audit six dimensions] --> B[Collect evidence]
B --> C{Execution confirmed}
C -->|No| D[Unconfirmed observation]
C -->|Yes| E[Confirmed finding]
D --> F[Assign Critical Major Minor]
E --> F
F --> G[Severity scorecard]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path=".agents/skills/python-test-suite-audit/scripts/audit_assertions.py" line_range="69-71" />
<code_context>
+ return [
+ node
+ for node in ast.walk(tree)
+ if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
+ and node.name.startswith("test")
+ ]
+
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The scanner traverses nested function bodies as if they belonged to the enclosing test. A nested `test_*` function that pytest will never collect is reported as a test, while an outer test whose unused nested helper contains the only `assert` is treated as assertion-bearing and its no-assertion finding is missed.
**Triggers:** When test modules contain nested helpers or nested functions named `test_*`.
**Suggested fix:** Limit test discovery to collected/top-level test functions and exclude nested function/class scopes when gathering assertions and calls for a test.
</issue_to_address>
### Comment 2
<location path=".agents/skills/python-test-suite-audit/scripts/audit_assertions.py" line_range="314-315" />
<code_context>
+ else:
+ truncated = True
+ continue
+ has_mock = _has_mock_import(tree)
+ for func in _test_functions(tree):
+ for item in _scan_function(func, path, has_mock):
+ counter += 1
</code_context>
<issue_to_address>
**nitpick (bug_risk):** `has_mock` is computed for the entire file and then passed to every test function, so importing a mock for one test enables mock-echo detection in unrelated tests. Any unrelated test in that file that uses a `return_value` or `side_effect` keyword and an assertion is reported as a mock echo even when the configured object is not a mock or the asserted value is transformed by the system under test.
**Triggers:** When one test in a file imports `unittest.mock` and another test uses `return_value` or `side_effect`.
**Suggested fix:** Track mock imports and mock configuration per function, or retain this as an explicitly file-level advisory with evidence tied to the actual mock object.
</issue_to_address>
### Comment 3
<location path=".agents/skills/python-test-suite-audit/scripts/audit_assertions.py" line_range="210-213" />
<code_context>
+ local_names.add(alias.asname or alias.name.split(".")[0])
+
+ for node in ast.walk(func):
+ if isinstance(node, ast.ExceptHandler):
+ if node.type is None or (
+ isinstance(node.type, ast.Name) and node.type.id == "Exception"
+ ):
+ findings.append(
+ {
</code_context>
<issue_to_address>
**nitpick (bug_risk):** The broad-exception heuristic only recognizes a bare handler or a handler whose type is exactly the name `Exception`; it does not flag `except (Exception, ValueError):`, so a documented broad-except pattern is silently omitted from common tuple-form handlers.
**Triggers:** When a test uses a tuple-form exception handler containing `Exception`.
**Suggested fix:** Recognize `ast.Tuple` exception types recursively and flag handlers containing `Exception` or other configured broad base exceptions.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: .agents/skills/python-test-suite-audit/scripts/audit_assertions.py:71
…e except Addresses Sourcery review on PR #2: - collect only top-level test functions and class methods; exclude nested scopes from assertion/call ownership - tie mock-echo to a mock factory call in the same test, including mocker.patch; unrelated return_value kwargs no longer flag - flag tuple-form broad handlers containing Exception/BaseException Adds 8 regression tests and documents scanner scope in heuristics.
Contributor
Author
|
Addressed all 3 Sourcery comments in d847b69 (all fixed, none skipped or deferred):
Verification: 8 new regression tests (40 in |
- exact len() equality no longer flagged as weak-length-assert - pytest-only match= requirement; unittest assertRaises exempt - unittest assert detection rejects assertion/failure lookalikes - global-environ-access restricted to os.environ - call-count set uses real mock attr called, drops called_once - has_raises precedence made explicit and aligned - heuristics doc updated for per-function mock-echo detection
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
1 participant
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.
Adds read-only
python-test-suite-auditskill (installable vianpx skills add nexusnv/python-agentic-skills --skill python-test-suite-audit).audit_assertions.py,plan_audit_scope.pytests/test_audit_scripts.py(32 tests)Verification:
pytest457 passed,ruff check+format --checkclean,agentskills validatepasses,skills.shdiscovery lists all 3 skills. Subagent review completed; load-bearing script-coverage gap fixed plus scanner FP refinements.Summary by Sourcery
Add a read-only Python test-suite auditing skill with bounded static analysis, evidence-based severity scoring, and repository validation support.
New Features:
python-test-suite-auditskill for evaluating Python test-suite fault-detection strength across six audit dimensions.Enhancements:
CI:
Documentation:
Tests: