Count recorder counting calls across type rather than per mock instance

[donnie745]

When I have multiple mocks of a specific type in my unit test, and I want to verify calls on each of the individual mocks, the call counts for each individual mock instance are incorrect as they are the sum of calls across all the mock instances of that type. Looking at the fflib_ApexMocks and fflibMethodCountRecorder classes I can see the call recordings are aggregated on a per type basis rather than recording calls per instance.

Of course I can get round this by creating separate fflib_ApexMocks instances and having the separate mocks I wish to verify counts on use their own individual fflib_ApexMocks instance but the behaviour of aggregating call counts on a per type basis rather than counting calls separately for the individual mock object instances seems wrong.

[donnie745]

Guess with Apex you can't use memory addresses to uniquely identify mocked instances. Perhaps the fflib_ApexMocks could have some unique ID generator method (e.g. an incremented integer on each call) for the mock objects, and the generated Mock classes could have their constructors call the fflib_ApexMocks arg instance to assign itself an ID for use with method count recorders?

[afawcett]

I think the === operator should be able to help with this. @frup42 whats your thoughts on this use case?

[donnie745]

If fflib_QualifiedMethod had its member variable String typeName replaced by a member variable Object mockInstance, and the equals() method implementation used === on the mockInstance comparison then that could be an alternative solution.

I guess the hashCode() implementation wouldn't want to call hashCode() on the mockInstance member (especially as this method could be implemented in the generated mock class resulting in null pointer exception if the test hasn't mocked a return value for the method), perhaps it could include the type name of the mocked instance instead in the hashcode calculation.

[donnie745]

On the subject of mocks overriding equals/hashcode (if it implements an interface with equals and hashcode), it looks like the recordMethod() in fflib_MethodCountRecorder will fail in the case where the methodArg argument has in its list of objects a mock which has an overridden equals method generated in the generated mock class code.

In this scenario the methodCountByArgs.get(methodArg) call in recordMethod() will call the overridden equals method in fflib_MethodArgValues, which in turn will call the mock instance's equals method which then results in a call back to the fflib_MethodCountRecorder recordMethod() method and we will end up in an infinite cycle of method calls (up till an inevitable crash happens).

I shall raise this as a separate issue to this one.

Perhaps the fflib_MethodArgValues class could do something clever to wrap mock objects and have wrapper implementing equals using referential === for mocked instances to avoid this problem (would need the code generator to make the generated mock classes implement some no-method fflib interface to identify the instances as being an fflib generated mock class for this I imagine).

[dfruddffdc]

It's definitely more correct to handle each mock independently. It could be a breaking change, if existing tests rely on shared stubbing and invocation counts... but you can't make a correct omelette without breaking a few tests.

The most intuitive model would be for each mock to store its own invocation counts, and stubbed return values. But it's a fairly major overhaul. Instead, I would suggest creating a TokenProvider to generate a unique token for each Object instance, which can be used in the existing ApexMocks map keys.

public class fflib_TokenProvider
{
    //List of objects we've provided tokens for.
    //The element index is acts as the token.
    private static List<Object> tokenAllocations = new List<Object>();

    public Integer getToken(Object o)
    {
        for (Integer i=0; i<tokenAllocations.size(); i++)
        {
            if (o === tokenAllocations[i])
            {
                return i;
            }
        }
        tokenAllocations.add(o);
        return tokenAllocations.size() - 1;
    }
}

[dfruddffdc]

Should be fixed by #42