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

Add default support to DummyValueCreationSession for type Lazy<T> #358

Closed
cmerat opened this issue Sep 12, 2014 · 9 comments
Closed

Add default support to DummyValueCreationSession for type Lazy<T> #358

cmerat opened this issue Sep 12, 2014 · 9 comments

Comments

@cmerat
Copy link
Contributor

cmerat commented Sep 12, 2014

Title pretty much says it all. The class already supports Task<T> results and fakes them appropriately. Currently, the default implementation will use the default Lazy<T> constructor which looks for an empty constructor on the class T (that may not exist in many cases).

I've already written the change in my repo, just need to fix a few things to make sure I conform with the guidelines for contributing (which included raising this issue).

Thanks for your consideration and great work on FakeItEasy. It's a real dream to use.

@blairconrad
Copy link
Member

Marking tentatively as P3, since my guess is that this comes up a lot less frequently than the requirement for Task<T> (the latter being baked into C# via async).

@cmerat, for our information, do you find that when you need this, you end up needing it for a whole lot of types, or do you generally have one or two in a project?

I'm sure you've already got something whipped up, but until (if?) we adopt the change, one could take this kind of approach to workaround the lack:

internal abstract class LazyDummyDefinition<T> : DummyDefinition<Lazy<T>>
{
    protected override Lazy<T> CreateDummy()
    {
        return new Lazy<T>(A.Dummy<T>, LazyThreadSafetyMode.None);
    }
}

internal class LazyListDefinition : LazyDummyDefinition<IList>
{
}

[Test]
public void Should_be_able_to_make_dummy_lazy_t()
{
    var dummy = A.Dummy<Lazy<IList>>();

    var value = dummy.Value;

    Assert.That(value, Is.Not.Null);
}

At least it's only one extra (tiny) class per supported Lazy type.

@cmerat
Copy link
Contributor Author

cmerat commented Sep 12, 2014

Actually, the issue I have is I have a repository class that returns a Lazy for different types. Let's call it IFoo:

public interface IFoo
{
    Lazy<T> CreateLazily<T>();
}

I create a Fake IFoo using A.Fake() and will then be making calls to Create using various types. I'd need to create a behavior each time I create a fake IFoo if I wanted it to use the Dummy.

While this is certainly possible, seeing as how Lazy is part of the core .NET toolbox and is a pretty easy behavior to bake in, I figured I'd simply contribute to the project instead of just making my own workaround. Maybe save some other devs some time down the road if they run into a similar situation. I believe it follows in the spirit of chaining recursive fakes by default.

@blairconrad
Copy link
Member

I can certainly see the usefulness in the case you outline.

And I didn't mean to suggest that you should workaround instead of trying to get the change into FakeItEasy. Sorry if it came off that way. I just thought the workaround might be a useful stopgap for you or other readers while your issue is under consideration.

It's a shame the DummyDefinitions work the way they do… an exact match on the type is a little limiting. Had they supported some sort of "predicate-on-type", making these extensions (either outside or inside of FakeItEasy) may have been easier.

@blairconrad
Copy link
Member

Because I can't leave these things alone, I did come up with another technique to perhaps make your (or someone else's) life easier while you work on extending FakeItEasy.

[TestFixture]
public class LazyDummies
{
    // Production interface. May have other methods.
    public interface IFoo
    {
        Lazy<T> CreateLazily<T>();
    }

    // Helper class in tests
    public abstract class FooThatKnowsAboutLazy : IFoo
    {
        public virtual Lazy<T> CreateLazily<T>()
        {
            return new Lazy<T>(A.Dummy<T>);
        }
    }

    // Helper method for creating a fake IFoo. Optional.
    public IFoo FakeFoo()
    {
        return A.Fake<FooThatKnowsAboutLazy>(options => options.CallsBaseMethods());
    }

    [Test]
    public void Should_be_able_to_get_a_lazy_ilist_from_repo()
    {
        var fakeFoo = FakeFoo();

        var lazyList = fakeFoo.CreateLazily<IList>();
        var aList = lazyList.Value;

        Assert.That(aList, Is.Not.Null);
    }

    [Test]
    public void Should_be_able_to_get_a_lazy_int_from_repo()
    {
        var fakeFoo = FakeFoo();

        var lazyInt = fakeFoo.CreateLazily<int>();
        var anInt = lazyInt.Value;

        Assert.That(anInt, Is.EqualTo(0));
    }

    [Test]
    public void Should_be_able_to_customize_createlazily_behaviour()
    {
        var fakeFoo = FakeFoo();

        A.CallTo(() => fakeFoo.CreateLazily<string>())
            .Returns(new Lazy<string>(() => "hippo"));

        var lazyString = fakeFoo.CreateLazily<string>();
        var aString = lazyString.Value;

        Assert.That(aString, Is.EqualTo("hippo"));
    }
}

As you can see, there's only one extra class needed in this case.

I'm not arguing against your issue. Just playing a little.

@cmerat
Copy link
Contributor Author

cmerat commented Sep 12, 2014

Yeah I've managed to work around it in my specific case (after all, it's a single test class so I'm just doing extra work in the [TestInitialize]). I just figured a more generic approach should be simple enough to implement and I've been dying to get my feet wet contributing to open source projects.

@blairconrad
Copy link
Member

Understood. I was just treating it like a puzzle: "what could we do from the outside, if we wanted to?". It's also a useful exercise because the pattern may come up for someone, but with a less-mainstream class than Lazy. Maybe their own custom cache or something. Anyhoo.

As far as contributing, to open source projects goes, it is quite fun, and you're well on your way. If you have any continuing procedure or Git-related questions that you think I can help with, please don't hesitate.

@blairconrad
Copy link
Member

@cmerat, I'll be computerless for 2 weeks starting on the 20th (on which I think I'll be driving through your town, a little before noon), and I think the other FakeItEasy chaps are travelling and otherwise quite busy, so if you submit a tidied up pull request in the meantime and it languishes for a bit, it'll be due to equipment/time constraints, not a lack of interest.

@blairconrad
Copy link
Member

Closed via #363.

@blairconrad blairconrad added this to the 2.0.0 milestone Dec 9, 2014
@blairconrad
Copy link
Member

@cmerat, thanks very much for your work on this issue. Look for your name in the release notes. 🏆

This issue has been fixed in release 2.0.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