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

Behavioural change between 4.11 -> 4.12 with CallBase=True and params args #877

Closed
chrisaut opened this issue Aug 8, 2019 · 3 comments
Closed

Comments

@chrisaut
Copy link

chrisaut commented Aug 8, 2019

Consider the following code:

    public class C1
    {
        public virtual string M(string s, params object[] args) => "fromBase";
    }
    public class Tests
    { 
        [Fact]
        public void Test1()
        {
            var c = new Mock<C1>();
            c.CallBase = true;
            c.Setup(x => x.M(It.IsAny<string>(), It.IsAny<object[]>()))
                //.Returns(default(string))
                .Verifiable();

            var result = c.Object.M("foo", 1, 2);
            Assert.Null(result);
        }
     }

In 4.11.0 this test passes, that is it does NOT call into C1's implementation of "M"

However in 4.12.0 this test fails, result = "fromBase", it is as if the setup doesn't match. Uncommenting the .Returns line fixes it in 4.12.0. I think it has to do with the params object array

This took me a while to figure out, and I'm not entirely sure what the expected behavior should be, but I think this is a bug?

@stakx
Copy link
Contributor

stakx commented Aug 8, 2019

This is noted in the changelog for 4.12:

Setups with no .Returns(…) nor .CallBase() no longer return default(T) for loose mocks, but a value that is consistent with the mock's CallBase and DefaultValue[Provider] settings. (@stakx, #849)

The change was made for improved consistency, and it's in line with a hint dropped by @kzu (can't find the reference right now) that setups on non-void methods without any Returns, Throws or CallBase are basically a user mistake and will be called out as such in Moq v5.

@stakx stakx closed this as completed Aug 8, 2019
@chrisaut
Copy link
Author

chrisaut commented Aug 8, 2019

Oh I see, sorry I didn't see a changelog under releases, but see it is a .md file.

@stakx
Copy link
Contributor

stakx commented Aug 8, 2019

Yes, perhaps I should start annotating releases with the relevant changelog entries. Thanks for the hint!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants