SetupAllProperties overrides previous setups of read-only properties

[dcastro]

Repro

public interface Interface
{
    string Property { get; }
}

var a = new Mock<Interface>();

a.Setup(x => x.Property).Returns("test");
a.SetupAllProperties();

Assert.NotNull(a.Object.Property);

Passes with Moq 4.2.1409.1722 or lower
Fails with Moq 4.2.1502.911 or greater

Problem

It appears that, before Moq 4.2.1502.911, the visible side-effects of SetupAllProperties were:

• Stub get-set properties
• Initialize get-set properties with a default value - thus, overriding any previous setups.

Since 4.2.1502.911, SetupAllProperties now does this:

• Stub get-set properties
• Initialize all properties with a default value - thus, overriding any previous setups.

[ploeh]

Nice repro 👍

It seems... unexpected... that SetupAllProperties() should affect read-only properties.

[MichaelLogutov]

Please, fix this.

[kzu]

We'll gladly accept a PR ;)

[david-banister]

Is this related to #275?

[stakx]

@david-banister: Yes, this is related to #275, in the sense that it has the same underlying cause (as mentioned in #162).

@dcastro: While I would judge the new behaviour to be a bug in the context of #162 and #275, I think it should actually be the correct behaviour in your issue's scenario:

[B]efore Moq 4.2.1502.911, the visible side-effects of SetupAllProperties were:

• Stub get-set properties
• Initialize get-set properties with a default value - thus, overriding any previous setups.

Since 4.2.1502.911, SetupAllProperties now does this:

• Stub get-set properties
• Initialize all properties with a default value - thus, overriding any previous setups.

The new behaviour was introduced by #137 to fix a regression that was reportedly introduced by this old Google Code issue 199. And this fix does make sense: A method called SetupAllProperties should do just that. The method is not called SetupAllGetSetProperties, after all.

If we have two setups that affect the same property, I would expect the last setup to win / override the former setup.

Viewed from that perspective, it can be argued that what needs to change in this particular scenario is the order in which you perform your setups: First, the one with the broader effect to set up a basic default: mock.SetupAllProperties(). Then, the one targeting only one specific property to override that default: mock.Setup(_ => _.Property).

That is why I tend to see this issue as a "won't fix".

Any opinions?

[stakx]

I am closing this issue for the time being, for three reasons:

1. The described functional change of Moq happened almost 3 years ago, changing it back only now might break more tests than it fixes. People have probably got used to the new behavior by now.
2. The new behavior can be seen to make good sense and fits well with Moq's "the last setup overrides previous setups" rule of thumb (see my reply just above).
3. Relatively long thread inactivity.

If someone would like to bring forward other arguments why this should be fixed nevertheless, please do so... we can always reopen this issue.