Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mixing "property behavior" and "Setup for properties" broken (or at least changed) in 4.17.1 #1265

Closed
BrightLight opened this issue Jun 2, 2022 · 2 comments

Comments

@BrightLight
Copy link

I wanted to update from Moq 4.16.1 to 4.18.1, but then noticed that suddenly some of our tests fail with the new version. The affected tests all show the same pattern:

  • create mock for an interface
  • call SetupAllProperties
  • define a value for a property using mock.Setup(x => x.property).Returns(value)
  • later directly set a value on the property, e.g. mock.Object.property = newvalue

In 4.16.1 this would return newvalue, since 4.17.1 it returns (old) value.

Example:

public interface IPerson
{
  string Name { get; set; }
}

[TestFixture]
public class Tests
{
  [Test]
  public void PropertyBehaviorTest()
  {
    var personMock = new Mock<IPerson>().SetupAllProperties(); // give all properties "property behavior"
    personMock.Setup(x => x.Name).Returns("Ryan"); // use Setup to define property value
    var person = personMock.Object;
    Assert.That(person.Name, Is.EqualTo("Ryan"));
    person.Name = "Not Ryan"; // set value using property behavior
    Assert.That(person.Name, Is.EqualTo("Not Ryan")); // fails; property will still return "Ryan"
  }
}

Is this scenario supported and just broke in 4.17.1? Or should the old behavior never have worked in the first place? (I know it's probably not ideal to mix "Setup" and direct property setters).

@stakx
Copy link
Contributor

stakx commented Jun 2, 2022

This is expected behavior. You first told your mock to stub all properties (including Name), but then told it to return "Ryan" for Name... and that's what it does from then onwards. Later setups always have preference over earlier ones; the one for Name essentially "shadows" the stubbed property getter.

Perhaps in this particular case you should simply initialize the property to "Ryan" by assigning that value to it, instead of creating a setup?

@stakx stakx closed this as completed Jun 2, 2022
@BrightLight
Copy link
Author

Got it. Thanks for the fast feedback.

Perhaps in this particular case you should simply initialize the property to "Ryan" by assigning that value to it, instead of creating a setup?

Yes, I will update the tests. Just wanted to ask first.
Thanks!

@devlooped devlooped locked and limited conversation to collaborators Sep 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants