Fix: presence map key and get with query (clientId and connectionId) #647
Conversation
Spec/RealtimeClientPresence.swift
Outdated
| @@ -3590,6 +3590,174 @@ class RealtimeClientPresence: QuickSpec { | |||
| } | |||
| } | |||
|
|
|||
| // RTP11c2 | |||
| fit("should return members filtered by clientId") { | |||
There was a problem hiding this comment.
Done d157cb2. Thanks.
Spec/RealtimeClientPresence.swift
Outdated
| let options = AblyTests.commonAppSetup() | ||
| let now = NSDate() | ||
|
|
||
| var clientMembers: ARTRealtime? |
There was a problem hiding this comment.
Is this used for anything? It doesn't seem to, as then you use presenceData as mock?
There was a problem hiding this comment.
@tcard Just to be sure it filters different connectionId's from 101 members.
There was a problem hiding this comment.
Sorry, I don't follow. Where are you checking that? I only see that you check against the presenceData dataset, not against the members you actually push.
There was a problem hiding this comment.
The query to filter members by connectionId https://github.com/ably/ably-ios/blob/fa2cf962304fd1d968813b808b62f483cb046d05/Spec/RealtimeClientPresence.swift#L3743
There was a problem hiding this comment.
But 1. that's not even in this test, and 2. in that query you're also checking against the presenceData dataset by mocking the SYNC process, so clientMembers are never retrieved unless I'm missing something.
There was a problem hiding this comment.
Yes, you're right. I was looking at the other test. I'll change it.
There was a problem hiding this comment.
Done d157cb2.
| } | ||
| expect(members).to(haveCount(2)) | ||
| expect(members).to(allPass({ (member: ARTPresenceMessage?) in member!.action != .absent })) | ||
| expect(members.filter{ $0.clientId == "a" }).to(beEmpty()) |
There was a problem hiding this comment.
How will this be empty, given that, in presenceData, messages with client ID a also have connection ID one?
There was a problem hiding this comment.
@tcard client ID a has a leave action.
Sorry, I copy&pasted the mock data from another test and maybe that's confusing you.
There was a problem hiding this comment.
Ah, I see the timestamps. It may be a bit clearer if the order in the array was the chronological order, but never mind.
ricardopereira
force-pushed
the
fix-presence-map-key
branch
from
fa2cf96 to
d157cb2
Compare
|
Both are passing https://travis-ci.org/ably/ably-ios/builds/302897096#L3032-L3033 but maybe I should look at the other tests. I don't know if it was me that broke the tests or they already were broken. |
|
WDYT? |
|
@ricardopereira I think this PR can be merged and that should be addressed separately (#652). |
#641