As per the Slack discussion, this PR fixes various issues with testing that caused a race condition in testInsertInsertConflict, and could have caused other issues in future tests. This should make the tests a lot more stable, and the test environment will more accurately reflect the real environment. This also matches more closely with how the tests work in the Android SDK
There were three issues:
SyncTestContext.streamAndSync was stopping the DataSynchronizer and starting the stream (via the InstanceChangeStreamDelegate) if the stream state was closed, but this was causing the stream to sometimes be opened more than once (setting documents stale in the process, causing the failing test), because the actual sync logic already starts the stream so it wasn't necessary. I changed streamAndSync to not change anything about the DataSynchronizer, and to wait for streams to open before doing the sync pass.
In DataSynchronizer.start(), the stream was not getting opened if the sync thread was disabled (which it is for testing). I moved this to happen regardless of whether the sync thread is enabled, since the stream starting is not related to the sync thread.
The TestNetworkMonitor in BaseStitchIntTestCocoaTouch was not properly dispatching network state changes to the DataSynchronizer, because of a Swift nuance with didSet. We were always dispatching the previous state, not the new state, so the DataSynchronizer never actually started a new stream when the network state was modified by the test.
Issues 2 and 3 are probably the reason why the race-condition causing workaround in 1 was made
As per the Slack discussion, this PR fixes various issues with testing that caused a race condition in
testInsertInsertConflict, and could have caused other issues in future tests. This should make the tests a lot more stable, and the test environment will more accurately reflect the real environment. This also matches more closely with how the tests work in the Android SDKThere were three issues:
SyncTestContext.streamAndSyncwas stopping the DataSynchronizer and starting the stream (via theInstanceChangeStreamDelegate) if the stream state was closed, but this was causing the stream to sometimes be opened more than once (setting documents stale in the process, causing the failing test), because the actual sync logic already starts the stream so it wasn't necessary. I changedstreamAndSyncto not change anything about the DataSynchronizer, and to wait for streams to open before doing the sync pass.DataSynchronizer.start(), the stream was not getting opened if the sync thread was disabled (which it is for testing). I moved this to happen regardless of whether the sync thread is enabled, since the stream starting is not related to the sync thread.TestNetworkMonitorinBaseStitchIntTestCocoaTouchwas not properly dispatching network state changes to theDataSynchronizer, because of a Swift nuance withdidSet. We were always dispatching the previous state, not the new state, so the DataSynchronizer never actually started a new stream when the network state was modified by the test.Issues 2 and 3 are probably the reason why the race-condition causing workaround in 1 was made