From 5d81ce290d2908c9a712f58ffa975d3c2fd8ecb8 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Mon, 2 Feb 2026 11:42:23 +0900 Subject: [PATCH 1/4] Make auto-mark output deterministic and fix blank line leak 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. --- scripts/update_lib/cmd_auto_mark.py | 5 ++++- scripts/update_lib/patch_spec.py | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/scripts/update_lib/cmd_auto_mark.py b/scripts/update_lib/cmd_auto_mark.py index a62b2795bba..007eab39c7c 100644 --- a/scripts/update_lib/cmd_auto_mark.py +++ b/scripts/update_lib/cmd_auto_mark.py @@ -350,7 +350,7 @@ def build_patches( """Convert failing tests to patch format.""" patches = {} error_messages = error_messages or {} - for class_name, method_name in test_parts_set: + for class_name, method_name in sorted(test_parts_set): if class_name not in patches: patches[class_name] = {} reason = error_messages.get((class_name, method_name), "") @@ -401,6 +401,9 @@ def _method_removal_range( and COMMENT in lines[first - 1] ): first -= 1 + # Also remove a preceding blank line to avoid double-blanks after removal + if first > 0 and not lines[first - 1].strip(): + first -= 1 return range(first, func_node.end_lineno) diff --git a/scripts/update_lib/patch_spec.py b/scripts/update_lib/patch_spec.py index d35a6351ee9..d27d2e22fa7 100644 --- a/scripts/update_lib/patch_spec.py +++ b/scripts/update_lib/patch_spec.py @@ -282,13 +282,13 @@ def _iter_patch_lines( yield (lineno - 1, textwrap.indent(patch_lines, indent)) # Phase 2: Iterate and mark inherited tests - for cls_name, tests in patches.items(): + for cls_name, tests in sorted(patches.items()): lineno = cache.get(cls_name) if not lineno: print(f"WARNING: {cls_name} does not exist in remote file", file=sys.stderr) continue - for test_name, specs in tests.items(): + for test_name, specs in sorted(tests.items()): decorators = "\n".join(spec.as_decorator() for spec in specs) # Check current class and ancestors for async method is_async = False From 2d05112a4ffc71d48ea0c639b0fa0cef847d3b6a Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Mon, 2 Feb 2026 11:48:23 +0900 Subject: [PATCH 2/4] deps code --- scripts/update_lib/deps.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/update_lib/deps.py b/scripts/update_lib/deps.py index 33db418e0c3..7acffe88d0b 100644 --- a/scripts/update_lib/deps.py +++ b/scripts/update_lib/deps.py @@ -503,6 +503,11 @@ def clear_import_graph_caches() -> None: "test_descrtut.py", ], }, + "code": { + "test": [ + "test_code_module.py", + ], + }, "contextlib": { "test": [ "test_contextlib.py", From f63f667ff4df04f4eafa25efc674aa4e3eb19ceb Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Mon, 2 Feb 2026 13:23:39 +0900 Subject: [PATCH 3/4] auto-mark handles crash better --- scripts/update_lib/cmd_auto_mark.py | 4 +- scripts/update_lib/tests/test_auto_mark.py | 181 +++++++++++++++++++++ 2 files changed, 183 insertions(+), 2 deletions(-) diff --git a/scripts/update_lib/cmd_auto_mark.py b/scripts/update_lib/cmd_auto_mark.py index 007eab39c7c..cbdebc92c9f 100644 --- a/scripts/update_lib/cmd_auto_mark.py +++ b/scripts/update_lib/cmd_auto_mark.py @@ -756,7 +756,7 @@ def auto_mark_file( results = run_test(test_name, skip_build=skip_build) # Check if test run failed entirely (e.g., import error, crash) - if not results.tests_result: + if not results.tests_result and not results.tests and not results.unexpected_successes: raise TestRunError( f"Test run failed for {test_name}. " f"Output: {results.stdout[-500:] if results.stdout else '(no output)'}" @@ -873,7 +873,7 @@ def auto_mark_directory( results = run_test(test_name, skip_build=skip_build) # Check if test run failed entirely (e.g., import error, crash) - if not results.tests_result: + if not results.tests_result and not results.tests and not results.unexpected_successes: raise TestRunError( f"Test run failed for {test_name}. " f"Output: {results.stdout[-500:] if results.stdout else '(no output)'}" diff --git a/scripts/update_lib/tests/test_auto_mark.py b/scripts/update_lib/tests/test_auto_mark.py index 15a80e49e44..84e98784f91 100644 --- a/scripts/update_lib/tests/test_auto_mark.py +++ b/scripts/update_lib/tests/test_auto_mark.py @@ -1,15 +1,21 @@ """Tests for auto_mark.py - test result parsing and auto-marking.""" import ast +import pathlib import subprocess +import tempfile import unittest +from unittest import mock from update_lib.cmd_auto_mark import ( Test, TestResult, + TestRunError, _expand_stripped_to_children, _is_super_call_only, apply_test_changes, + auto_mark_directory, + auto_mark_file, collect_test_changes, extract_test_methods, parse_results, @@ -94,6 +100,34 @@ def test_parse_tests_result(self): result = parse_results(_make_result("== Tests result: FAILURE ==\n")) self.assertEqual(result.tests_result, "FAILURE") + def test_parse_crashed_run_no_tests_result(self): + """Test results are still parsed when the runner crashes (no Tests result line).""" + stdout = """\ +Run 1 test sequentially in a single process +0:00:00 [1/1] test_ast +test_foo (test.test_ast.test_ast.TestA.test_foo) ... FAIL +test_bar (test.test_ast.test_ast.TestA.test_bar) ... ok +test_baz (test.test_ast.test_ast.TestB.test_baz) ... ERROR +""" + result = parse_results(_make_result(stdout)) + self.assertEqual(result.tests_result, "") + self.assertEqual(len(result.tests), 2) + names = {t.name for t in result.tests} + self.assertIn("test_foo", names) + self.assertIn("test_baz", names) + + def test_parse_crashed_run_has_unexpected_success(self): + """Unexpected successes are parsed even without Tests result line.""" + stdout = """\ +Run 1 test sequentially in a single process +0:00:00 [1/1] test_ast +test_foo (test.test_ast.test_ast.TestA.test_foo) ... unexpected success +UNEXPECTED SUCCESS: test_foo (test.test_ast.test_ast.TestA.test_foo) +""" + result = parse_results(_make_result(stdout)) + self.assertEqual(result.tests_result, "") + self.assertEqual(len(result.unexpected_successes), 1) + def test_parse_error_messages(self): """Single and multiple error messages are parsed from tracebacks.""" stdout = """\ @@ -747,5 +781,152 @@ async def test_one(self): ) +class TestAutoMarkFileWithCrashedRun(unittest.TestCase): + """auto_mark_file should process partial results when test runner crashes.""" + + CRASHED_STDOUT = """\ +Run 1 test sequentially in a single process +0:00:00 [1/1] test_example +test_foo (test.test_example.TestA.test_foo) ... FAIL +test_bar (test.test_example.TestA.test_bar) ... ok +====================================================================== +FAIL: test_foo (test.test_example.TestA.test_foo) +---------------------------------------------------------------------- +Traceback (most recent call last): + File "test.py", line 10, in test_foo + self.assertEqual(1, 2) +AssertionError: 1 != 2 +""" + + 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 + +class TestA(unittest.TestCase): + def test_foo(self): + pass + + def test_bar(self): + pass +""" + with tempfile.TemporaryDirectory() as tmpdir: + test_file = pathlib.Path(tmpdir) / "test_example.py" + test_file.write_text(test_code) + + mock_result = TestResult() + mock_result.tests_result = "" + mock_result.tests = [ + Test( + name="test_foo", + path="test.test_example.TestA.test_foo", + result="fail", + error_message="AssertionError: 1 != 2", + ), + ] + + with mock.patch( + "update_lib.cmd_auto_mark.run_test", return_value=mock_result + ): + added, removed, regressions = auto_mark_file( + test_file, mark_failure=True, verbose=False + ) + + self.assertEqual(added, 1) + contents = test_file.read_text() + self.assertIn("expectedFailure", contents) + + def test_auto_mark_file_no_results_at_all_raises(self): + """auto_mark_file raises TestRunError when there are zero parsed results.""" + test_code = """import unittest + +class TestA(unittest.TestCase): + def test_foo(self): + pass +""" + with tempfile.TemporaryDirectory() as tmpdir: + test_file = pathlib.Path(tmpdir) / "test_example.py" + test_file.write_text(test_code) + + mock_result = TestResult() + mock_result.tests_result = "" + mock_result.tests = [] + mock_result.stdout = "some crash output" + + with mock.patch( + "update_lib.cmd_auto_mark.run_test", return_value=mock_result + ): + with self.assertRaises(TestRunError): + auto_mark_file(test_file, verbose=False) + + +class TestAutoMarkDirectoryWithCrashedRun(unittest.TestCase): + """auto_mark_directory should process partial results when test runner crashes.""" + + def test_auto_mark_directory_crashed_run(self): + """auto_mark_directory processes results even when tests_result is empty.""" + test_code = f"""import unittest + +class TestA(unittest.TestCase): + def test_foo(self): + pass +""" + with tempfile.TemporaryDirectory() as tmpdir: + test_dir = pathlib.Path(tmpdir) / "test_example" + test_dir.mkdir() + test_file = test_dir / "test_sub.py" + test_file.write_text(test_code) + + mock_result = TestResult() + mock_result.tests_result = "" + mock_result.tests = [ + Test( + name="test_foo", + path="test.test_example.test_sub.TestA.test_foo", + result="fail", + error_message="AssertionError: oops", + ), + ] + + with mock.patch( + "update_lib.cmd_auto_mark.run_test", return_value=mock_result + ), mock.patch( + "update_lib.cmd_auto_mark.get_test_module_name", + side_effect=lambda p: ( + "test_example" + if p == test_dir + else "test_example.test_sub" + ), + ): + added, removed, regressions = auto_mark_directory( + test_dir, mark_failure=True, verbose=False + ) + + self.assertEqual(added, 1) + contents = test_file.read_text() + self.assertIn("expectedFailure", contents) + + def test_auto_mark_directory_no_results_raises(self): + """auto_mark_directory raises TestRunError when zero results.""" + with tempfile.TemporaryDirectory() as tmpdir: + test_dir = pathlib.Path(tmpdir) / "test_example" + test_dir.mkdir() + test_file = test_dir / "test_sub.py" + test_file.write_text("import unittest\n") + + mock_result = TestResult() + mock_result.tests_result = "" + mock_result.tests = [] + mock_result.stdout = "crash" + + with mock.patch( + "update_lib.cmd_auto_mark.run_test", return_value=mock_result + ), mock.patch( + "update_lib.cmd_auto_mark.get_test_module_name", + return_value="test_example", + ): + with self.assertRaises(TestRunError): + auto_mark_directory(test_dir, verbose=False) + + if __name__ == "__main__": unittest.main() From 6ff5f8dc8b812b8ad41eefbd03fdf2807b64ffb0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 2 Feb 2026 04:26:27 +0000 Subject: [PATCH 4/4] Auto-format: ruff format --- scripts/update_lib/cmd_auto_mark.py | 12 +++++++-- scripts/update_lib/tests/test_auto_mark.py | 30 ++++++++++++---------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/scripts/update_lib/cmd_auto_mark.py b/scripts/update_lib/cmd_auto_mark.py index cbdebc92c9f..94cda308c2d 100644 --- a/scripts/update_lib/cmd_auto_mark.py +++ b/scripts/update_lib/cmd_auto_mark.py @@ -756,7 +756,11 @@ def auto_mark_file( results = run_test(test_name, skip_build=skip_build) # Check if test run failed entirely (e.g., import error, crash) - if not results.tests_result and not results.tests and not results.unexpected_successes: + if ( + not results.tests_result + and not results.tests + and not results.unexpected_successes + ): raise TestRunError( f"Test run failed for {test_name}. " f"Output: {results.stdout[-500:] if results.stdout else '(no output)'}" @@ -873,7 +877,11 @@ def auto_mark_directory( results = run_test(test_name, skip_build=skip_build) # Check if test run failed entirely (e.g., import error, crash) - if not results.tests_result and not results.tests and not results.unexpected_successes: + if ( + not results.tests_result + and not results.tests + and not results.unexpected_successes + ): raise TestRunError( f"Test run failed for {test_name}. " f"Output: {results.stdout[-500:] if results.stdout else '(no output)'}" diff --git a/scripts/update_lib/tests/test_auto_mark.py b/scripts/update_lib/tests/test_auto_mark.py index 84e98784f91..36eb95a3d9c 100644 --- a/scripts/update_lib/tests/test_auto_mark.py +++ b/scripts/update_lib/tests/test_auto_mark.py @@ -887,14 +887,15 @@ def test_foo(self): ), ] - with mock.patch( - "update_lib.cmd_auto_mark.run_test", return_value=mock_result - ), mock.patch( - "update_lib.cmd_auto_mark.get_test_module_name", - side_effect=lambda p: ( - "test_example" - if p == test_dir - else "test_example.test_sub" + with ( + mock.patch( + "update_lib.cmd_auto_mark.run_test", return_value=mock_result + ), + mock.patch( + "update_lib.cmd_auto_mark.get_test_module_name", + side_effect=lambda p: ( + "test_example" if p == test_dir else "test_example.test_sub" + ), ), ): added, removed, regressions = auto_mark_directory( @@ -918,11 +919,14 @@ def test_auto_mark_directory_no_results_raises(self): mock_result.tests = [] mock_result.stdout = "crash" - with mock.patch( - "update_lib.cmd_auto_mark.run_test", return_value=mock_result - ), mock.patch( - "update_lib.cmd_auto_mark.get_test_module_name", - return_value="test_example", + with ( + mock.patch( + "update_lib.cmd_auto_mark.run_test", return_value=mock_result + ), + mock.patch( + "update_lib.cmd_auto_mark.get_test_module_name", + return_value="test_example", + ), ): with self.assertRaises(TestRunError): auto_mark_directory(test_dir, verbose=False)