Add CI step for checking redundant test patches by ShaharNaveh · Pull Request #7126 · RustPython/RustPython · GitHub

Summary by CodeRabbit

Chores

Improved continuous integration pipeline with enhanced Python code quality checks to automatically identify and report redundant test patches.

📝 Walkthrough

Walkthrough

Adds a Python script to identify and flag redundant test patches in the codebase—test functions that solely invoke super().<test>() variants—and integrates this checker into the CI/CD pipeline via an automated lint step.

Changes

Cohort / File(s)	Summary
CI/CD Workflow Integration .github/workflows/ci.yaml	Updated lint job name and added Python environment setup with a new step to execute the redundant test patch checker script.
New Redundant Patch Detector scripts/check_redundant_patches.py	New Python utility that parses test files, identifies test functions with no decorators, detects those matching super().() call patterns, and reports them as removable with file path, function name, and line number. Exits with non-zero status when matches are found.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

PR #6089: Relates to Python test AST processing for unittest-style wrapper patches; while this PR flags removable super().() wrappers, that PR handles generation and application of such patterns.

Suggested reviewers

youknowone
reactive-firewall

Poem

🐰 A hop and a skip through AST nodes so fine,
We hunt redundant patches that simply align,
With super().() calls, oh what a sight!
The CI now checks them—removed overnight,
One rabbit's joy: cleaner code done right! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1 ❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add CI step for checking redundant test patches' accurately describes the main change: adding a new CI workflow step (in .github/workflows/ci.yaml) that runs a script to check for redundant test patches.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ShaharNaveh Can this be auto-fix using update_lib instead just lint?

@ShaharNaveh Can this be auto-fix using update_lib instead just lint?

Yes, but I'd rather not doing that, for multiple reasons:

The update_lib is not stable enough imo.
We would run into issues where someone fixes something else, but this will cause the entire test file to be updated and it will add more stuff to the PR that can be out of scope.

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In @.github/workflows/ci.yaml:
- Around line 462-470: Remove the duplicate checkout step: the workflow contains
two identical steps using "uses: actions/checkout@v6.0.2"; keep the first
checkout that precedes the "Check for redundant test patches" step and delete
the second "uses: actions/checkout@v6.0.2" entry to avoid re-checking out the
repo unnecessarily.

In `@scripts/check_redundant_patches.py`:
- Around line 18-23: The except SyntaxError block after calling
ast.parse(contents) currently uses pass, which lets execution continue to
ast.walk(tree) and causes NameError or stale-tree reuse; change the handler so
the file is skipped (e.g., continue out of the loop) or explicitly set tree =
None and skip calling ast.walk for that file — update the block around
ast.parse(contents) / ast.walk(tree) to ensure that when ast.parse raises
SyntaxError you do not call ast.walk on an undefined or previous tree.

🧹 Nitpick comments (2)

scripts/check_redundant_patches.py (2)
12-12: rglob("**/*.py") double-recurses; use rglob("*.py") instead.

Path.rglob already applies the pattern recursively, so the leading **/ is redundant. It still works, but rglob("*.py") is the idiomatic form.
Proposed fix
-    for file in TEST_DIR.rglob("**/*.py"):
+    for file in TEST_DIR.rglob("*.py"):
51-52: Prefer sys.exit() over the builtin exit().

sys is already imported. The builtin exit() is intended for the interactive interpreter; sys.exit() is the standard way to exit from scripts.
Proposed fix
 if __name__ == "__main__":
-    exit(main())
+    sys.exit(main())

Code has been automatically formatted

The code in this PR has been formatted using:

cargo fmt --all
Please pull the latest changes before pushing again:

git pull origin lint-script-patches

@ShaharNaveh could you check coderabbitai reviews?

What i feel is this is a harmless glitch and automatically fixed when upgrading to next version. I'd rather not to bother contributors by that. Let's try how this work. We can disable it again if it reveals actually annoying.

ShaharNaveh added 2 commits February 14, 2026 10:13

Basic script to detect leftover patches

e7dfac3

Add CI step

f0e5abb

Checkout code

7bdefc5

This was referenced Feb 14, 2026

Remove redundant patches from test_codecs.py #7127

Merged

Remove redundant patches from test_pickle.py #7128

Merged

ShaharNaveh marked this pull request as ready for review February 14, 2026 14:26

coderabbitai Bot reviewed Feb 14, 2026

View reviewed changes

Comment thread .github/workflows/ci.yaml Outdated

Comment thread scripts/check_redundant_patches.py

Merge remote-tracking branch 'upstream/main' into lint-script-patches

3fa7178

ShaharNaveh added 2 commits February 15, 2026 10:55

Merge remote-tracking branch 'upstream/main' into lint-script-patches

9e179aa

coderabbit review

0da173a

ShaharNaveh force-pushed the lint-script-patches branch from 800bf35 to 0da173a Compare February 15, 2026 08:56

youknowone approved these changes Feb 15, 2026

View reviewed changes

youknowone merged commit 64088bb into RustPython:main Feb 15, 2026
13 checks passed

youknowone pushed a commit to youknowone/RustPython that referenced this pull request Mar 22, 2026

Add CI step for checking redundant test patches (RustPython#7126 )

42bb3a4

coderabbitai Bot mentioned this pull request Mar 25, 2026

Unify lint CI job #7505

Merged

Add CI step for checking redundant test patches by ShaharNaveh · Pull Request #7126 · RustPython/RustPython · GitHub

Uh oh!

Add CI step for checking redundant test patches - #7126

Add CI step for checking redundant test patches#7126
youknowone merged 6 commits into
RustPython:mainfrom
ShaharNaveh:lint-script-patches

ShaharNaveh commented Feb 14, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Feb 14, 2026 •
edited
Loading

Uh oh!

Uh oh!

youknowone commented Feb 14, 2026

Uh oh!

ShaharNaveh commented Feb 14, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Feb 14, 2026

Uh oh!

youknowone commented Feb 15, 2026

Uh oh!

youknowone commented Feb 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Add CI step for checking redundant test patches by ShaharNaveh · Pull Request #7126 · RustPython/RustPython · GitHub

Uh oh!

Add CI step for checking redundant test patches - #7126

Add CI step for checking redundant test patches#7126youknowone merged 6 commits intoRustPython:mainRustPython/RustPython:mainfrom ShaharNaveh:lint-script-patchesShaharNaveh/RustPython:lint-script-patchesCopy head branch name to clipboard

Conversation

ShaharNaveh commented Feb 14, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Chores

Uh oh!

coderabbitai Bot commented Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

youknowone commented Feb 14, 2026

Uh oh!

ShaharNaveh commented Feb 14, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Feb 14, 2026

Uh oh!

youknowone commented Feb 15, 2026

Uh oh!

youknowone commented Feb 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Add CI step for checking redundant test patches#7126
youknowone merged 6 commits into
RustPython:mainfrom
ShaharNaveh:lint-script-patches

ShaharNaveh commented Feb 14, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Feb 14, 2026 •
edited
Loading