Mock for specific method is not consistent for different scenarios

[gonzaloalu]

For this protocol:

protocol MyClassProtocol {
    func set(param1: Int, param2: Int)
}

It generates:

class MyClassMock: MyClassProtocol {
    var invokedSet = false
    var invokedSetCount = 0
    var invokedSetParameters: (param1: Int, param2: Int)?
    var invokedSetParametersList = [(param1: Int, param2: Int)]()
    func set(param1: Int, param2: Int) {
        invokedSet = true
        invokedSetCount += 1
        invokedSetParameters = (param1, param2)
        invokedSetParametersList.append((param1, param2))
    }
}

BUT if I add a second, similar method the mock for the first one changes:

protocol MyClassProtocol {
    func set(param1: Int, param2: Int)
    func set(param1: Int)
}

Generated code:

class MyClassMock: MyClassProtocol {
    var invokedSetParam1 = false
    var invokedSetParam1Count = 0
    var invokedSetParam1Parameters: (param1: Int, param2: Int)?
    var invokedSetParam1ParametersList = [(param1: Int, param2: Int)]()
    func set(param1: Int, param2: Int) {
        invokedSetParam1 = true
        invokedSetParam1Count += 1
        invokedSetParam1Parameters = (param1, param2)
        invokedSetParam1ParametersList.append((param1, param2))
    }
    var invokedSet = false
    var invokedSetCount = 0
    var invokedSetParameters: (param1: Int, Void)?
    var invokedSetParametersList = [(param1: Int, Void)]()
    func set(param1: Int) {
        invokedSet = true
        invokedSetCount += 1
        invokedSetParameters = (param1, ())
        invokedSetParametersList.append((param1, ()))
    }
}

Also, I think that mocks should reflect the FULL method signature to better comply with Swift conventions, resulting in something like this:

class MyClassMock: MyClassProtocol {
    var invokedSetParam1Param2 = false
    var invokedSetParam1Param2Count = 0
    var invokedSetParam1Param2Parameters: (param1: Int, param2: Int)?
    var invokedSetParam1Param2ParametersList = [(param1: Int, param2: Int)]()
    func set(param1: Int, param2: Int) {
        invokedSetParam1Param2 = true
        invokedSetParam1Param2Count += 1
        invokedSetParam1Param2Parameters = (param1, param2)
        invokedSetParam1Param2ParametersList.append((param1, param2))
    }
    var invokedSetParam1 = false
    var invokedSetParam1Count = 0
    var invokedSetParam1Parameters: (param1: Int, Void)?
    var invokedSetParam1ParametersList = [(param1: Int, Void)]()
    func set(param1: Int) {
        invokedSetParam1 = true
        invokedSetParam1Count += 1
        invokedSetParam1Parameters = (param1, ())
        invokedSetParam1ParametersList.append((param1, ()))
    }
}

I'm trying on Swift 5 and Xcode 10.2

[gonzaloalu]

@seanhenry Do you know if this is the default behaviour ?

[seanhenry]

Hi @gonzaloalu

Thanks for raising this issue and sorry for the late reply.

What you've described is expected behaviour. The way it works is that it will try to make the simplest unique name for each item. So in your case, you started with a method which was not overloaded so the simplest name invokedSet was used. When you added the next method, it is now overloaded and the simpler method gets the simpler name and the more complex method gets the more verbose name.

Unfortunately the generator can't really know about the history of the generated mocks so it can't tell which was created first. And even if it could, in some cases, it would be impossible to avoid changing the generated name for the original method. For example:

protocol P {
  func doSomething(param1: Int, param2: Int)
}

would generate:

...
  var invokedDoSomething = false
...

then adding a new method to the protocol:

protocol P {
  func doSomething(param1: Int, param2: Int)
  func doSomething()
}

wouldn't be able to keep invokedDoSomething for func doSomething(param1: Int, param2: Int) because it wouldn't be able to generate a unique name for func doSomething().

I hear where you're coming from with using the full signature for the generated names but I think generating verbose names when it's not required would take away from the readability of your tests in most cases. Also, they would have to be more verbose than you're suggesting because you can have methods which have identical parameter labels but different types. So in your example the mock would become:

...
  var invokedSetParam1IntParam2Int = false
...

I do recognise that the current strategy isn't perfect and re-generating that test double would likely have caused your tests to fail but this strategy was a conscious decision to maintain the readability of your tests with an unfortunate compromise of causing the generated names to change if adding an overloaded method.

What do you think?

[gonzaloalu]

Hello @seanhenry, first of all thanks for your response and explanation, and sorry for the late response on our part.

I agree with you on the part that if we want to support same method signatures but different parameter types, It will be too verbose. ( I don't know another solution for this) But I think that this is not very common scenario. Also, the plugin doesn't support that today either.

Despite beign too verbose, I think that the plugin should build the method name with all its parameters. As I told you, we want to be in compliance with the Swift API Design Guidelines, and we found that in a lot of cases it's difficult to understand what method we are calling from the test due to lack of information. Let me give you an example that complies with this reference https://swift.org/documentation/api-design-guidelines/#argument-labels:

protocol MyLocationProtocol {
    func set(latitude: Double, longitude: Double)
}

class MyLocationProtocolMock {
    var invokedSet = false
    var invokedSetCount = 0
    var invokedSetParameters: (latitude: Double, longitude: Double)?
    var invokedSetParametersList = [(latitude: Double, longitude: Double)]()
    func set(latitude: Double, longitude: Double) {
        invokedSet = true
        invokedSetCount += 1
        invokedSetParameters = (latitude, longitude)
        invokedSetParametersList.append((latitude, longitude))
    }
}

As you can see invokedSet doesn't give you too much information about what you are calling.

We think that having the tool be more accurate when creating the mocks for every method would be more helpful than trying to have them simplified

invokedSet is easy to read but doesn't give you any tip about what you are setting.
invokedSetLatitudeLongitute is more verbose, but it's self explainatory.

Maybe this could be toggled in a setting? What do you think?

[seanhenry]

Hi, sorry again for the slow reply - I'm very busy with other projects at the moment.

You're right, it could be a toggled setting. I'm happy to do this when I find time but it would be great to see some support for this feature. Everybody reading this, please respond with a 👍 if this feature is of interest to you.