From a35079967a7f12670ca0482a262926a0c4a6fa76 Mon Sep 17 00:00:00 2001 From: Shahar Naveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Sat, 9 May 2026 18:27:51 +0300 Subject: [PATCH] Fix comment confusion bug in `update_lib` (#7802) * Use `unittest.expectedSuccess` to mark failing tests * Fix comment find logic * Add test for it --- Lib/test/test_descr.py | 8 +++---- scripts/update_lib/patch_spec.py | 22 +++++++++++++------ scripts/update_lib/tests/test_patch_spec.py | 24 +++++++++++++++++++++ 3 files changed, 44 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index a15ab20e6..ec60e99f8 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -1860,8 +1860,8 @@ class ClassPropertiesAndMethods(unittest.TestCase): self.assertEqual(b.foo, 3) self.assertEqual(b.__class__, D) - # TODO: RUSTPYTHON; The `expectedFailure` here is from CPython, so this test must fail - # @unittest.expectedFailure + @unittest.expectedSuccess # TODO: RUSTPYTHON; The `expectedFailure` here is from CPython, so this test must fail + @unittest.expectedFailure def test_bad_new(self): self.assertRaises(TypeError, object.__new__) self.assertRaises(TypeError, object.__new__, '') @@ -1908,8 +1908,8 @@ class ClassPropertiesAndMethods(unittest.TestCase): object.__init__(A(3)) self.assertRaises(TypeError, object.__init__, A(3), 5) - # TODO: RUSTPYTHON; The `expectedFailure` here is from CPython, so this test must fail - # @unittest.expectedFailure + @unittest.expectedSuccess # TODO: RUSTPYTHON; The `expectedFailure` here is from CPython, so this test must fail + @unittest.expectedFailure def test_restored_object_new(self): class A(object): def __new__(cls, *args, **kwargs): diff --git a/scripts/update_lib/patch_spec.py b/scripts/update_lib/patch_spec.py index 836d54e72..565d01e3e 100644 --- a/scripts/update_lib/patch_spec.py +++ b/scripts/update_lib/patch_spec.py @@ -190,13 +190,23 @@ class PatchEntry(typing.NamedTuple): if ut_method.has_cond(): cond = ast.unparse(dec_node.args[0]) else: - # Search first on decorator line, then in the line before - for line in lines[dec_node.lineno - 1 : dec_node.lineno - 3 : -1]: - if found := re.search(rf"{COMMENT}.?(.*)", line): - reason = found.group() - break + pattern = re.compile(rf"{COMMENT}.?(.*)") + dec_lineno = dec_node.lineno + + curr_line = lines[dec_lineno - 1] + prev_line = lines[dec_lineno - 2] + + # If we see our comment at the decorator line, take it + if found := pattern.search(curr_line): + reason = found.group() + elif prev_line.strip().startswith("#") and ( + found := pattern.search(prev_line) + ): + # Search the previous line of the decorator, + # only take the comment if the line starts with a `#` + reason = found.group() else: - # Didn't find our `COMMENT` :) + # Didn't find our `COMMENT`, so the patch isn't ours :) continue reason = reason.removeprefix(COMMENT).strip(";:, ") diff --git a/scripts/update_lib/tests/test_patch_spec.py b/scripts/update_lib/tests/test_patch_spec.py index 798bd851b..2598d25b3 100644 --- a/scripts/update_lib/tests/test_patch_spec.py +++ b/scripts/update_lib/tests/test_patch_spec.py @@ -149,6 +149,30 @@ class TestFoo(unittest.TestCase): specs = patches["TestFoo"]["test_one"] self.assertEqual(len(specs), 2) + def test_comment_confusion(self): + """ + Test that we only extract our patches when CPython set one of the UT methods, + that we search for + """ + + code = f""" +class TestFoo(unittest.TestCase): + @unittest.expectedSuccess # {COMMENT}; reason + @unittest.expectedFailure + def test_one(self): + pass + """ + + patches = extract_patches(code) + specs = patches["TestFoo"]["test_one"] + self.assertEqual(len(specs), 1) + + spec = specs[0] + self.assertEqual( + spec, + PatchSpec(ut_method=UtMethod.ExpectedSuccess, cond=None, reason="reason"), + ) + class TestApplyPatches(unittest.TestCase): """Tests for apply_patches function."""