Use a private encoder to compare messages, if needed

[olivaresf]

Main has a single test broken: test_encodingWithCustomEncoder. The reason for this is that comparing two Messages isn't straightforward since their JSON values are saved as Strings.

[jayohms]

Can you elaborate on the broken test? I'm not seeing any failures: https://github.com/hotwired/strada-ios/actions

Where did you find that checking message equality is necessary?

[olivaresf]

Can you elaborate on the broken test? I'm not seeing any failures: https://github.com/hotwired/strada-ios/actions

Actually, the CI threw me for a loop once I opened the PR. I worked on #22 and when I ran the tests locally, test_encodingWithCustomEncoder failed every time and it makes sense it would fail. Here's the error message Xcode 15.0 (15A240d) throws:

XCTAssertEqual failed: ("Message(id: "1", component: "page", event: "connect", metadata: Optional(Strada.Message.Metadata(url: "https://37signals.com")), jsonData: "{\"title\":\"Page-title\",\"subtitle\":\"Page-subtitle\",\"action_name\":\"go\"}")") is not equal to ("Message(id: "1", component: "page", event: "connect", metadata: Optional(Strada.Message.Metadata(url: "https://37signals.com")), jsonData: "{\"action_name\":\"go\",\"title\":\"Page-title\",\"subtitle\":\"Page-subtitle\"}")")

As you can see, the Strings are clearly not the same:

"{\"title\":\"Page-title\",\"subtitle\":\"Page-subtitle\",\"action_name\":\"go\"}")"
"{\"action_name\":\"go\",\"title\":\"Page-title\",\"subtitle\":\"Page-subtitle\"}")"

So I don't understand how it's passing on CI. I see we're using Xcode 14.2 so I'm downloading it to test it.

Where did you find that checking message equality is necessary?

Message conforms to Equatable and we have unit tests for it 🤔... I just removed it and nothing broke except the tests. I don't know if I'm missing something then. @svara?

[svara]

Good catch @olivaresf!
@jayohms I've added conformance to Equatable for testing purposes.
For me, the offending test fails on both Xcode 14.3 and 15.0. It wasn't failing when I was working on this though. 🤔

Why is the test failing?
When replacing data using replacing<T: Encodable>(event updatedEvent: String? = nil, data: T) -> Message, the JSON decoder doesn't guarantee any specific sorting. There's the option to set it to produce JSON with dictionary keys sorted in lexicographic order. I wouldn't enforce this though, since this solves only half of the problem. The user can still set the raw JSON string.

@olivaresf solution is good and solves the problem.

[svara]

@olivaresf The custom equality implementation is great. I've left a few comments to keep the code tidy.

[svara]

This can be removed since it's unused.

[svara]

Since comparing messages is an implementation detail, I'd move this to its own extension file, Message+Equatable. This will also keep the Message interface clean.

[jayohms]

Why is the test failing?
When replacing data using replacing<T: Encodable>(event updatedEvent: String? = nil, data: T) -> Message, the JSON decoder doesn't guarantee any specific sorting.

Ah, thanks for the explanation 🙏

[joemasilotti]

Ha! I tripped over the same thing when debugging a an issue last night. Thanks for linking me here, @svara.

What can we do to get this merged? Having this in place for #24 will fix a few outstanding issues folks are running into.

[svara]

@joemasilotti I've addressed the issues. Should be good to go, but please give it another look.

cc @olivaresf

[svara]

FYI, the tests are failing due to timing issues. We should probably consider migrating evaluateJavaScript to its async counterpart.

[joemasilotti]

We should probably consider migrating evaluateJavaScript to its async counterpart.

I took a first pass in #26. I'm not sure how I like exposing the public API to Strada as async functions, though. Thoughts on that?

[joemasilotti]

This will be addressed in #29.