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

Verify gets confused between the same generic and non-generic signature #749

Closed
lepijohnny opened this issue Feb 14, 2019 · 4 comments
Closed
Labels
Milestone

Comments

@lepijohnny
Copy link
Contributor

lepijohnny commented Feb 14, 2019

Hi,

A few days ago I came across this SO question. Although the question itself states something which is not correct I started investigating a bit and found one kinda strange thing which I want to share. Imagine this situation:

public interface IBazParam { }

public interface IBaz
{
	void Subscribe<T>() where T : IBazParam;
	void Subscribe();
}

//this test will actually pass
[Fact]
public void This_is_weird_test()
{
       //Arrange
       var mock = new Mock<IBaz>();

       //Act
       mock.Object.Subscribe<IBazParam>();

       //Assert
       mock.Verify(foo => foo.Subscribe(), Times.Once);
}

My actual question is why this test is passing, the method Subcribe<T>() is not the same as Subscribe()?

I also looked at the code and found potential bug within InvocationShape.cs class. During the matching of the invocation clearly MethodInfo of these methods are not equal so the IsOverride has to be called. IsOverride method will check if the verifying method is generic, in this case not as we are verifying Subscribe and it will fallback to parameter check which is in this case equal as Subcribe<T> and Subcribe do not have any parameters(in general it will be true if both method have the same set of parameters). IMO the check should be changed to if any of these methods IsGenericMethod, something like:

if (method.IsGenericMethod || invocationMethod.IsGenericMethod)
{
    ...
}
@lepijohnny lepijohnny changed the title Verify confused between the same generic and non-generic signiture Verify gets confused between the same generic and non-generic signiture Feb 15, 2019
@stakx stakx added the bug label Feb 15, 2019
@stakx stakx added this to the 4.11.0 milestone Feb 15, 2019
@stakx
Copy link
Contributor

stakx commented Feb 15, 2019

@lepijohnny - Thank you for your report & analysis. This indeed looks like a bug, and your analysis of InvocationShape.IsOverride seems accurate. I think we should change this, if it doesn't break existing behaviour (tests). Would you like to submit a fix for this (including additional tests such as the above)?

@lepijohnny
Copy link
Contributor Author

lepijohnny commented Feb 15, 2019

@stakx - I already have a fix for this it won't break any of the existing tests. I would be honored to contribute to this magnificent library so I would like to send PR with the fix (and tests of course).

@stakx
Copy link
Contributor

stakx commented Feb 15, 2019

@lepijohnny - Please go ahead! Contributions are welcome! 👍

lepijohnny added a commit to lepijohnny/moq4 that referenced this issue Feb 15, 2019
@lepijohnny lepijohnny changed the title Verify gets confused between the same generic and non-generic signiture Verify gets confused between the same generic and non-generic signature Feb 16, 2019
@stakx
Copy link
Contributor

stakx commented Feb 16, 2019

Fixed in #750.

@stakx stakx closed this as completed Feb 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants