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

In error messages, name the method that we can't intercept on an instance #786

Closed
blairconrad opened this issue Jul 1, 2016 · 7 comments
Assignees
Milestone

Comments

@blairconrad
Copy link
Member

Inspired by a StackOverflow question.

The questioner had the equivalent of

public interface IHaveAList
{
    IEnumerable<string> GetList();
}

var fake = A.Fake<IHaveAList>();
A.CallTo(() => fake.GetList().Single())
    .Throws(new Exception());

And didn't understand why he was told

FakeItEasy.Configuration.FakeConfigurationException: 
The current proxy generator can not intercept the specified method for the following reason: 
- Extension methods can not be intercepted since they're static.

The thought was the complaint was about GetList, when really it was about Single.

We should say which method (I'd almost hope for something like Enumerable.Single<TSource>(this IEnumerable<TSource> source), but whatever we can get would be great.
We could maybe take a cue from DefaultFakeObjectCallFormatter, which has very advanced formatting rules based on whether the method is a method or a property and so on, but it's concerned with object calls and we only have MethodInfos available where we build the exception message above (in DefaultInterceptionAsserter.AssertThatMethodCanBeInterceptedOnInstance), so we'd need to do a little work for a full-fidelity solution.

@adamralph
Copy link
Contributor

Makes sense to me.

@thomaslevesque
Copy link
Member

(I'd almost hope for something like Enumerable.Single<TSource>(this IEnumerable<TSource> source)

We should probably use the CLR format instead (see #1047)

@thomaslevesque
Copy link
Member

btw @blairconrad did you intend to omit the namespace? Should it be System.Linq.Enumerable.Single<TSource>(this System.Collections.Generic.IEnumerable<TSource> source) instead? It's a bit verbose (especially when TSource will be replaced with an actual type), but it's more specific.

@thomaslevesque
Copy link
Member

Also, if we're going to change this message, what do you think about dropping the two blank lines and two spaces at the beginning of the message? As far as I can tell, they don't bring any benefit.

@blairconrad
Copy link
Member Author

@thomaslevesque, as you say, we should use the CLR format. And namespaces are good.

The whitespace is there to make the exception render pretty when the user makes this error:

FakeConfigurationException: 

  The current proxy generator can not intercept the specified method for the following reason:
    - Non virtual methods can not be intercepted.

(You probably know this, and were still thinking they could be dropped.)

I think this sort of thing is done in multiple places, and if we were going to start reformatting messages, it might be better to take a look at all of them?
(Which I'm not that excited about…)

@thomaslevesque
Copy link
Member

(You probably know this, and were still thinking they could be dropped.)

Actually, I didn't realize this. I guess it makes sense, then.

@blairconrad
Copy link
Member Author

This issue was fixed in Release 3.3.0.

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

3 participants