-
Notifications
You must be signed in to change notification settings - Fork 6
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add subscriber network connectivity tests
Resolves #539. These are based on the corresponding tests in the Android codebase at commit f6d163f (plus a fix added in eea952b). 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].) [1] https://ably-real-time.slack.com/archives/CSQEKCE81/p1678802535684519
- Loading branch information
1 parent
a8b813b
commit 67d88fd
Showing
9 changed files
with
1,621 additions
and
1 deletion.
There are no files selected for viewing
35 changes: 35 additions & 0 deletions
35
Tests/Support/AblyAssetTrackingTesting/MultipleErrors.swift
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
/// Maintains a list of errors and provides a way to throw any contained errors. | ||
/// | ||
/// Useful in the situation where you want to perform a sequence of operations, continuing even if one fails, and subsequently report on the result of these operations once they’ve all been attempted. | ||
public struct MultipleErrors { | ||
private var errors: [Error] = [] | ||
|
||
/// Creates a new `MultipleErrors` instance. | ||
public init() {} | ||
|
||
public enum MultipleErrorsError: Error { | ||
case multipleErrors([Error]) | ||
} | ||
|
||
/// Adds an error to the list. | ||
public mutating func add(_ error: Error) { | ||
errors.append(error) | ||
} | ||
|
||
/// Throws any errors in the list. | ||
/// | ||
/// - If the list of errors is empty, this does nothing. | ||
/// - If the list of errors contains a single error, this throws that error. | ||
/// - If the list of errors contains multiple errors, this throws a ``MultipleErrorsError.multipleErrors`` describing those errors. | ||
public func check() throws { | ||
if errors.isEmpty { | ||
return | ||
} | ||
|
||
if errors.count == 1 { | ||
throw errors[0] | ||
} | ||
|
||
throw MultipleErrorsError.multipleErrors(errors) | ||
} | ||
} |
87 changes: 87 additions & 0 deletions
87
Tests/SystemTests/NetworkConnectivityTests/Subscriber/Helper/CombineSubscriberDelegate.swift
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 trackableStatesSubject = 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 resolutionsSubject = 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 publisherPresenceSubject = CurrentValueSubject<Bool?, 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> | ||
private let locationsSubject = CurrentValueSubject<LocationUpdate?, Never>(nil) | ||
|
||
init(logHandler: InternalLogHandler) { | ||
self.trackableStates = trackableStatesSubject.compactMap { $0 }.eraseToAnyPublisher() | ||
self.resolutions = resolutionsSubject.compactMap { $0 }.eraseToAnyPublisher() | ||
self.publisherPresence = publisherPresenceSubject.compactMap { $0 }.eraseToAnyPublisher() | ||
self.locations = locationsSubject.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) | ||
trackableStatesSubject.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) | ||
resolutionsSubject.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) | ||
publisherPresenceSubject.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) | ||
locationsSubject.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) | ||
} | ||
} | ||
} |
47 changes: 47 additions & 0 deletions
47
...stemTests/NetworkConnectivityTests/Subscriber/Helper/CombineSubscriberDelegateTests.swift
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
import AblyAssetTrackingSubscriberTesting | ||
import AblyAssetTrackingTesting | ||
import Combine | ||
import XCTest | ||
|
||
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) | ||
} | ||
} |
Oops, something went wrong.