AutoConfiguredMoqCustomization and read-only properties

[dcastro]

This issue relates to the problem described here: AutoConfiguredMoqCustomization and unsettable properties

Repro

public interface Interface
{
    string Property { get; }
}

var fixture = new Fixture().Customize(new AutoConfiguredMoqCustomization());
var a = fixture.Create<Interface>();
Assert.NotNull(a.Property);

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

Background
The setup of Interface in the repro is roughly equivalent to these 3 steps:

var a = new Mock<Interface>();

// MockVirtualMethodsCommand - sets up all non-void methods,
// including getters (e.g. the auto-generated `get_Property`)
a.Setup(x => x.Property).Returns("test");

// StubPropertiesCommand
a.SetupAllProperties();

// AutoMockPropertiesCommand - sets all properties
// - skipped since there are no settable properties - 

Assert.NotNull(a.Object.Property);

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.

I see this as a breaking change introduced by Moq.

Whether it was intentional or not, I don't know - but even if it is a bug, judging from past experience, I don't think it'll be addressed anytime soon since this is kind of an edge case.

Should we address this issue and provide a fix? (e.g., by reordering our setup steps)

[moodmosaic]

Shouldn't we raise an issue to Moq? This looks like a Moq-related issue to me. I could be wrong though...

[dcastro]

@moodmosaic Agreed - done so here: devlooped/moq#196

[moodmosaic]

@dcastro 👍

[ploeh]

As usual an exemplary bug report, @dcastro 👍

even if it is a bug, judging from past experience, I don't think it'll be addressed anytime soon

Unfortunately, I'd tend to agree with this assessment, but let's give the Moq team a couple of days to prove us wrong.

If, after a couple of days, nothing has happened in Moq, I think that we should address the issue here, if we can (and it doesn't pull us in the wrong direction).

[ploeh]

While I've added the label bug to this issue, I haven't added the label jump in. This shouldn't be taken as an indication that none except the 'core team' must address this defect.

The actual reason I didn't add the jump in tag is that I'm not sure whether or not this issue is easy to tackle for newcomers.

@dcastro, you know this part of the code base batter than I do. Do you think we should add the jump in tag?

[dcastro]

Thanks guys!

@ploeh Well, I'm not sure what the criteria is for adding the jump in tag, so I can only speak as to how straightforward the solution might be.

And I can't think of a straightforward solution, so far. My first thought was to switch the order of the setups - (1) call SetupAllProperties and then (2) setup all non-void methods. But that wouldn't work either, because step (2) would setup getters as well, overriding the "stub behavior" introduced by step (1) and therefore rendering it useless. It seems like these two steps are stepping on each other's toes.

So, I think this deserves a bit of thought.

Of course, I can give this a try myself, if we don't get a response from the Moq team soon, as you said.

[ploeh]

For the record, the jump in tag is described here.

[codialsoftware]

@dcastro Thanks for mentioning my stackoverflow's question and your support

[ploeh]

Closing this due to inactivity.

[MichaelLogutov]

Sorry for bringing this up, but is there any workaround? It doesn't seems that Moq team will fix this anytime soon.

[EsbenSkovPedersen]

Still no work around? The codebase I'm currently on is full of get-only properties on interfaces so could really use this

[ploeh]

@MichaelLogutov, @EsbenSkovPedersen, have you considered sending Moq a pull request addressing the underlying issue?

[EsbenSkovPedersen]

@ploeh fair enough. I'll see what I can do. Btw thanks for an awesome library. Saves us so much time.

[nphmuller]

The issue has since been closed by the Moq team and described as their desired behaviour.
See: devlooped/moq#196 (comment)

[zvirja]

@nphmuller Thanks for the follow up. That means that we should fix this on our side as they will not change the behavior.

I'm not too familiar with Moq and don't use it. Therefore, let's see whether somebody with knowledge could fix this.