Skip to content

Commit

Permalink
Add subscriber network connectivity tests
Browse files Browse the repository at this point in the history
Resolves #539.

These are based on the corresponding tests in the Android codebase at
commit f6d163f. They stick as closely as possible to the structure and
behaviour of those tests, to which I have not applied much of a critical
eye. The majority of the log messages and comments are simply copied
across.

The only structural difference between these tests and the Android ones
is that I have removed the caller’s responsibility to call
`SubscriberMonitor#close`. There are places in the Android
implementation where we forget to call it, and also it isn’t called if
an error is thrown by `waitForStateTransition`. By making the subscriber
monitor responsible for its own cleanup, we remove these issues.

When I started writing these tests, we were still targeting iOS 12 and
above, which meant it was not possible to use Swift concurrency. Hence,
all of the code is written using either completion handlers or blocking
functions. Since then, we’ve decided to set our deployment target to iOS
13 (see #597), which means we could now use Swift concurrency.  However,
by that point I was well into the writing of these tests and did not
want to re-structure them, especially since they require functionality
that doesn’t exist out of the box in Swift concurrency (for example
timeouts). We may wish to revisit at some point and switch to using
Swift concurrency, but I don’t think it’s urgent.

In order to generate the list of which faults to skip, I ran all of the
tests and checked which ones failed. Whilst doing so, I encountered a
crash in ably-cocoa, which I believe is already reported as
ably/ably-cocoa#1380. In order to get past this issue, I tried using
Marat’s fix from ably-cocoa branch
`fix/1380-dispatch-ARTOSReachability_Callback` (commit 036be28). This
removed the crashes. The list of skipped tests is the list of tests that
failed when using this branch. It then actually turned out that the
non-skipped tests do not exhibit this crash anyway, so I didn’t include
any version change to ably-cocoa in this PR. We can deal with this crash
when we try re-enabling the tests in #575, by which point hopefully the
fix for ably-cocoa#1380 will have been released. (Internal thread re
this crash and its appearance in these tests is [1].)

TODO check copied docstrings
  • Loading branch information
lawrence-forooghian committed Mar 16, 2023
1 parent f2ae612 commit 854e23a
Show file tree
Hide file tree
Showing 9 changed files with 1,631 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import AblyAssetTrackingInternal
import AblyAssetTrackingSubscriber
import AblyAssetTrackingSubscriberTesting
import class Combine.CurrentValueSubject
import struct Combine.AnyPublisher

extension SubscriberNetworkConnectivityTests {
/// A helper implementation of `SubscriberDelegate` which exposes Combine publishers which emit the events sent by a `Subscriber` to its delegate.
///
/// The idea of this class is to allow us to keep the implementation of the subscriber `NetworkConnectivityTests` as similar to those of Android as possible. The Android `Subscriber` does not use a delegate pattern; rather, it uses [Kotlin flows](https://kotlinlang.org/docs/flow.html) to emit values over time.
///
/// Furthermore, the Android version of the subscriber `NetworkConnectivityTests` relies on some specific behaviours of the flow types provided by Kotlin. This class reproduces those behaviours as much as is necessary to satisfy the tests.
class CombineSubscriberDelegate: SubscriberDelegate {
private let logHandler: InternalLogHandler

/// Publishes the connection status values received by ``SubscriberDelegate.subscriber(sender:,didChangeAssetConnectionStatus:)``.
///
/// This is meant to mimic Android’s `_trackableStates = MutableStateFlow(TrackableState.Offline())`.
///
/// This publisher sends the latest received value to each new subscriber, to mimic the behaviour of a Kotlin `StateFlow`. Unlike a `StateFlow`, however, this publisher does not publish an initial value.
let trackableStates: AnyPublisher<ConnectionState, Never>
private let _trackableStates = CurrentValueSubject<ConnectionState?, Never>(nil)

/// Publishes the resolution values received by ``SubscriberDelegate.subscriber(sender:,didUpdateResolution:)``.
///
/// This is meant to mimic Android’s `_resolutions = MutableSharedFlow(replay = 1)`.
///
/// This publisher sends the latest received value to each new subscriber, to mimic the behaviour of a Kotlin `SharedFlow` with `replay = 1`.
let resolutions: AnyPublisher<Resolution, Never>
private let _resolutions = CurrentValueSubject<Resolution?, Never>(nil)

/// Publishes the resolution values received by ``SubscriberDelegate.subscriber(sender:,didUpdatePublisherPresence:)``.
///
/// This is meant to mimic Android’s `_publisherPresence = MutableStateFlow(false)`.
///
/// This publisher sends the latest received value to each new subscriber, to mimic the behaviour of a Kotlin `StateFlow`. Unlike a `StateFlow`, however, this publisher does not publish an initial value.
let publisherPresence: AnyPublisher<Bool, Never>
private let _publisherPresence = CurrentValueSubject<Bool?, Never>(nil)

private let _locations = CurrentValueSubject<LocationUpdate?, Never>(nil)
/// Publishes the location values received by ``SubscriberDelegate.subscriber(sender:,didUpdateEnhancedLocation:)``.
///
/// This is meant to mimic Android’s `_enhancedLocations = MutableSharedFlow(replay = 1)`.
///
/// This publisher sends the latest received value to each new subscriber, to mimic the behaviour of a Kotlin `SharedFlow` with `replay = 1`.
let locations: AnyPublisher<LocationUpdate, Never>

init(logHandler: InternalLogHandler) {
self.trackableStates = _trackableStates.compactMap { $0 }.eraseToAnyPublisher()
self.resolutions = _resolutions.compactMap { $0 }.eraseToAnyPublisher()
self.publisherPresence = _publisherPresence.compactMap { $0 }.eraseToAnyPublisher()
self.locations = _locations.compactMap { $0 }.eraseToAnyPublisher()

self.logHandler = logHandler.addingSubsystem(Self.self)
}

// MARK: SubscriberDelegate

func subscriber(sender: Subscriber, didChangeAssetConnectionStatus status: ConnectionState) {
logHandler.debug(message: "Delegate received subscriber(sender:,didChangeAssetConnectionStatus:) - status \(status)", error: nil)
_trackableStates.value = status
logHandler.debug(message: "Sent status \(status) to _trackableStates", error: nil)
}

func subscriber(sender: Subscriber, didUpdateResolution resolution: Resolution) {
logHandler.debug(message: "Delegate received subscriber(sender:,didUpdateResolution:) - resolution \(resolution)", error: nil)
_resolutions.value = resolution
logHandler.debug(message: "Sent resolution \(resolution) to _resolutions", error: nil)
}

func subscriber(sender: Subscriber, didUpdatePublisherPresence isPresent: Bool) {
logHandler.debug(message: "Delegate received subscriber(sender:,didUpdatePublisherPresence:) - isPresent \(isPresent)", error: nil)
_publisherPresence.value = isPresent
logHandler.debug(message: "Sent isPresent \(isPresent) to _publisherPresence", error: nil)
}

func subscriber(sender: Subscriber, didUpdateEnhancedLocation locationUpdate: LocationUpdate) {
logHandler.debug(message: "Delegate received subscriber(sender:,didUpdateEnhancedLocation:) - locationUpdate \(locationUpdate)", error: nil)
_locations.value = locationUpdate
logHandler.debug(message: "Sent locationUpdate to _locations", error: nil)
}

func subscriber(sender: Subscriber, didFailWithError error: ErrorInformation) {
logHandler.error(message: "Delegate received subscriber(sender:,didFailWithError:", error: error)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import XCTest
import Combine
import AblyAssetTrackingTesting
import AblyAssetTrackingSubscriberTesting

class CombineSubscriberDelegateTests: XCTestCase {
// The testing pattern is taken from
// https://heckj.github.io/swiftui-notes/#patterns-testing-and-debugging

func testReplaysLastValueToAllNewSubscribers() {
let combineSubscriberDelegate = SubscriberNetworkConnectivityTests.CombineSubscriberDelegate(
logHandler: TestLogging.sharedInternalLogHandler
)

let subscriber = SubscriberMock()

// Given...
// ...that the object under test has received an invocation of `subscriber(sender:,didChangeAssetConnectionStatus:)`

combineSubscriberDelegate.subscriber(sender: subscriber, didChangeAssetConnectionStatus: .online)

// When...

var cancellables = Set<AnyCancellable>()
let firstExpectation = expectation(description: "First subscriber gets value")
let secondExpectation = expectation(description: "Second subscriber gets value")

// ...a subscriber is added to the object under test’s `trackableStates`...
combineSubscriberDelegate.trackableStates.sink { status in
XCTAssertEqual(status, .online)
firstExpectation.fulfill()

// ...and, when that first subscriber receives a value, another subscriber is added to the object under test’s `trackableStates`...
combineSubscriberDelegate.trackableStates.sink { status in
XCTAssertEqual(status, .online)
secondExpectation.fulfill()
}.store(in: &cancellables)
}.store(in: &cancellables)

// Then...
// ...both subscribers receive the connection status sent to the object under test’s `subscriber(sender:,didChangeAssetConnectionStatus:)` method.

waitForExpectations(timeout: 10)
}
}
Loading

0 comments on commit 854e23a

Please sign in to comment.