gh-124176: create_autospec must not change how dataclass defaults are mocked

[sobolevn]

See #124176 (comment)
@ncoghlan found that my change introduced a behavior change (or even a regression) to dataclasses with defaults.

Before my change create_autospec would use the default's type for _spec_class.
After my change it would use the annotation's type.

Which is not always correct. For example, int | str would produce a __class__ of types.UnionType

People might rely on that, so no need to break this.

• Issue: Improve UX for mocking dataclasses that are problematic to instantiate #124176

[cjw296]

How come we don't end up using UnionType here from the int | None annotation?

[sobolevn]

Because I override entries with default_entries here: https://github.com/python/cpython/pull/124724/files#diff-347d0254250a1ab7ab8e31b405e2c35b74cd2838df4ee74f1b658a459eb91f1aR2765

This way field(default=0) has the priority over the annotation.

[cjw296]

Two questions from that:

1. What happens with a class such as this?

   @dataclass
   class SampleClass:
       sample_attr: int | None
       

2. What does the mock machinery do with the field object it gets in the case you show above?

[sobolevn]

1. In this case spec would be UnionType, because we have no default
2. Mock does nothing special to field, dataclass itself sets SampleClass.sample_attr = default, and we just use getattr to get the existing default

[cjw296]

Would it be possible to add a unit test (or more if necessary) showing this?

[sobolevn]

Done!

[ncoghlan]

I agree with the backport labels, since we want to backport the test case, even if the functional change doesn't apply cleanly.

[ncoghlan]

LGTM, but I've suggested some naming clarifications for the new test cases.

[ncoghlan]

The default value type here is actually narrower than the field annotation , so describing it as with_wider_default ends up being confusing because "default" is being used in two different senses. (the new dataclass handling picks up the wider field annotation by default, so we want the narrower class attribute handling that reads the default value to override that)

Suggested names below:

Suggested change

	def test_dataclass_with_wider_default(self):
	def test_dataclass_default_value_type_overrides_field_annotation(self):

[ncoghlan]

Suggested change

	class WithWiderDefault:
	narrow_default: int | None = field(default=30)
	for mock in [
	create_autospec(WithWiderDefault, instance=True),
	create_autospec(WithWiderDefault()),
	class WithUnionAnnotation:
	narrow_default: int | None = field(default=30)
	for mock in [
	create_autospec(WithUnionAnnotation, instance=True),
	create_autospec(WithUnionAnnotation()),

[ncoghlan]

Suggested change

	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)
	def test_dataclass_field_with_no_default_value(self):
	@dataclass
	class WithUnionAnnotation:
	no_default: int | None
	mock = create_autospec(WithUnionAnnotation, instance=True)
	self.assertIs(mock.no_default.__class__, type(int | None))
	mock = create_autospec(WithUnionAnnotation(1))
	self.assertIs(mock.no_default.__class__, int)

[ncoghlan]

This is obscure enough that I think it's worth adding a comment explaining why the code works as it does:

Suggested change

	dataclass_fields = fields(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)