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

Misleading exception message when failing to create a Dummy #1367

Closed
blairconrad opened this issue Jun 9, 2018 · 7 comments
Closed

Misleading exception message when failing to create a Dummy #1367

blairconrad opened this issue Jun 9, 2018 · 7 comments
Assignees
Milestone

Comments

@blairconrad
Copy link
Member

public class Unconstructable
{
    private Unconstructable() { }
}

[Test]
public void Test()
{
    A.Dummy<Unconstructable>();
}

Fails like so:

Test 'FakeItEasyQuestions2015.PrivateConstructor.Test' failed: FakeItEasy.Core.FakeCreationException : Unable to create fake object.
	at FakeItEasy.Creation.DefaultFakeAndDummyManager.CreateDummy(Type typeOfDummy)
	at FakeItEasy.A.Dummy[T]()
	PrivateConstructor.cs(17,0): at FakeItEasyQuestions2015.PrivateConstructor.Test()

"Unable to create fake object." is misleading. We were trying to make a Dummy, which need not be a Fake.

@thomaslevesque
Copy link
Member

Should we create a specific exception type, or just adjust the message?
Also, it's annoying that the exception doesn't give any reason for failure.

@blairconrad
Copy link
Member Author

blairconrad commented Jun 29, 2018

/// <summary>
/// An exception that is thrown when there was an error creating a fake object.
/// </summary>

Perhaps a new exception type.

It is annoying that there's no reason for failure. We could add

"There's no user-supplied DummyFactory for <type> and it's not a Task. If it's a Task<TResult> or Lazy<TResult>, we couldn't make a Dummy TResult for one of the reasons we're describing now. <type< isn't fakeable. If it's a value type, we couldn't make it using Activator.CreateInstance. If none of those applied, we couldn't make it because we couldn't find an available constructor whose arguments we could resolve by making Dummies."

Maybe that needs a little work.

(On a slightly more serious note, we could dynamically build the message, selectively omitting some of the "If" clauses.)

@blairconrad
Copy link
Member Author

@blairconrad
Copy link
Member Author

Okay. Additional notes on a potentially useful error message:

  1. if the requested Dummy type is actually a Task, Task<T>, or Lazy<T>, we will never fail to create the Dummy without throwing an exception, so those cases are handled
  2. same if the Dummy type is a value type

So that just leaves us with the cases where we fail to create a Fake and then we fail to instantiate a class by providing Dummy arguments to the constructors.
As we've recently seen, there's an elaborate mechanism in place (but not quite hooked up to Dummy Resolution) to report the reasons why a Fake might not be created.
We could replicate that when trying to create the Dummy by instantiating a class, but if we accumulate reasons for failure from both Fake creation and non-Fake type creation, it might be overwhelming for the user. Still, I can try to whip something up.

@thomaslevesque
Copy link
Member

if the requested Dummy type is actually a Task, Task<T>, or Lazy<T>, we will never fail to create the Dummy without throwing an exception, so those cases are handled

I'm not sure I understand... From what I can see, it never throws an exception at all if you try to create a dummy Lazy<T> or Task<T>. When you access the value/result, it just returns null.

As we've recently seen, there's an elaborate mechanism in place (but not quite hooked up to Dummy Resolution) to report the reasons why a Fake might not be created.
We could replicate that when trying to create the Dummy by instantiating a class, but if we accumulate reasons for failure from both Fake creation and non-Fake type creation, it might be overwhelming for the user. Still, I can try to whip something up.

Maybe we could still throw a DummyCreationException with additional details as the inner exception?

@blairconrad
Copy link
Member Author

blairconrad commented Jul 2, 2018

it never throws an exception at all if you try to create a dummy Lazy<T> or Task<T>

It might do if users have registered a bad DummyFactory.

Maybe we could still throw a DummyCreationException with additional details as the inner exception?

Possibly. I'm working on something that would (UserCallbackExceptions aside) provide details in a DummyCreationException.

@blairconrad
Copy link
Member Author

This change has been released as part of FakeItEasy 4.8.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

2 participants