From bd73cda30a2cfde82c15bf58e7d3419d4c433e07 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sat, 28 Sep 2024 09:57:22 +0300 Subject: [PATCH 1/3] gh-124176: `create_autospec` must not change how dataclass defaults are mocked --- Lib/test/test_unittest/testmock/testhelpers.py | 15 +++++++++++++++ Lib/unittest/mock.py | 8 +++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_unittest/testmock/testhelpers.py b/Lib/test/test_unittest/testmock/testhelpers.py index f260769eb8c35e..246bfe8f200324 100644 --- a/Lib/test/test_unittest/testmock/testhelpers.py +++ b/Lib/test/test_unittest/testmock/testhelpers.py @@ -1107,6 +1107,21 @@ class WithNonFields: with self.assertRaisesRegex(AttributeError, msg): mock.b + def test_dataclass_with_wider_default(self): + # If field defines an actual default, we don't need to change + # the default type. Since this is how it used to work before #124176 + @dataclass + class WithWiderDefault: + narrow_default: int | None = field(default=30) + + for mock in [ + create_autospec(WithWiderDefault, instance=True), + create_autospec(WithWiderDefault()), + ]: + with self.subTest(mock=mock): + self.assertIs(mock.narrow_default.__class__, int) + + class TestCallList(unittest.TestCase): def test_args_list_contains_call_list(self): diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 21ca061a77c26f..c558d22631e894 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -2758,13 +2758,15 @@ def create_autospec(spec, spec_set=False, instance=False, _parent=None, f'[object={spec!r}]') is_async_func = _is_async_func(spec) - entries = [(entry, _missing) for entry in dir(spec)] + base_entries = {entry: _missing for entry in dir(spec)} if is_type and instance and is_dataclass(spec): dataclass_fields = fields(spec) - entries.extend((f.name, f.type) for f in dataclass_fields) + entries = {f.name: f.type for f in dataclass_fields} + entries.update(base_entries) _kwargs = {'spec': [f.name for f in dataclass_fields]} else: _kwargs = {'spec': spec} + entries = base_entries if spec_set: _kwargs = {'spec_set': spec} @@ -2822,7 +2824,7 @@ def create_autospec(spec, spec_set=False, instance=False, _parent=None, _name='()', _parent=mock, wraps=wrapped) - for entry, original in entries: + for entry, original in entries.items(): if _is_magic(entry): # MagicMock already does the useful magic methods for us continue From 81b4e0022e28b47b275ab6197d36f66323af0f47 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Fri, 4 Oct 2024 10:26:10 +0300 Subject: [PATCH 2/3] Add more tests --- Lib/test/test_unittest/testmock/testhelpers.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Lib/test/test_unittest/testmock/testhelpers.py b/Lib/test/test_unittest/testmock/testhelpers.py index 246bfe8f200324..007f8ecefea7a5 100644 --- a/Lib/test/test_unittest/testmock/testhelpers.py +++ b/Lib/test/test_unittest/testmock/testhelpers.py @@ -1121,6 +1121,17 @@ class WithWiderDefault: with self.subTest(mock=mock): self.assertIs(mock.narrow_default.__class__, int) + def test_dataclass_with_no_default(self): + @dataclass + class WithWiderDefault: + narrow_default: int | None + + mock = create_autospec(WithWiderDefault, instance=True) + self.assertIs(mock.narrow_default.__class__, type(int | None)) + + mock = create_autospec(WithWiderDefault(1)) + self.assertIs(mock.narrow_default.__class__, int) + class TestCallList(unittest.TestCase): From bfa938527cf0400fa1beedfb835fa8a1d1df362d Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sat, 12 Oct 2024 12:56:58 +0300 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Alyssa Coghlan --- .../test_unittest/testmock/testhelpers.py | 22 +++++++++---------- Lib/unittest/mock.py | 4 ++++ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_unittest/testmock/testhelpers.py b/Lib/test/test_unittest/testmock/testhelpers.py index 007f8ecefea7a5..b1d59047c9d472 100644 --- a/Lib/test/test_unittest/testmock/testhelpers.py +++ b/Lib/test/test_unittest/testmock/testhelpers.py @@ -1107,30 +1107,30 @@ class WithNonFields: with self.assertRaisesRegex(AttributeError, msg): mock.b - def test_dataclass_with_wider_default(self): + def test_dataclass_default_value_type_overrides_field_annotation(self): # If field defines an actual default, we don't need to change # the default type. Since this is how it used to work before #124176 @dataclass - class WithWiderDefault: + class WithUnionAnnotation: narrow_default: int | None = field(default=30) for mock in [ - create_autospec(WithWiderDefault, instance=True), - create_autospec(WithWiderDefault()), + create_autospec(WithUnionAnnotation, instance=True), + create_autospec(WithUnionAnnotation()), ]: with self.subTest(mock=mock): self.assertIs(mock.narrow_default.__class__, int) - def test_dataclass_with_no_default(self): + def test_dataclass_field_with_no_default_value(self): @dataclass - class WithWiderDefault: - narrow_default: int | None + class WithUnionAnnotation: + no_default: int | None - mock = create_autospec(WithWiderDefault, instance=True) - self.assertIs(mock.narrow_default.__class__, type(int | None)) + mock = create_autospec(WithUnionAnnotation, instance=True) + self.assertIs(mock.no_default.__class__, type(int | None)) - mock = create_autospec(WithWiderDefault(1)) - self.assertIs(mock.narrow_default.__class__, int) + mock = create_autospec(WithUnionAnnotation(1)) + self.assertIs(mock.no_default.__class__, int) class TestCallList(unittest.TestCase): diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index c558d22631e894..2375c5f0467034 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -2760,6 +2760,10 @@ def create_autospec(spec, spec_set=False, instance=False, _parent=None, base_entries = {entry: _missing for entry in dir(spec)} if is_type and instance and is_dataclass(spec): + # Dataclass instance mocks created from a class may not have all of their fields + # prepopulated with default values. Create an initial set of attribute entries from + # the dataclass field annotations, but override them with the actual attribute types + # when fields have already been populated. dataclass_fields = fields(spec) entries = {f.name: f.type for f in dataclass_fields} entries.update(base_entries)