◐ Shell
reader mode source ↗
Skip to content

Make auto-mark output deterministic and fix blank line leak#6957

Merged
youknowone merged 4 commits into
RustPython:mainfrom
youknowone:auto-mark
Feb 2, 2026
Merged

Make auto-mark output deterministic and fix blank line leak#6957
youknowone merged 4 commits into
RustPython:mainfrom
youknowone:auto-mark

Conversation

@youknowone

@youknowone youknowone commented Feb 2, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

also mapping code and test_code_module

Summary by CodeRabbit

  • Bug Fixes

    • Prevented extra blank lines after removals; refined failure detection so partial/crashed test runs are handled correctly.

  • Chores

    • Made patch and processing order deterministic for consistent, reproducible outputs.
    • Added an additional test dependency mapping to improve test organization.

  • Tests

    • Added extensive unit tests covering crashed/partial test-run scenarios and related handling.

@coderabbitai

coderabbitai Bot commented Feb 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR makes patch generation deterministic by sorting test parts and class/test names, tightens method-removal to also remove a preceding blank line, broadens the definition of a failed test run to accept partial results, adds tests for crashed/partial runs, and adds a new code test dependency mapping.

Changes

Cohort / File(s) Summary
Auto-mark and patch ordering
scripts/update_lib/cmd_auto_mark.py, scripts/update_lib/patch_spec.py		 Sorts test parts and class/test names to produce deterministic patch ordering. Adjusts _method_removal_range to remove a preceding blank line when deleting a method. Broadens failure-detection logic in auto_mark_file and auto_mark_directory to allow partial results.
Tests for crashed/partial runs
scripts/update_lib/tests/test_auto_mark.py		 Adds unit tests covering crashed test runs, unexpected successes, partial parsed results, and error raising when zero results are parsed; includes mocking of test-run and module-name helpers.
Dependency mappings
scripts/update_lib/deps.py		 Adds a code entry to DEPENDENCIES with test: ["test_code_module.py"] to include an irregular test dependency for code-related tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

skip:ci

Suggested reviewers

  • ShaharNaveh
  • moreal

Poem

🐰 I hopped through diffs with tidy feet,

Sorted tests so patches meet,
I nibbled blank lines, one by one,
Kept partial runs when tests were done,
A small bright patch — now work's more neat.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main changes: making auto-mark output deterministic and fixing a blank line issue that affects the output.

✏️ 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

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.

@youknowone youknowone marked this pull request as ready for review February 2, 2026 03:52
coderabbitai[bot]
coderabbitai Bot reviewed Feb 2, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hide comment

Actionable comments posted: 1

🤖 Fix all issues with AI agents 
In `@scripts/update_lib/deps.py`:
- Around line 506-510: The explicit test list under the "code" key currently
only includes "test_code_module.py", which excludes the default test_code.py;
update the "code": {"test": [...]} entry to also include "test_code.py"
alongside "test_code_module.py" so both Lib/test/test_code.py and
Lib/test/test_code_module.py are run during migration.

@youknowone
Make auto-mark output deterministic and fix blank line leak …
5d81ce2 
Sort set iteration in build_patches and dict iteration in
_iter_patch_lines Phase 2 so expectedFailure markers are
always added in alphabetical order.

Include preceding blank line in _method_removal_range so
removing a super-call override doesn't leave behind the
blank line that was added with the method.
@youknowone
deps code
2d05112
@youknowone
auto-mark handles crash better
f63f667
@github-actions

github-actions Bot commented Feb 2, 2026

Copy link
Copy Markdown
Contributor

Code has been automatically formatted

The code in this PR has been formatted using:

  • ruff format
    Please pull the latest changes before pushing again:
git pull origin auto-mark

coderabbitai[bot]
coderabbitai Bot reviewed Feb 2, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hide comment

Actionable comments posted: 2

🤖 Fix all issues with AI agents 
In `@scripts/update_lib/tests/test_auto_mark.py`:
- Line 803: The test string assigned to the variable test_code uses an
unnecessary f-string prefix (f"""...""") even though there are no placeholders;
update the assignment in tests/test_auto_mark.py by removing the leading "f" so
the literal is a plain triple-quoted string (change test_code = f"""import
unittest to test_code = """import unittest), ensuring any other nearby
multi-line test_code usages follow the same pattern.
- Line 867: The test constructs a multi-line string assigned to test_code using
an unnecessary f-string prefix; update the assignment of test_code in
test_auto_mark.py (the test_code variable) by removing the leading "f" so it
becomes a plain string literal (no interpolation) to match other similar test
strings and avoid misleading formatting.

Hide details View details @youknowone youknowone merged commit babc3c6 into RustPython:main Feb 2, 2026
12 of 13 checks passed
@youknowone youknowone deleted the auto-mark branch February 2, 2026 04:53
@coderabbitai coderabbitai Bot mentioned this pull request Feb 2, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@fanninpm fanninpm Awaiting requested review from fanninpm

@ShaharNaveh ShaharNaveh Awaiting requested review from ShaharNaveh

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@youknowone @fanninpm