GeezerVison

Walkthrough

Added AST-based utilities to detect super-only methods and remove expected failure decorators. Enhanced test parsing to collect unexpected successes alongside failures. Modified the test runner to track failing tests separately and automatically patch expected failures for tests that now pass.

Changes

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• reactive-firewall

Poem

🐰 Tests once failing now shine bright,
Expected failures fade from sight,
AST paths trim the decorators away,
Hopping forward to a cleaner day! ✨

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

• 📝 Generate docstrings

scripts/auto_mark_test.py (2)

67-74: Class-level mutable defaults are a Python anti-pattern.

tests = [] and unexpected_successes = [] are shared across all instances. While parse_results correctly reinitializes these, the pattern is fragile. Consider using a dataclass or __init__ method.

♻️ Suggested refactor using dataclass

+from dataclasses import dataclass, field
+
+@dataclass
 class TestResult:
-    tests_result: str = ""
-    tests = []
-    unexpected_successes = []  # Tests that passed but were marked as expectedFailure
-    stdout = ""
+    tests_result: str = ""
+    tests: list = field(default_factory=list)
+    unexpected_successes: list = field(default_factory=list)
+    stdout: str = ""
 
     def __str__(self):
         return f"TestResult(tests_result={self.tests_result},tests={len(self.tests)},unexpected_successes={len(self.unexpected_successes)})"

If using dataclass, you can also remove the explicit initializations at lines 80-81 since the factory creates fresh lists.

---

304-338: LGTM!

The flow correctly handles both marking new failures and removing expected failures from tests that now pass. The ordering (remove first, then apply new patches) is sensible.

Minor style nit: Line 306's condition could use in for consistency with line 100:

if test.result in ("fail", "error"):

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.778Z
Learning: Applies to Lib/test/**/*.py : Use `unittest.skip("TODO: RustPython <reason>")` or `unittest.expectedFailure` with `# TODO: RUSTPYTHON <reason>` comment when marking tests in Lib/ that cannot run

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.778Z
Learning: Applies to **/test*.py : Only remove `unittest.expectedFailure` decorators and upper TODO comments from tests when tests actually pass, or add these decorators when tests cannot be fixed

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.778Z
Learning: Applies to **/test*.py : When a test cannot pass due to missing language features, keep it as expectedFailure and document the reason instead of modifying the test

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.778Z
Learning: Applies to **/test*.py : Only remove `unittest.expectedFailure` decorators and upper TODO comments from tests when tests actually pass, or add these decorators when tests cannot be fixed

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.778Z
Learning: Applies to **/test*.py : NEVER comment out or delete any test code lines except for removing `unittest.expectedFailure` decorators and upper TODO comments

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 6089
File: scripts/lib_updater.py:198-202
Timestamp: 2025-09-07T05:38:31.690Z
Learning: In scripts/lib_updater.py, the iter_patches function intentionally does not handle SyntaxError from ast.parse(contents). The author confirmed this behavior is fine and intended - the tool should fail fast on unparseable files rather than silently skip processing.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

• X
• Mastodon
• Reddit
• LinkedIn