Skip to content

Commit

Permalink
wip #19
Browse files Browse the repository at this point in the history
equatable conformances are for tests, but not sure if right thing to do

note that I've had ot make RoomLifecycle.current async, this seems odd
but not sure there's a better way if we want to make use of swift's
built-in concurrency, also it highlights to callers that there can't be
a definitive concept of 'current' which was indeed a concern i'd
highlighted earlier

but also had to make `onChange` async so that I could do some state
management, that seems odder. is there a way to instead bake the
async-ness into the sequence?

things where testing framework makes things messy with concurrency:

- `async let` with things like XCTUnwrap, XCAssertEqual
- async operations with XCTAssertThrowsError

I hope that once Xcode 16 is released we can instead use Swift Testing.
  • Loading branch information
lawrence-forooghian committed Aug 27, 2024
1 parent 721f10c commit 6dd74bb
Show file tree
Hide file tree
Showing 6 changed files with 401 additions and 3 deletions.
1 change: 1 addition & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ To check formatting and code quality, run `swift run BuildTool lint`. Run with `
- We should aim to make it easy for consumers of the SDK to be able to mock out the SDK in the tests for their own code. A couple of things that will aid with this:
- Describe the SDK’s functionality via protocols (when doing so would still be sufficiently idiomatic to Swift).
- When defining a `struct` that is emitted by the public API of the library, make sure to define a public memberwise initializer so that users can create one to be emitted by their mocks. (There is no way to make Swift’s autogenerated memberwise initializer public, so you will need to write one yourself. In Xcode, you can do this by clicking at the start of the type declaration and doing Editor → Refactor → Generate Memberwise Initializer.)
- TODO something about adding spec points in code and tests

## Building for Swift 6

Expand Down
18 changes: 18 additions & 0 deletions Sources/AblyChat/Ably+Concurrency.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import Ably

// This file contains extensions to ably-cocoa’s types, to make them easier to use in Swift concurrency.

internal extension ARTRealtimeChannelProtocol {
// TODO: it's not good that this isn't automatically bridged by Swift — create an ably-cocoa issue for revisiting this developer experience
func attachAsync() async throws {
try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Void, Swift.Error>) in
attach { error in
if let error {
continuation.resume(throwing: error)
} else {
continuation.resume()
}
}
}
}
}
77 changes: 74 additions & 3 deletions Sources/AblyChat/RoomStatus.swift
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import Ably

public protocol RoomStatus: AnyObject, Sendable {
var current: RoomLifecycle { get }
var current: RoomLifecycle { get async }
// TODO: (https://github.com/ably-labs/ably-chat-swift/issues/12): consider how to avoid the need for an unwrap
var error: ARTErrorInfo? { get }
func onChange(bufferingPolicy: BufferingPolicy) -> Subscription<RoomStatusChange>
func onChange(bufferingPolicy: BufferingPolicy) async -> Subscription<RoomStatusChange>
}

public enum RoomLifecycle: Sendable {
public enum RoomLifecycle: Sendable, Equatable {
case initialized
case attaching
case attached
Expand All @@ -31,3 +31,74 @@ public struct RoomStatusChange: Sendable {
self.error = error
}
}

internal actor DefaultRoomStatus: RoomStatus {
private(set) var current: RoomLifecycle
// TODO: how to clean this up?
private var stateChangeSubscriptions: [Subscription<RoomStatusChange>] = []
private let contributors: [ARTRealtimeChannelProtocol]

init(contributors: [ARTRealtimeChannelProtocol] = []) {
current = .initialized
self.contributors = contributors
}

init(forTestingWhatHappensWhenCurrentlyIn current: RoomLifecycle) {
self.current = current
contributors = []
}

internal nonisolated var error: ARTErrorInfo? {
fatalError("Not yet implemented")
}

internal func onChange(bufferingPolicy: BufferingPolicy) -> Subscription<RoomStatusChange> {
let subscription = Subscription<RoomStatusChange>(bufferingPolicy: bufferingPolicy)
stateChangeSubscriptions.append(subscription)
return subscription
}

/**
Updates ``current`` and emits a status change event.
*/
private func changeStatus(to new: RoomLifecycle) {
let previous = current
current = new
let statusChange = RoomStatusChange(current: current, previous: previous)
emitStatusChange(statusChange)
}

private func emitStatusChange(_ change: RoomStatusChange) {
for subscription in stateChangeSubscriptions {
subscription.emit(change)
}
}

internal func performAttachOperation() async throws {
switch current {
case .attached:
// CHA-RL1a
return
case .releasing:
// CHA-RL1b
throw ARTErrorInfo.create(withCode: 102_102, message: "Attempted to attach room that's in RELEASING state")
case .released:
// CHA-RL1c
throw ARTErrorInfo.create(withCode: 102_103, message: "Attempted to attach room that's in RELEASED state")
case .initialized, .suspended, .attaching, .detached, .detaching, .failed:
break
}

// CHA-RL1e
changeStatus(to: .attaching)

// CHA-RL1f
for contributor in contributors {
// TODO: address warning "Passing argument of non-sendable type 'any ARTRealtimeChannelProtocol' outside of actor-isolated context may introduce data races"
try await contributor.attachAsync()
}

// CHA-RL1g
changeStatus(to: .attached)
}
}
91 changes: 91 additions & 0 deletions Tests/AblyChatTests/DefaultRoomStatusTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import Ably
@testable import AblyChat
import XCTest

final class DefaultRoomStatusTests: XCTestCase {
// CHA-RS2a (TODO what's the best way to test this), CHA-RS3
func testInitialStatus() async {
let status = DefaultRoomStatus()

let current = await status.current
XCTAssertEqual(current, .initialized)
}

// CHA-RL1a
func testAttachWhenAlreadyAttached() async throws {
let status = DefaultRoomStatus(forTestingWhatHappensWhenCurrentlyIn: .attached)

// TODO: is this the right place for this to go?
try await status.performAttachOperation()
// TODO: how to check it's a no-op? what are the things that it might do?
}

// TODO: why is some tool removing this `await` below?

// CHA-RL1b
func testAttachWhenReleasing() async throws {
let status = DefaultRoomStatus(forTestingWhatHappensWhenCurrentlyIn: .releasing)

await assertThrowsARTErrorInfo(withCode: 102_102) {
try await status.performAttachOperation()
}
}

// CHA-RL1c
func testAttachWhenReleased() async throws {
let status = DefaultRoomStatus(forTestingWhatHappensWhenCurrentlyIn: .released)

await assertThrowsARTErrorInfo(withCode: 102_103) {
try await status.performAttachOperation()
}
}

// CHA-RL1e
func testAttachTransitionsToAttaching() async throws {
let (continuation, attachResult) = makeAsyncFunctionWithContinuation(of: Void.self, throwing: ARTErrorInfo.self)

let status = DefaultRoomStatus(contributors: [MockRealtimeChannel(attachResult: .fromFunction(attachResult))])
let statusChangeSubscription = await status.onChange(bufferingPolicy: .unbounded)
async let statusChange = statusChangeSubscription.first { _ in true }

async let attachSignal: Void = try await status.performAttachOperation()

// Check that status change was emitted and that its `current` is as expected
guard let statusChange = await statusChange else {
XCTFail("Expected status change but didn’t get one")
return
}

// Check that current status is as expected
let current = await status.current
XCTAssertEqual(current, .attaching)

// Now that we’ve seen the ATTACHING state, allow the contributor `attach` call to complete
continuation.yield(())
_ = try await attachSignal

XCTAssertEqual(statusChange.current, .attaching)
}

// CHA-RL1g
func testWhenAllContributorsAttachSuccessfullyItTransitionsToAttached() async throws {
let contributors = (1 ... 3).map { _ in MockRealtimeChannel(attachResult: .complete(.success(()))) }
let status = DefaultRoomStatus(contributors: contributors)

let statusChangeSubscription = await status.onChange(bufferingPolicy: .unbounded)
async let attachedStatusChange = statusChangeSubscription.first { $0.current == .attached }

try await status.performAttachOperation()

guard let statusChange = await attachedStatusChange else {
XCTFail("Expected status change to attached but didn't get one")
return
}

XCTAssertEqual(statusChange.current, .attached)

// Check that current status is as expected
let current = await status.current
XCTAssertEqual(current, .attached)
}
}
42 changes: 42 additions & 0 deletions Tests/AblyChatTests/Helpers/Helpers.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import Ably
import XCTest

/**
Asserts that a given async expression throws an ``ARTErrorInfo`` with a given ``ARTErrorInfo.code``.
*/
// TODO: we can’t use ARTErrorCode, I suppose? That's an issue
// TODO: it doesn't take an autoclosure because for whatever reason one of our linting tools removes the `await` on the expression
func assertThrowsARTErrorInfo<T>(withCode expectedCode: Int, _ expression: () async throws -> T, file: StaticString = #filePath, line: UInt = #line) async {
let result: Result<T, Error>

do {
result = try await .success(expression())
} catch {
result = .failure(error)
}

guard case let .failure(error) = result else {
XCTFail("Expected expression to throw an error", file: file, line: line)
return
}

guard let artError = error as? ARTErrorInfo else {
XCTFail("Expected expression to throw an ARTErrorInfo but got \(type(of: error))", file: file, line: line)
return
}

XCTAssertEqual(artError.code, expectedCode, "Expected thrown ARTErrorInfo to have code \(expectedCode) but got \(artError.code)", file: file, line: line)
}

// TODO: note that this function can't be called multiple times
// TODO: improve this API (shouldn’t return that Continuation)
func makeAsyncFunctionWithContinuation<Element, Failure: Error>(of _: Element.Type = Element.self, throwing _: Failure.Type = Failure.self) -> (continuation: AsyncThrowingStream<Element, Error>.Continuation, function: () async -> Result<Element, Failure>) {
let (stream, continuation) = AsyncThrowingStream.makeStream(of: Element.self)
return (continuation: continuation, function: {
do {
return try await .success(stream.first { _ in true }!)
} catch {
return .failure(error as! Failure)
}
})
}
Loading

0 comments on commit 6dd74bb

Please sign in to comment.