GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Add CI step for checking redundant test patches by ShaharNaveh · Pull Request #7126 · RustPython/RustPython

Conversation

ShaharNaveh commented Feb 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Summary by CodeRabbit

Chores

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ShaharNaveh added 2 commits

February 14, 2026 10:13


          Basic script to detect leftover patches

e7dfac3


          Add CI step

f0e5abb

coderabbitai Bot commented Feb 14, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

📝 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.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          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

youknowone commented Feb 14, 2026

Copy link

Copy Markdown

Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ShaharNaveh commented Feb 14, 2026

Copy link

Copy Markdown

Contributor Author

@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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ShaharNaveh marked this pull request as ready for review

February 14, 2026 14:26

coderabbitai Bot reviewed

Feb 14, 2026

View reviewed changes

coderabbitai Bot left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/[email protected]"; keep the first
checkout that precedes the "Check for redundant test patches" step and delete
the second "uses: actions/[email protected]" 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())

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

.github/workflows/ci.yaml Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

scripts/check_redundant_patches.py Resolved

Uh oh!

There was an error while loading. Please reload this page.


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

3fa7178

github-actions Bot commented Feb 14, 2026

Copy link

Copy Markdown

Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

youknowone commented Feb 15, 2026

Copy link

Copy Markdown

Member

@ShaharNaveh could you check coderabbitai reviews?

ShaharNaveh reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

youknowone commented Feb 15, 2026

Copy link

Copy Markdown

Member

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 reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

Uh oh!

There was an error while loading. Please reload this page.

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

youknowone pushed a commit that referenced this pull request

Sep 16, 2026


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

1d0c813

Reviewers

coderabbitai[bot]

coderabbitai[bot] left review comments

youknowone

youknowone approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

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.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

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

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ShaharNaveh commented Feb 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot commented Feb 14, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.