Make auto-mark output deterministic and fix blank line leak #6957
Conversation
📝 WalkthroughWalkthroughThis 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 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this 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.
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.
|
Code has been automatically formatted The code in this PR has been formatted using:
git pull origin auto-mark |
There was a problem hiding this 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.
|
|
||
| def test_auto_mark_file_crashed_run(self): | ||
| """auto_mark_file processes results even when tests_result is empty (crash).""" | ||
| test_code = f"""import unittest |
There was a problem hiding this comment.
Unnecessary f-string prefix.
The string literal has no placeholders, so the f prefix is unnecessary. This was flagged by static analysis (F541).
🔧 Proposed fix
- test_code = f"""import unittest
+ test_code = """import unittest🧰 Tools
🪛 Flake8 (7.3.0)
[error] 803-803: f-string is missing placeholders
(F541)
🤖 Prompt for AI Agents
In `@scripts/update_lib/tests/test_auto_mark.py` at 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.
|
|
||
| def test_auto_mark_directory_crashed_run(self): | ||
| """auto_mark_directory processes results even when tests_result is empty.""" | ||
| test_code = f"""import unittest |
There was a problem hiding this comment.
Unnecessary f-string prefix.
Same issue as above - the string has no placeholders.
🔧 Proposed fix
- test_code = f"""import unittest
+ test_code = """import unittest📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test_code = f"""import unittest | |
| test_code = """import unittest |
🧰 Tools
🪛 Flake8 (7.3.0)
[error] 867-867: f-string is missing placeholders
(F541)
🤖 Prompt for AI Agents
In `@scripts/update_lib/tests/test_auto_mark.py` at 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.
2 participants
also mapping
codeandtest_code_moduleSummary by CodeRabbit
Bug Fixes
Chores
Tests