Repository navigation
Scope the AGENTS.md test-file allowlist to Lib/test - #9519
Conversation
Test Code Modification Rules says the only acceptable modifications to "test files" are adding or removing expectedFailure markers. Read literally, that forbids adding a test to an existing extra_tests/snippets file, which the Testing section asks for. Limit that allowlist to the CPython tests in Lib/test and say snippet additions are allowed. The rules against weakening existing tests still apply everywhere. Assisted-by: Claude Code:claude-opus-5-5
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Clarify test-file change rules AGENTS.md |
The rules scope two decorator changes to CPython test files in Lib/test and allow adding tests to existing extra_tests/snippets files. |
Estimated code review effort: 1 (Trivial) | ~5 minutes
Change: Other
Possibly related PRs
- RustPython/RustPython#9515: Adds tests to an existing snippet file, a change explicitly covered by the updated guidance.
- RustPython/RustPython#9493: Removes expected-failure decorators in CPython tests, which relates to the guidance’s decorator-change scope.
- RustPython/RustPython#9444: Adds tests to an existing snippet file, a change explicitly covered by the updated guidance.
Suggested reviewers: youknowone
Merge Risk: 
04bad
-
The guidance clarifies permitted test additions and decorator changes while retaining restrictions against weakening existing tests. No merge-blocking risk is identified.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "Scope the AGENTS.md test-file allowlist to Lib/test" names the main change. The PR limits the expected-failure-marker allowlist to CPython test files in Lib/test, which the title states. Th… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "Scope the AGENTS.md test-file allowlist to Lib/test" names the main change. The PR limits the expected-failure-marker allowlist to CPython test files in Lib/test, which the title states. The title omits the second change, which clarifies that snippet additions to existing extra_tests/snippets files are allowed. That omission is acceptable because the title does not need to cover every detail. The title is specific and uses no vague terms.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
youknowone
left a comment
There was a problem hiding this comment.
Thank you for catching this! 👍
Summary
AGENTS.mdgives two rules about test files that contradict each other forextra_tests/snippets. The Testing section asks for new tests in "the most closely related existing file inextra_tests/snippets", while Test Code Modification Rules says "the only acceptable modifications to test files" are adding or removing@unittest.expectedFailuremarkers. Read literally, the second forbids what the first asks for.Reviewers that read
AGENTS.mdfollow the second rule. Codex flagged the snippet additions in #9515, #9516 and #9517 as violations of it, each citing AGENTS.md L280-L282, and asked to drop the tests and keep only the Rust fix.The allowlist came in with #5840 next to
Lib/test/test_typing.pychanges, and both of its items areunittestmarkers, so it is about the CPython tests inLib/test.Changes
Lib/test.I used Claude Code (claude-opus-5-5) to draft the change and this description; I reviewed both myself.
Summary by CodeRabbit
Lib/test.extra_tests/snippetsfiles and linked to the Testing section.