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

Rework integration with Moq to support generic methods #1139

Open
zvirja opened this issue Sep 22, 2019 · 13 comments
Open

Rework integration with Moq to support generic methods #1139

zvirja opened this issue Sep 22, 2019 · 13 comments
Milestone

Comments

@zvirja
Copy link
Member

zvirja commented Sep 22, 2019

This issue is to track our activities regarding rework of Moq integration. Currently we are creating Mock<T> using reflection and then configure each method individually using reflection. Due to this approach we have a limitation and do not support generic methods, as we cannot configure all possible instantiations upfront.

We should change our integration approach and instead register AutoFixture as default value provider. This way we inject "passively" and are invoked by Moq when value is required. We are already following that way with NSubstitute and it works pretty well.

We haven't investigated yet whether it's possible, but @stakx promised to give it a look one day.

Let's postpone this activity till the moment we start our work on v5, as this change might require us to change minimum supported version for Moq. Unless somebody wants to look upfront to prepare Moq for this.

I'll update this issue once we start work on v5.

@riezebosch
Copy link

riezebosch commented Sep 23, 2019

    public class ValueProviderTests
    {
        [Fact]
        public void UseValueProvider()
        {
            var fixture = new Fixture();
            var mock = new Mock<IInterfaceWithGenericMethod>
            {
                DefaultValueProvider = new AutoFixtureValueProvider(fixture)
            };


            Assert.NotNull(mock.Object.GenericMethod<string>());
        }
        
        [Fact]
        public void CustomizedAnonymousType()
        {
            var fixture = new Fixture();
            fixture.Customize<string>(x => x.FromSeed(y => "asf"));
            var mock = new Mock<IInterfaceWithGenericMethod>
            {
                DefaultValueProvider = new AutoFixtureValueProvider(fixture)
            };


            Assert.Equal("asf", mock.Object.GenericMethod<string>());
        }

        public class AutoFixtureValueProvider : DefaultValueProvider
        {
            private readonly IFixture _fixture;

            public AutoFixtureValueProvider(IFixture fixture)
            {
                _fixture = fixture;
            }

            protected override object GetDefaultValue(Type type, Mock mock) =>
                _fixture.Create(type, new SpecimenContext(_fixture));
        }
    }

This looks promising, except for the fact the mock is still tight to the interface. However, I expect most of reflection code could be replaced by this.

@stakx
Copy link

stakx commented Sep 23, 2019

@riezebosch: Looking good. Btw., if you subclass LookupOrFallbackDefaultValueProvider instead of DefaultValueProvider, you'll get automatic support for default values wrapped in completed Task<> and ValueTask<>. In case you want that.

@stakx
Copy link

stakx commented Sep 23, 2019

@zvirja: Thanks for the reminder!

Someone noticed a while ago over at Moq's repo that due to the accessibility of DefaultValueProvider's methods, these providers aren't as composable as they could be. I've been wanting to fix this in one of the next minor releases. It'll likely be a very small breaking change. I thought I'd mention it here so that we can coordinate versions / release schedules if needed.

Also, if anyone here finds other problems with Moq's DefaultValueProvider, please let me know.

@riezebosch
Copy link

riezebosch commented Sep 24, 2019

Checkout my work on the value provider: https://github.com/riezebosch/AutoFixture/tree/value-provider. Rewrote the tests a bit that explicitly create a Mock<T> from the fixture and assert on .Object Update: these are removed because they tested a class I removed. Have to double-check if I lost some specified behavior there.

What's not working (yet):

  1. ref/out parameters, moq doesn't seem to visit the value provider for that (@stakx) ?!
  2. delegates
  3. Virtual methods are ignored (only creating objects for interfaces, abstract classes and delegates)
  4. removed the configure members (for now).

Had to use reflection to invoke the SetupAllProperties since it's only public available on Mock<T> (@stakx).

Regarding the ref parameters, this little test fails:

[Fact]
public void RefParameter()
{
    var mock = new Mock<IInterfaceWithRefMethod> { DefaultValueProvider = new TestValueProvider("bye") };
    var s = "hi";

    mock.Object.Method(ref s);
    
    Assert.Equal("bye", s);
}
[Fact]
public void OutParameter()
{
    var mock = new Mock<IInterfaceWithOutMethod> { DefaultValueProvider = new TestValueProvider(4) };

    mock.Object.Method(out var s);
    
    Assert.Equal(4, s);
}
private class TestValueProvider : DefaultValueProvider
{
    private readonly object _result;
    public TestValueProvider(object result) => _result = result;
    protected override object GetDefaultValue(Type type, Mock mock) => _result;
}
AutoFixture.AutoMoq.ValueProviderTests.RefParameter

Assert.Equal() Failure
          ↓ (pos 0)
Expected: bye
Actual:   hi
          ↑ (pos 0)AutoFixture.AutoMoq.ValueProviderTests.OutParameter

Assert.Equal() Failure
Expected: 4
Actual:   0

So it looks like Moq is not overwriting the value for a ref parameter?

Current status:
Screen Shot 2019-09-24 at 15 01 13

@stakx
Copy link

stakx commented Sep 24, 2019

ref/out parameters, moq doesn't seem to visit the value provider for that

As far as I can tell, from Moq's perspective, that's by design:

  • ref parameters are assumed to already have a value assigned before the method call, so there's no strict need to set them.

  • out parameters are historically treated in a very peculiar way: they're read at setup time to determine the desired value that the argument should be set to at the end of each method call.

That's why the default value provider doesn't get queried for those by-refs.

If AutoFixture always sets by-ref parameters, then Moq and AutoFixture have different modes of operation, and I'm not yet sure how we can seamlessly bring the two together.

Had to use reflection to invoke the SetupAllProperties since it's only public available on Mock<T>

Yes, Moq doesn't have a non-generic public API, it's in the planning however (see e.g. devlooped/moq#887). For the moment, I'm afraid you'll need that reflection detour.

@riezebosch
Copy link

riezebosch commented Sep 24, 2019

ref/out parameters, moq doesn't seem to visit the value provider for that

As far as I can tell, from Moq's perspective, that's by design:

  • ref parameters are assumed to already have a value assigned before the method call, so there's no strict need to set them.
  • out parameters are historically treated in a very peculiar way: they're read at setup time to determine the desired value that the argument should be set to at the end of each method call.

That's why the default value provider doesn't get queried for those by-refs.

If AutoFixture always sets by-ref parameters, then Moq and AutoFixture have different modes of operation, and I'm not yet sure how we can seamlessly bring the two together.

Had to use reflection to invoke the SetupAllProperties since it's only public available on Mock<T>

Yes, Moq doesn't have a non-generic public API, it's in the planning however (see e.g. moq/moq4#887). For the moment, I'm afraid you'll need that reflection detour.

Minor correction, for the ref parameter the value provider is visited but for some reason the variable doesn't get overwritten (probably boxing or something like that?). For the out parameter the provider is not visited at all.

For some reason the ref methods were ignored in current implementation so I guess this is already a step forward.

@riezebosch
Copy link

@zvirja Please, checkout https://github.com/riezebosch/AutoFixture/tree/value-provider. Made quite some progress and was able to remove a lot of code. I did some tweaks to make it running on my macbook, the actual implementation and the removal of code in separate commits.

Let me know what you think.

@stakx
Copy link

stakx commented Sep 24, 2019

@riezebosch:

Minor correction, for the ref parameter the value provider is visited but for some reason the variable doesn't get overwritten (probably boxing or something like that?).

Strange... none of what you've described should happen. May I ask you to post a small demo code—either here or (perhaps better, so we don't hijack this issue) over at the moq/moq4 repo?

@riezebosch
Copy link

It's in my reply above: #1139 (comment)

@stakx
Copy link

stakx commented Sep 24, 2019

It's in my reply above: #1139 (comment)

I ran those tests, which fail (as expected).

Minor correction, for the ref parameter the value provider is visited

I tried to reproduce that claim, but failed. I made the following change:

 private class TestValueProvider : DefaultValueProvider
 {
     private readonly object _result;
     public TestValueProvider(object result) => _result = result;
-    protected override object GetDefaultValue(Type type, Mock mock) => _result;
+    protected override object GetDefaultValue(Type type, Mock mock) => throw new InvalidOperationException("unexpectedly invoked default value provider");
 }

But that exception never gets thrown by the two tests, meaning GetDefaultValue didn't get invoked. So your correction appears to be incorrect. What am I missing?

@riezebosch
Copy link

riezebosch commented Sep 24, 2019

My guess is the exception gets swallowed by Moq. I slightly modified the provider:

[Fact]
public void RefParameter()
{
    var provider = new TestValueProvider("bye");
    var mock = new Mock<IInterfaceWithRefMethod> { DefaultValueProvider = provider };
    var s = "hi";
    mock.Object.Method(ref s);
    
    Assert.True(provider.IsInvoked);
    Assert.Equal("bye", s);
}
[Fact]
public void OutParameter()
{
    var provider = new TestValueProvider(4);
    var mock = new Mock<IInterfaceWithOutMethod> { DefaultValueProvider = provider };
    mock.Object.Method(out var s);
    
    Assert.True(provider.IsInvoked);
    Assert.Equal(4, s);
}
private class TestValueProvider : DefaultValueProvider
{
    private readonly object _result;
    public TestValueProvider(object result) => _result = result;
    protected override object GetDefaultValue(Type type, Mock mock)
    {
        IsInvoked = true;
        return _result;
    }
    public bool IsInvoked { get; set; }
}
AutoFixture.AutoMoq.ValueProviderTests.RefParameter

Assert.Equal() Failure
          ↓ (pos 0)
Expected: bye
Actual:   hi
          ↑ (pos 0)
AutoFixture.AutoMoq.ValueProviderTests.OutParameter

Assert.True() Failure
Expected: True
Actual:   False

moq 4.13.0 BTW.

@stakx
Copy link

stakx commented Sep 24, 2019

@riezebosch, I cannot reproduce your RefParameter test result using Moq 4.13.0, the above code, and the two interfaces as defined below. Both tests fail due to the default value provider not having been invoked (as expected).

public interface IInterfaceWithRefMethod { void Method(ref string arg); }
public interface IInterfaceWithOutMethod { void Method(out int    arg); }

(Perhaps your IInterfaceWithRefMethod.Method method has a non-void return type? That would obviously cause the default value provider to get invoked, and then your test discards the return value produced by it.)

@riezebosch
Copy link

You're right, the value provider is visited for the ref parameter method but only because the interface method is defined as string Method(ref string s);.

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

Successfully merging a pull request may close this issue.

3 participants