Expose caught exception(s) in protected/abstract constructors about fakeiteasy HOT 10 CLOSED

I understand, thanks for explaining. Currently I'm not really prepared to invest the time necessary in making a PR in order to implement this feature, so I'll try to go with the route of creating my own test double just for this specific test scenario, albeit reluctantly :)

from fakeiteasy.

Hi @silkfire,

As you mentioned, the misleading "No constructor matches" message is a known issue. I guess it kinda fell off our radar, but we should definitely address it.

Now, regarding your request, I guess it would be possible to expose the exception(s) that caused the failure, but before we undertake what would probably be a non-trivial refactoring, I want to make sure I understand your issue correctly. I was a little surprised by this:

so that I can assert them.

It seems pretty strange to me to want to assert something about an error that comes from FakeItEasy. When you receive such an error, it usually indicates a mistake in the way you're using FakeItEasy, so the typical thing to do is to fix your test. But you seem to be relying on receiving this error...
If you want to assert that your constructor throws a given exception, why not just call the constructor directly, instead of creating a fake?

from fakeiteasy.

Interestingly, the idea of exposing the original exception has already been discussed almost 10 years ago! But it wasn't pursued, with @blairconrad just mentioning this:

Including InnerException is more work than I thought

Doesn't mean it's not feasible, of course. And it might even be useful. The discussion at the time revolved around having a pretty error message, and apparently having the inner exception was thought to be detrimental to that goal. But we could probably have the inner exception and not include it in the "pretty" error message; just have it available when you're debugging.

from fakeiteasy.

The reason why I can't use the expression-based overload of A.Fake(...) is because the class itself is defined as abstract. In a normal case I would not need to use this approach at all.

And for the second question, I'm not relying on the FIE exception, it's more that I can't access the real, underlying exception here without catching the FakeCreationException and then perhaps inspecting the inner exception.

If this could be solved by the inner exception being thrown then that'd be really clean, then I wouldn't have to rely on the inner workings of FIE, but I'm also fine with being able to read the inner exception from the FIE exception as long it be made accessible somehow, considering this is a bit of a niche case.

Thank you for understanding my problem and I really hope this can be implemented. It feels like we're already half way there, it just needs a bit more polish.

from fakeiteasy.

@silkfire, I think one of @thomaslevesque's questions got lost (heck, I was coming here to ask it and barely noticed that he had asked it before I made my comment):

If you want to assert that your constructor throws a given exception, why not just call the constructor directly, instead of creating a fake?

Your notes about the flaws in the "No constructor matches" and lack of inner exception are valid, but this isn't the imagined use case. The thrown exception is intended to give a human a message to interpret so they can fix either their production or test code (Granted, we could give better information.), not to provide a programmatic interface to allow test code to verify expected exceptions thrown by constructors.

Were I in the situation I understand you are, I'd try to call the constructor directly. Rather than faking your abstract class, make the simplest possible subclass and call its constructor. Then you don't have any FakeItEasy layer to confound things.

from fakeiteasy.

Rather than faking your abstract class, make the simplest possible subclass and call its constructor. Then you don't have any FakeItEasy layer to confound things.

Exactly. @silkfire would that work for you? Given your situation I think it would be the easiest approach.

from fakeiteasy.

That's exactly what I want to avoid. If I wanted to use test doubles, I wouldn't have the need for a dynamic mock framework. FIE already allows me to mock an abstract class, and even passing arguments to it. It's just the case where you want to validate against any exceptions being thrown which is not possible right now.

I don't like having to create dummy classes just to test a a small unit of my production code - that's why I turn to mocks. I think the effort required is rather small and not introducing any breaking changes, but would provide solid value to me (and hopefully others as well).

I hope you could reconsider this decision. (Please?)

from fakeiteasy.

If I wanted to use test doubles, I wouldn't have the need for a dynamic mock framework

I'm not saying you should use manually written test doubles for everything, just for this particular case. I doubt it would take you more than 2 minutes. You don't need to provide actual implementations for abstract methods, just let them throw NotImplementedException.

I don't like having to create dummy classes just to test a a small unit of my production code

Yet it's a very common practice, even when you usually use mocks. I do this quite frequently when I need to test the behavior of an abstract class.

I think the effort required is rather small

If you look at the code, I suspect you will find that it's not as simple as you think. But feel free to submit a PR if you want 😉

While I can see how exposing the inner exception could be useful, we have to consider the value/effort ratio, and at the moment it's not in favor of implementing this feature. Going through the non-trivial effort of implementing it, just so that you can avoid writing a trivial class, is just not worth it.

Again, if it's important to you, you're welcome to contribute a PR!

from fakeiteasy.

Why is it not trivial? You're obviously capturing the exception and putting its message and stack into the exception message of FakeCreationException?

from fakeiteasy.

Why is it not trivial? You're obviously capturing the exception and putting its message and stack into the exception message of FakeCreationException?

Again, you could look at the code to find out 😉
It's not trivial, because we don't throw the FakeCreationException right at the point where the original exception is caught. There's a lot of stuff in between, so we'd need to transport the exception across multiple layers. And we need to think about the case where there are multiple exceptions (which can happen when we try multiple constructors). How do we expose them? If there are several, we need to associate some context with each one, otherwise we don't have the necessary information to make use of the exception. So, a lot of design work, plus implementation, plus testing. All in all, not trivial.

from fakeiteasy.