Skip to content

Commit babc3c6

Browse files
youknowonegithub-actions[bot]
youknowone
and
github-actions[bot]
authored
Make auto-mark output deterministic and fix blank line leak (#6957)
* 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. * deps code * auto-mark handles crash better * Auto-format: ruff format --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 100b870 commit babc3c6

File tree

4 files changed

+206
-5
lines changed

4 files changed

+206
-5
lines changed

‎scripts/update_lib/cmd_auto_mark.py‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ def build_patches(
350350
"""Convert failing tests to patch format."""
351351
patches = {}
352352
error_messages = error_messages or {}
353-
for class_name, method_name in test_parts_set:
353+
for class_name, method_name in sorted(test_parts_set):
354354
if class_name not in patches:
355355
patches[class_name] = {}
356356
reason = error_messages.get((class_name, method_name), "")
@@ -401,6 +401,9 @@ def _method_removal_range(
401401
and COMMENT in lines[first - 1]
402402
):
403403
first -= 1
404+
# Also remove a preceding blank line to avoid double-blanks after removal
405+
if first > 0 and not lines[first - 1].strip():
406+
first -= 1
404407
return range(first, func_node.end_lineno)
405408

406409

@@ -753,7 +756,11 @@ def auto_mark_file(
753756
results = run_test(test_name, skip_build=skip_build)
754757

755758
# Check if test run failed entirely (e.g., import error, crash)
756-
if not results.tests_result:
759+
if (
760+
not results.tests_result
761+
and not results.tests
762+
and not results.unexpected_successes
763+
):
757764
raise TestRunError(
758765
f"Test run failed for {test_name}. "
759766
f"Output: {results.stdout[-500:] if results.stdout else '(no output)'}"
@@ -870,7 +877,11 @@ def auto_mark_directory(
870877
results = run_test(test_name, skip_build=skip_build)
871878

872879
# Check if test run failed entirely (e.g., import error, crash)
873-
if not results.tests_result:
880+
if (
881+
not results.tests_result
882+
and not results.tests
883+
and not results.unexpected_successes
884+
):
874885
raise TestRunError(
875886
f"Test run failed for {test_name}. "
876887
f"Output: {results.stdout[-500:] if results.stdout else '(no output)'}"

‎scripts/update_lib/deps.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,11 @@ def clear_import_graph_caches() -> None:
503503
"test_descrtut.py",
504504
],
505505
},
506+
"code": {
507+
"test": [
508+
"test_code_module.py",
509+
],
510+
},
506511
"contextlib": {
507512
"test": [
508513
"test_contextlib.py",

‎scripts/update_lib/patch_spec.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,13 +282,13 @@ def _iter_patch_lines(
282282
yield (lineno - 1, textwrap.indent(patch_lines, indent))
283283

284284
# Phase 2: Iterate and mark inherited tests
285-
for cls_name, tests in patches.items():
285+
for cls_name, tests in sorted(patches.items()):
286286
lineno = cache.get(cls_name)
287287
if not lineno:
288288
print(f"WARNING: {cls_name} does not exist in remote file", file=sys.stderr)
289289
continue
290290

291-
for test_name, specs in tests.items():
291+
for test_name, specs in sorted(tests.items()):
292292
decorators = "\n".join(spec.as_decorator() for spec in specs)
293293
# Check current class and ancestors for async method
294294
is_async = False

‎scripts/update_lib/tests/test_auto_mark.py‎

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
11
"""Tests for auto_mark.py - test result parsing and auto-marking."""
22

33
import ast
4+
import pathlib
45
import subprocess
6+
import tempfile
57
import unittest
8+
from unittest import mock
69

710
from update_lib.cmd_auto_mark import (
811
Test,
912
TestResult,
13+
TestRunError,
1014
_expand_stripped_to_children,
1115
_is_super_call_only,
1216
apply_test_changes,
17+
auto_mark_directory,
18+
auto_mark_file,
1319
collect_test_changes,
1420
extract_test_methods,
1521
parse_results,
@@ -94,6 +100,34 @@ def test_parse_tests_result(self):
94100
result = parse_results(_make_result("== Tests result: FAILURE ==\n"))
95101
self.assertEqual(result.tests_result, "FAILURE")
96102

103+
def test_parse_crashed_run_no_tests_result(self):
104+
"""Test results are still parsed when the runner crashes (no Tests result line)."""
105+
stdout = """\
106+
Run 1 test sequentially in a single process
107+
0:00:00 [1/1] test_ast
108+
test_foo (test.test_ast.test_ast.TestA.test_foo) ... FAIL
109+
test_bar (test.test_ast.test_ast.TestA.test_bar) ... ok
110+
test_baz (test.test_ast.test_ast.TestB.test_baz) ... ERROR
111+
"""
112+
result = parse_results(_make_result(stdout))
113+
self.assertEqual(result.tests_result, "")
114+
self.assertEqual(len(result.tests), 2)
115+
names = {t.name for t in result.tests}
116+
self.assertIn("test_foo", names)
117+
self.assertIn("test_baz", names)
118+
119+
def test_parse_crashed_run_has_unexpected_success(self):
120+
"""Unexpected successes are parsed even without Tests result line."""
121+
stdout = """\
122+
Run 1 test sequentially in a single process
123+
0:00:00 [1/1] test_ast
124+
test_foo (test.test_ast.test_ast.TestA.test_foo) ... unexpected success
125+
UNEXPECTED SUCCESS: test_foo (test.test_ast.test_ast.TestA.test_foo)
126+
"""
127+
result = parse_results(_make_result(stdout))
128+
self.assertEqual(result.tests_result, "")
129+
self.assertEqual(len(result.unexpected_successes), 1)
130+
97131
def test_parse_error_messages(self):
98132
"""Single and multiple error messages are parsed from tracebacks."""
99133
stdout = """\
@@ -747,5 +781,156 @@ async def test_one(self):
747781
)
748782

749783

784+
class TestAutoMarkFileWithCrashedRun(unittest.TestCase):
785+
"""auto_mark_file should process partial results when test runner crashes."""
786+
787+
CRASHED_STDOUT = """\
788+
Run 1 test sequentially in a single process
789+
0:00:00 [1/1] test_example
790+
test_foo (test.test_example.TestA.test_foo) ... FAIL
791+
test_bar (test.test_example.TestA.test_bar) ... ok
792+
======================================================================
793+
FAIL: test_foo (test.test_example.TestA.test_foo)
794+
----------------------------------------------------------------------
795+
Traceback (most recent call last):
796+
File "test.py", line 10, in test_foo
797+
self.assertEqual(1, 2)
798+
AssertionError: 1 != 2
799+
"""
800+
801+
def test_auto_mark_file_crashed_run(self):
802+
"""auto_mark_file processes results even when tests_result is empty (crash)."""
803+
test_code = f"""import unittest
804+
805+
class TestA(unittest.TestCase):
806+
def test_foo(self):
807+
pass
808+
809+
def test_bar(self):
810+
pass
811+
"""
812+
with tempfile.TemporaryDirectory() as tmpdir:
813+
test_file = pathlib.Path(tmpdir) / "test_example.py"
814+
test_file.write_text(test_code)
815+
816+
mock_result = TestResult()
817+
mock_result.tests_result = ""
818+
mock_result.tests = [
819+
Test(
820+
name="test_foo",
821+
path="test.test_example.TestA.test_foo",
822+
result="fail",
823+
error_message="AssertionError: 1 != 2",
824+
),
825+
]
826+
827+
with mock.patch(
828+
"update_lib.cmd_auto_mark.run_test", return_value=mock_result
829+
):
830+
added, removed, regressions = auto_mark_file(
831+
test_file, mark_failure=True, verbose=False
832+
)
833+
834+
self.assertEqual(added, 1)
835+
contents = test_file.read_text()
836+
self.assertIn("expectedFailure", contents)
837+
838+
def test_auto_mark_file_no_results_at_all_raises(self):
839+
"""auto_mark_file raises TestRunError when there are zero parsed results."""
840+
test_code = """import unittest
841+
842+
class TestA(unittest.TestCase):
843+
def test_foo(self):
844+
pass
845+
"""
846+
with tempfile.TemporaryDirectory() as tmpdir:
847+
test_file = pathlib.Path(tmpdir) / "test_example.py"
848+
test_file.write_text(test_code)
849+
850+
mock_result = TestResult()
851+
mock_result.tests_result = ""
852+
mock_result.tests = []
853+
mock_result.stdout = "some crash output"
854+
855+
with mock.patch(
856+
"update_lib.cmd_auto_mark.run_test", return_value=mock_result
857+
):
858+
with self.assertRaises(TestRunError):
859+
auto_mark_file(test_file, verbose=False)
860+
861+
862+
class TestAutoMarkDirectoryWithCrashedRun(unittest.TestCase):
863+
"""auto_mark_directory should process partial results when test runner crashes."""
864+
865+
def test_auto_mark_directory_crashed_run(self):
866+
"""auto_mark_directory processes results even when tests_result is empty."""
867+
test_code = f"""import unittest
868+
869+
class TestA(unittest.TestCase):
870+
def test_foo(self):
871+
pass
872+
"""
873+
with tempfile.TemporaryDirectory() as tmpdir:
874+
test_dir = pathlib.Path(tmpdir) / "test_example"
875+
test_dir.mkdir()
876+
test_file = test_dir / "test_sub.py"
877+
test_file.write_text(test_code)
878+
879+
mock_result = TestResult()
880+
mock_result.tests_result = ""
881+
mock_result.tests = [
882+
Test(
883+
name="test_foo",
884+
path="test.test_example.test_sub.TestA.test_foo",
885+
result="fail",
886+
error_message="AssertionError: oops",
887+
),
888+
]
889+
890+
with (
891+
mock.patch(
892+
"update_lib.cmd_auto_mark.run_test", return_value=mock_result
893+
),
894+
mock.patch(
895+
"update_lib.cmd_auto_mark.get_test_module_name",
896+
side_effect=lambda p: (
897+
"test_example" if p == test_dir else "test_example.test_sub"
898+
),
899+
),
900+
):
901+
added, removed, regressions = auto_mark_directory(
902+
test_dir, mark_failure=True, verbose=False
903+
)
904+
905+
self.assertEqual(added, 1)
906+
contents = test_file.read_text()
907+
self.assertIn("expectedFailure", contents)
908+
909+
def test_auto_mark_directory_no_results_raises(self):
910+
"""auto_mark_directory raises TestRunError when zero results."""
911+
with tempfile.TemporaryDirectory() as tmpdir:
912+
test_dir = pathlib.Path(tmpdir) / "test_example"
913+
test_dir.mkdir()
914+
test_file = test_dir / "test_sub.py"
915+
test_file.write_text("import unittest\n")
916+
917+
mock_result = TestResult()
918+
mock_result.tests_result = ""
919+
mock_result.tests = []
920+
mock_result.stdout = "crash"
921+
922+
with (
923+
mock.patch(
924+
"update_lib.cmd_auto_mark.run_test", return_value=mock_result
925+
),
926+
mock.patch(
927+
"update_lib.cmd_auto_mark.get_test_module_name",
928+
return_value="test_example",
929+
),
930+
):
931+
with self.assertRaises(TestRunError):
932+
auto_mark_directory(test_dir, verbose=False)
933+
934+
750935
if __name__ == "__main__":
751936
unittest.main()

0 commit comments

Comments
 (0)