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

Issue480 #481

Merged
merged 2 commits into from Apr 28, 2015
Merged

Issue480 #481

merged 2 commits into from Apr 28, 2015

Conversation

thomaslevesque
Copy link
Member

Changed HasSameBaseMethod to ensure we're always working with the generic method definition

fixes #480

@blairconrad
Copy link
Member

Thanks, @thomaslevesque. Generally looks good, I think, but I think we may be a while before we get other owners' eyes on it.

One thing that I would suggest though, and maybe I'm being overly picky, is that the commit messages could be more descriptive. It's true that they are a test and fix for #480, but it'd be nice to have more context, such as what you provided for your commits in #471.

We appreciate your input.

@thomaslevesque
Copy link
Member Author

One thing that I would suggest though, and maybe I'm being overly picky, is that the commit messages could be more descriptive. It's true that they are a test and fix for #480, but it'd be nice to have more context, such as what you provided for your commits in #471.

Sure, I can reword the commits

@blairconrad blairconrad added the P3 label Apr 28, 2015
A.CallTo throws a NullReferenceException when trying to configure a
non-virtual generic method. It should throw a FakeConfigurationException,
as it does for a non-virtual non-generic method.

This commit just adds a test case to prove the issue.
The problem was in MethodInfoManager.HasSameBaseMethod. Apparently, for a
virtual generic method, GetBaseMethod returns the generic definition
(IsGenericMethodDefinition is true), but not for a non-virtual generic
method (in this case it returns the realized method itself). This caused
IsSameMethod to return false, which in turn caused GetInvokedMethod to
return null, and this case was not handled in
GetReasonForWhyMethodCanNotBeIntercepted.

Fixed the issue by always calling GetGenericMethodDefinition on generic
methods.
@thomaslevesque
Copy link
Member Author

I pushed the reworded commits

adamralph added a commit that referenced this pull request Apr 28, 2015
@adamralph adamralph merged commit 5c2c01b into FakeItEasy:master Apr 28, 2015
@adamralph
Copy link
Contributor

Thanks @thomaslevesque! Look out for your name in the release notes 🏆

@thomaslevesque thomaslevesque deleted the issue480 branch February 26, 2016 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NullReferenceException when trying to fake a non-virtual generic method
3 participants