Add CallsBaseMethods option to fake creation

[Alxandr]

Currently I find myself adding extension-methods like these:

namespace FakeItEasy
{
    static class FakeUtil
    {
        public static IFakeOptionsBuilder<TFaked> CallBase<TFaked>(this IFakeOptionsBuilder<TFaked> config)
        {
            return config.Strict()
                .OnFakeCreated(fake =>
                    A.CallTo(fake).Where(c => !c.Method.IsFinal && c.Method.GetBaseDefinition() != null)
                        .CallsBaseMethod());
        }
    }
}

However, as far as I can tell, this has two drawbacks. First, it's not actually part of FakeItEasy, and second (which is more important, but also caused by the first) it doesn't work optimally. What I want is to be able to say that if you don't find a configuration for the current method-call AND the method has a base method.

Note, if this is possible today in some way I don't know about, please let me know.

[adamralph]

@Alxandr thank you for raising this - we'll look into it ASAP.

[Alxandr]

It should probably be simple to implement. The same place you fallback to throw an exception in strict, one could fall back to calling base (if it exists).

[Alxandr]

Just thought I'd notify that I found a logic error in my previous sample. The following seems to work consistently though:

namespace FakeItEasy
{
    internal static class FakeUtil
    {
        public static IFakeOptionsBuilder<TFaked> CallBase<TFaked>(this IFakeOptionsBuilder<TFaked> config)
        {
            return config.Strict()
                .OnFakeCreated(fake =>
                    A.CallTo(fake).Where(c => !c.Method.IsFinal && !c.Method.IsAbstract)
                        .CallsBaseMethod());
        }
    }
}

Though I'm still worried about which order the configurations are used in. For instance, if I do something like create a fake object like this:

var fake = A.Fake<object>(conf => conf.CallBase());
A.CallTo(() => fake.ToString()).Returns("This is a fake");

I need to be absolutely sure that fake.ToString() returns my custom string, and doesn't call the base method. And as far as I know, with FakeItEasy today is just up to "it just happened to be made that way", and can at any point be refactored. If this is to be supported, some tests would definitely be in place.

[blairconrad]

Hi, @Alxandr.

I'm not a sophisticated IFakeOptionsBuilder- or Strict-user, so can you walk me through exactly what you're proposing?

I don't understand what "per-default calling" means, but it looks like your extension method provides an easy way for you to configure new fakes so that

• they're Strict, and
• any call to the fake, where the method is implemented on the faked type and is overridable, call the "base method" on the faked type

So far so good?

If so (and if not, please correct me!), can you explain to me what the benefit is? It seems to me that having most methods (that is non-abstract ones) default to the base method negates the value of making the fake Strict. This isn't a challenge, but something I'm curious about: why fallback to the implemented (non-abstract) methods? Is it because you typically fake interfaces but want the original methods on object (ToString, GetHash, Equals) to be called? Or do you commonly fake classes, and segregate the methods into abstract and non-abstract according to some scheme?

Finally (sorry, maybe I'm slow, or maybe just not putting enough effort into this), you're suggesting that your extension method be moved into the framework to live alongside Strict, yes? I don't think there's any efficiency benefit to be gained by bringing the extension into the framework, nor do I think the logic can be greatly simplified (which aren't reasons to not bring it in, I'm just trying to allay some or your voiced concerns).

One last point (something I know!): your example where you call CallBase and then explicitly configure fake.ToString will work, and work consistently. Later rules made using A.CallTo will override earlier ones, so ToString should always return "This is a fake". You say

with FakeItEasy today is just up to "it just happened to be made that way", and can at any point be refactored

but there's nothing to worry about here: subsequent A.CallTos overriding earlier ones is the intended behaviour and is promised in the documentation; there's no plan to change the behaviour, and we'd work to avoid such a surprise.

[Alxandr]

Hi.

There's one thing you're mistaken about with regards to the point of my extension method. The point is not to make a strict object that calls base whenever possible. The only reason I made it strict, is that since the framework provides me no way of configuring fakes to call base method "by default" (get back to that later), so by making it strict, I was sure that no methods accidentally didn't do just that (ie. I had a bug in the logic of what methods to call base on).

When you say that "later configurations always trumps older ones" that means that it's actually doable with the extension-method I have today, but I think this should be a deeper integration though. Now, I haven't read the source of FakeItEasy, so I'm making some assumptions as to how things work underneath, but it should help show what it is I'm asking for.

This is (simplified) how I imagine a call to a "faked" method goes:

1. The method Foo() is called.
2. Check if it's configured, if it is, call the configured callback or similar, and return, else continue downwards.
3. Special casing of certain return types like Task.
4. Return default(ReturnType).

What I propose is that a bool is added to fake objects, specifying that if it is set a method-call should by default run the base method, so it would turn into something like this:

1. The method Foo() is called.
2. Check if it's configured, if it is, call the configured callback or similar, and return, else continue downwards.
3. Check if callBase is set. If it is, and a non-abstract base-method exist, call that and return, else continue downwards.
4. Special casing of certain return types like Task.
5. Return default(ReturnType).

Did that make sense?

[blairconrad]

Thanks for the response, @Alxandr. That's cleared things up.

Your guess about how FakeItEasy handles a call to a faked method is close. A little closer is

1. Of all the rules that are defined on the fake, find the first one that matches the call and has not been called too many times already. n.b.: All fakes come with 5 rules out of the box:
   • one, which is considered before user-supplied rules, that handles events, and
   • four, which are considered after user-supplied rules, that handle:
     • object methods such as ToString
     • auto-faking of properties
     • property setters
     • supplying a default return value (this one always matches, so will be called if nothing else was), which will return a Dummy, and that's what handles things such as Task: one of the builtin Dummy rules is for Tasks.
2. Apply the rule

Hm. That list wasn't was exciting as I thought it would be. Anyhow, you can see that FakeItEasy has no concept of an "unconfigured method". When it's executing a method, FakeItEasy just looks through the list of rules until one applies. So, an equivalent version of your suggestion would be to insert a rule into the list so that it fires before one of the "built-in" rules. This is exactly what your extension method does.

You don't actually care about making the fake Strict. This means that, if your guard logic were correct, you could use

public static IFakeOptionsBuilder<TFaked> CallBase<TFaked>(this IFakeOptionsBuilder<TFaked> config)
{
    return config.OnFakeCreated(fake =>
        A.CallTo(fake).Where(c => !c.Method.IsFinal && !c.Method.IsAbstract)
            .CallsBaseMethod());
}

or perhaps even

public static IFakeOptionsBuilder<TFaked> CallBase<TFaked>(this IFakeOptionsBuilder<TFaked> config)
{
    return config.OnFakeCreated(fake =>
        A.CallTo(fake).Where(c => !c.Method.IsAbstract)
            .CallsBaseMethod());
}

since, as I understand things, FakeItEasy shouldn't even see the call to a method that IsFinal.

And so, it seems your request comes down to moving one that latest extension method onto FakeOptionsBuilderExtensions, to sit alongside Strict.

Have I got it?

[Alxandr]

Yeah. That seems correct. The reason I came to this question (if I haven't already written so in an earlier post) was that I was converting a test-base from one test framework to another (and figured I'd at the same time convert the mocking framework too from Moq to FakeItEasy). And Moq has a notion of creating a new Moq that has CallsBase = true, for which I found no equivalent.

[blairconrad]

Excellent. Thanks for clearing things up. Me, I'm a fan of "fewer features is more", but I can see how this may be of use to people (I mean besides you), so I'm inclined to add it. I think the name would be better changed from CallBase to CallsBaseMethods to align with RuleBuilder.CallsBaseMethod. Then we'd have:

fake = A.Fake<IMyInterface>(options => options.CallsBaseMethods());

@FakeItEasy/owners, how do you stand on the feature?

[philippdolder]

@blairconrad In general, if we can increase the number of happy users of FakeItEasy with this kind of features I'm in strong favor of doing these features.

So, yes, do it :)

[blairconrad]

@philippdolder, that's good enough for me. Thanks.

@Alxandr, are you interested in implementing, or would you prefer me to? It is all the same to me. At this point, it's just copying the code and writing tests, which will end up dwarfing the implementation. :)

[Alxandr]

I have exams. So by all mean, please do write it on your own.

[patrik-hagne]

I love it!

[blairconrad]

@patrik-hagne, I'm glad! If you love it enough, don't be shy about merging it. :wink:

[blairconrad]

@alxandr, thanks very much for your work on this issue. Look for your name in the release notes. :trophy:

This issue has been fixed in release 1.24.0.

https://www.nuget.org/packages/FakeItEasy/1.24.0 https://github.com/FakeItEasy/FakeItEasy/releases/tag/1.24.0

[Alxandr]

No problem :) Thanks for such an awesome piece of software in the first case. Glad to help :).