How to write accurate tests for state mutation with custom equality implementation?

[mamouneyya]

We have a state that looks like this:

public struct State: Hashable, Codable {
    public internal(set) var items: Set<Item>
    // ... etc
}

For performance considerations and because we're using Item within sets, we had to write a custom implementation for Hashable & Equatable that doesn't include all properties in that struct. Something like this:

public struct Item: Codable {
    public var identifier: String
    public var someData: String
    public var someOtherData: String
}

extension Item: Hashable {
    public func hash(into hasher: inout Hasher) {
        hasher.combine(identifier)
    }
    
    public static func == (lhs: Self, rhs: Self) -> Bool {
        return lhs.identifier == rhs.identifier
    }
}

Now the problem is, TCA's test functions seem to internally rely on Equatable to evaluate the expected state mutations which results in basically ignoring differences to all variables except identifier. This means, in our tests even when providing an incorrect expected value for someData for example the test still succeeds.

The interesting thing is that the diffing logic implemented for showing the error messages / console logs seems to be identifying all differences correctly. So if I for example provide an incorrect value for identifier, the message would clearly say that both identifier & someData are incorrect. If I, however, provide the correct value only for identifier (someData is still incorrect) the test succeeds and no errors appear.

[mbrandonw]

I'm gonna convert this to a discussion since it's more appropriate there.

Where are the performance problems you encounter with the default Equatable conformance? Is it in TCA code such as ViewStore, or elsewhere?

[mamouneyya]

Where are the performance problems you encounter with the default Equatable conformance? Is it in TCA code such as ViewStore, or elsewhere?

It's non-TCA related. Basically in our reducer we need the ability to update a subset of items in the original set based on the received action. Having this setup enables us to do something like this:

 case .uploadSucceeded(items: var items):
        for var item in items {
            item.status = .uploaded
            // this only works because `status` is excluded from `Equatable`
            state.items.update(with: item)
        }

The implementation of Hashable guarantees that no duplicate items with the exact same identifier will ever exist in the set which is something we want.

[mbrandonw]

Gotcha.

I can only think of two ways around this right now:

1.) You can create a wrapper type around State that provides a custom Equatable conformance so that you can restore the per-field equality check, and then in tests you can do something like .scope(state: Wrapper.init) on the TestStore to wrap state in that type. That will make it so that tests use the full equatable check.

2.) Alternatively, you can go back to the automatically synthesized Equatable and move the efficiency concerns to the application layer. You can write all your algorithms to not leverage Item's equatable conformance and instead just look at identifiers.

Neither are ideal, but I would lean towards the latter. In the first suggestion you will be writing wrapper types with custom Equatable conformances for anything that holds onto Item, and all of those conformance will need to be updated anytime Item is changed or you run the risk of not testing what you think you're testing. It is safest to let value types implement their own Equatable, and just opt out of using it if performance becomes a concern.

[mamouneyya]

Got it. Thanks for the suggestions. I agree the latter would be a safer approach. Will try and see if I can refactor this in an efficient way.

[mamouneyya]

By the way, out of curiosity, cannot the logic of detecting the diffs for logging purposes be somehow extended to be used for the actual diffing instead of simply relying on Equatable? I don't like the idea that I need to refactor my code only for the testing when it works beautifully in real life. Also, having different algorithms there was a bit confusing that at the beginning I thought the testing functions are somehow broken. When identifier was incorrect, the printed message was showing both identifier and the other variables, the message went away when fixing identifier only and nothing else.

[mbrandonw]

We don't think it's a good idea to rely on the stringified output of types in order to test for equality in tests. Those outputs are not guaranteed to be correct or exhaustive. You can check out the code to see that it's just a big mess of reflection in order to try to get something reasonable out of any type, but there are a few known ways it can go wrong (such as Objective-C enums) and probably a lot of unknown ways it can go wrong.

And I understand it may have seemed like the testing functions were broken when you witnessed that behavior, but we feel that TCA works best when value types are kept as simple as possible, and that includes keeping the automatically synthesized equality check.

It's worth mentioning that this isn't really a TCA issue. If you were using this code in a non-TCA application and you wrote a test with XCTAssertEqual you would see the same behavior. Equal values with unequal data. And I bet that this issue can sneak into your application code if you are not careful too. Changing a type's notion of equality is a very tricky thing to do. That's why I feel my second suggestion would be the best route in the long run.

[mamouneyya]

Fair. Thank you! I'm gonna close this based on the above.