Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thread safety #586

Merged
merged 27 commits into from
Mar 23, 2017
Merged

Thread safety #586

Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
04ba309
New EventEmitter (using NSNotificationCenter)
ricardopereira Mar 10, 2017
f344825
Events for the EventEmitter
ricardopereira Mar 10, 2017
4260a4c
Fix: should cancel timers when connection times out
ricardopereira Mar 10, 2017
6f4787a
Fix: new state change can occur before receiving publishing acknowled…
ricardopereira Mar 10, 2017
b38fa55
Test suite: async forced transitions
ricardopereira Mar 10, 2017
2cfc1b1
Test suite: ack order
ricardopereira Mar 10, 2017
fa43d00
Test suite: stop when there's no internet
ricardopereira Mar 10, 2017
1dc13f0
Fix: instance objects released to soon
ricardopereira Mar 10, 2017
44bf0fe
Performed a static analysis from Xcode
ricardopereira Mar 12, 2017
345f16d
fixup! Test suite: ack order
ricardopereira Mar 12, 2017
676c26b
Memory leak: call session invalidate to dispose of its strong referen…
ricardopereira Mar 12, 2017
8bdb646
fixup! Test suite: ack order
ricardopereira Mar 15, 2017
39d8b20
Fix RTN19a: guarantee of a new transport (check transport reference)
ricardopereira Mar 15, 2017
7fa81ec
Fix: ACK or NACK has not yet been received for a message, the client …
ricardopereira Mar 15, 2017
98c9a2f
Enhance RTN14b: better timings
ricardopereira Mar 15, 2017
388d6b2
Fix: REST and Realtime, wait for last operation to release the object
ricardopereira Mar 15, 2017
5de21e9
fixup! Test suite: ack order
ricardopereira Mar 16, 2017
17bca96
Fix: cancel timers when a connection gets closed
ricardopereira Mar 17, 2017
d533e20
fixup! Test suite: ack order
ricardopereira Mar 17, 2017
b68b806
Test suite: timings
ricardopereira Mar 17, 2017
e812cda
fixup! Enhance RTN14b: better timings
ricardopereira Mar 21, 2017
3f9558e
Test suite: close connections
ricardopereira Mar 22, 2017
8ad250f
Fix: turn off immediately reachability when close occurs
ricardopereira Mar 22, 2017
49efb8a
Fix RTC1d: wait for host is not reachable error
ricardopereira Mar 22, 2017
3501f43
fixup! Test suite: ack order
ricardopereira Mar 22, 2017
b0e5f94
Travis update
ricardopereira Mar 22, 2017
257454a
Fix RTN19a
ricardopereira Mar 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions Spec/RealtimeClientConnection.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3368,27 +3368,23 @@ class RealtimeClientConnection: QuickSpec {
let channel = client.channels.get("test")
let transport = client.transport as! TestProxyTransport

expect(client.connection.state).toEventually(equal(ARTRealtimeConnectionState.Connected), timeout: testTimeout)

waitUntil(timeout: testTimeout) { done in
channel.attach { _ in done() }
}

waitUntil(timeout: testTimeout) { done in
transport.ignoreSends = true
channel.publish(nil, data: "message") { error in
expect(error).to(beNil())
guard let newTransport = client.transport as? TestProxyTransport else {
fail("Transport is nil"); done(); return
}
expect(newTransport).toNot(beIdenticalTo(transport))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get how are we guaranteeing this in this test. Where is the old transport disconnected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old transport is the transport.

expect(transport.protocolMessagesReceived.filter{ $0.action == .Connected }).to(haveCount(1))
expect(newTransport.protocolMessagesReceived.filter{ $0.action == .Connected }).to(haveCount(1))
expect(transport.protocolMessagesSent.filter{ $0.action == .Message }).to(haveCount(0))
expect(transport.protocolMessagesSentIgnored.filter{ $0.action == .Message }).to(haveCount(1))
expect(transport.protocolMessagesReceived.filter{ $0.action == .Connected }).to(haveCount(1))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this exactly line 3382?

Unless I'm missing something, these changes make the test not test anymore what it's supposed to be testing. We want to send a message, break the connection before the message gets its ACK, connect again, and check that the message is sent again in the new connection. This is now just testing that the message is sent in the new connection, regardless of whether it was also sent in the old connection and/or it got the ACK.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the transport.ignoreSends = false to give to the test a more real case scenario. The message is published and the connection gets broken right after. The test should check the protocolMessagesSent for MESSAGE, you're right.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 257454a.

expect(newTransport.protocolMessagesSent.filter{ $0.action == .Message }).to(haveCount(1))
done()
}
transport.ignoreSends = false
client.onDisconnected()
}
}
Expand Down