From f5cab27fdf118810aa04e3d93b4a18e14e690437 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 19 Sep 2024 13:23:54 -0300 Subject: [PATCH 1/4] Switch to Swift Testing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We’re doing this because it’s, I guess, the future, and offers performance improvements, but more immediately because it reduces verbosity when testing `async` functions and properties. I’ve done the migration in a pretty straightforward fashion, just stripping the `test` prefix from the method names (which my eyes are still getting used to), and not trying to use any new features like test descriptions, nested suites, or tags. We can figure out how we want to use these as we get used to using Swift Testing. Decided to keep testing for errors via a do { … } catch { … } instead of using #expect(…, throws:) because I like being able to write tests in a Given / When / Then fashion — i.e. do things, then make assertions. Whilst working on this, I noticed that some of Swift Testing’s useful test failure messages stop being so useful when testing asynchronous code, which takes a bit of the shine off it. For example, if you write > #expect(status.current == .attached) then you’ll get a failure message of > Expectation failed: (current → .detached) == .attached that is, it shows you information about `current` which helps you to understand why the expectation failed. But if, on the other hand, you write > #expect(await status.current == .attached) then you’ll get a failure message of > Expectation failed: await status.current == .detached which is less useful. I asked about this in [1] and was told that it’s a known issue and that Swift macro limitations mean it’s unlikely to be fixed soon. ([2] is a similar question that I found after.) If we decide that this is a dealbreaker and that we want the rich failure messages, then we’ll need to switch back to the current way of doing things; that is, first do the `await` and then the #expect. Resolves #55. [1] https://forums.swift.org/t/expectation-failure-messages-are-less-useful-with-await/74754 [2] https://forums.swift.org/t/try-expect-throwing-or-expect-try-throwing/73076/17 --- .../DefaultChatClientTests.swift | 18 ++++---- .../DefaultInternalLoggerTests.swift | 27 +++++++----- .../DefaultRoomStatusTests.swift | 33 ++++++-------- Tests/AblyChatTests/DefaultRoomTests.swift | 44 ++++++++----------- Tests/AblyChatTests/DefaultRoomsTests.swift | 25 ++++++----- Tests/AblyChatTests/Helpers/Helpers.swift | 16 +++---- Tests/AblyChatTests/InternalLoggerTests.swift | 15 ++++--- .../MessageSubscriptionTests.swift | 25 +++++------ Tests/AblyChatTests/SubscriptionTests.swift | 18 ++++---- 9 files changed, 107 insertions(+), 114 deletions(-) diff --git a/Tests/AblyChatTests/DefaultChatClientTests.swift b/Tests/AblyChatTests/DefaultChatClientTests.swift index 9798d50d..1f207ae2 100644 --- a/Tests/AblyChatTests/DefaultChatClientTests.swift +++ b/Tests/AblyChatTests/DefaultChatClientTests.swift @@ -1,17 +1,19 @@ @testable import AblyChat -import XCTest +import Testing -class DefaultChatClientTests: XCTestCase { - func test_init_withoutClientOptions() { +struct DefaultChatClientTests { + @Test + func init_withoutClientOptions() { // Given: An instance of DefaultChatClient is created with nil clientOptions let client = DefaultChatClient(realtime: MockRealtime.create(), clientOptions: nil) // Then: It uses the default client options let defaultOptions = ClientOptions() - XCTAssertTrue(client.clientOptions.isEqualForTestPurposes(defaultOptions)) + #expect(client.clientOptions.isEqualForTestPurposes(defaultOptions)) } - func test_rooms() throws { + @Test + func rooms() throws { // Given: An instance of DefaultChatClient let realtime = MockRealtime.create() let options = ClientOptions() @@ -20,8 +22,8 @@ class DefaultChatClientTests: XCTestCase { // Then: Its `rooms` property returns an instance of DefaultRooms with the same realtime client and client options let rooms = client.rooms - let defaultRooms = try XCTUnwrap(rooms as? DefaultRooms) - XCTAssertIdentical(defaultRooms.realtime, realtime) - XCTAssertTrue(defaultRooms.clientOptions.isEqualForTestPurposes(options)) + let defaultRooms = try #require(rooms as? DefaultRooms) + #expect(defaultRooms.realtime === realtime) + #expect(defaultRooms.clientOptions.isEqualForTestPurposes(options)) } } diff --git a/Tests/AblyChatTests/DefaultInternalLoggerTests.swift b/Tests/AblyChatTests/DefaultInternalLoggerTests.swift index 07df9ac9..a40bf223 100644 --- a/Tests/AblyChatTests/DefaultInternalLoggerTests.swift +++ b/Tests/AblyChatTests/DefaultInternalLoggerTests.swift @@ -1,15 +1,17 @@ @testable import AblyChat -import XCTest +import Testing -class DefaultInternalLoggerTests: XCTestCase { - func test_defaults() { +struct DefaultInternalLoggerTests { + @Test + func defaults() { let logger = DefaultInternalLogger(logHandler: nil, logLevel: nil) - XCTAssertTrue(logger.logHandler is DefaultLogHandler) - XCTAssertEqual(logger.logLevel, .error) + #expect(logger.logHandler is DefaultLogHandler) + #expect(logger.logLevel == .error) } - func test_log() throws { + @Test + func log() throws { // Given: A DefaultInternalLogger instance let logHandler = MockLogHandler() let logger = DefaultInternalLogger(logHandler: logHandler, logLevel: nil) @@ -22,13 +24,14 @@ class DefaultInternalLoggerTests: XCTestCase { ) // Then: It calls log(…) on the underlying logger, interpolating the code location into the message and passing through the level - let logArguments = try XCTUnwrap(logHandler.logArguments) - XCTAssertEqual(logArguments.message, "(Ably/Room.swift:123) Hello") - XCTAssertEqual(logArguments.level, .error) - XCTAssertNil(logArguments.context) + let logArguments = try #require(logHandler.logArguments) + #expect(logArguments.message == "(Ably/Room.swift:123) Hello") + #expect(logArguments.level == .error) + #expect(logArguments.context == nil) } - func test_log_whenLogLevelArgumentIsLessSevereThanLogLevelProperty_itDoesNotLog() { + @Test + func log_whenLogLevelArgumentIsLessSevereThanLogLevelProperty_itDoesNotLog() { // Given: A DefaultInternalLogger instance let logHandler = MockLogHandler() let logger = DefaultInternalLogger( @@ -44,6 +47,6 @@ class DefaultInternalLoggerTests: XCTestCase { ) // Then: It does not call `log(…)` on the underlying logger - XCTAssertNil(logHandler.logArguments) + #expect(logHandler.logArguments == nil) } } diff --git a/Tests/AblyChatTests/DefaultRoomStatusTests.swift b/Tests/AblyChatTests/DefaultRoomStatusTests.swift index ec5d2cb5..89ea6d80 100644 --- a/Tests/AblyChatTests/DefaultRoomStatusTests.swift +++ b/Tests/AblyChatTests/DefaultRoomStatusTests.swift @@ -1,20 +1,21 @@ @testable import AblyChat -import XCTest +import Testing -class DefaultRoomStatusTests: XCTestCase { - func test_current_startsAsInitialized() async { +struct DefaultRoomStatusTests { + @Test + func current_startsAsInitialized() async { let status = DefaultRoomStatus(logger: TestLogger()) - let current = await status.current - XCTAssertEqual(current, .initialized) + #expect(await status.current == .initialized) } - func test_error_startsAsNil() async { + @Test() + func error_startsAsNil() async { let status = DefaultRoomStatus(logger: TestLogger()) - let error = await status.error - XCTAssertNil(error) + #expect(await status.error == nil) } - func test_transition() async { + @Test + func transition() async throws { // Given: A RoomStatus let status = DefaultRoomStatus(logger: TestLogger()) let originalState = await status.current @@ -30,17 +31,11 @@ class DefaultRoomStatusTests: XCTestCase { await status.transition(to: newState) // Then: It emits a status change to all subscribers added via onChange(bufferingPolicy:), and updates its `current` property to the new state - guard let statusChange1 = await statusChange1, let statusChange2 = await statusChange2 else { - XCTFail("Expected status changes to be emitted") - return + for statusChange in try await [#require(statusChange1), #require(statusChange2)] { + #expect(statusChange.previous == originalState) + #expect(statusChange.current == newState) } - for statusChange in [statusChange1, statusChange2] { - XCTAssertEqual(statusChange.previous, originalState) - XCTAssertEqual(statusChange.current, newState) - } - - let current = await status.current - XCTAssertEqual(current, .attached) + #expect(await status.current == .attached) } } diff --git a/Tests/AblyChatTests/DefaultRoomTests.swift b/Tests/AblyChatTests/DefaultRoomTests.swift index d886dae0..8956a7e0 100644 --- a/Tests/AblyChatTests/DefaultRoomTests.swift +++ b/Tests/AblyChatTests/DefaultRoomTests.swift @@ -1,9 +1,10 @@ import Ably @testable import AblyChat -import XCTest +import Testing -class DefaultRoomTests: XCTestCase { - func test_attach_attachesAllChannels_andSucceedsIfAllSucceed() async throws { +struct DefaultRoomTests { + @Test + func attach_attachesAllChannels_andSucceedsIfAllSucceed() async throws { // Given: a DefaultRoom instance with ID "basketball", with a Realtime client for which `attach(_:)` completes successfully if called on the following channels: // // - basketball::$chat::$chatMessages @@ -26,19 +27,15 @@ class DefaultRoomTests: XCTestCase { // Then: `attach(_:)` is called on each of the channels, the room `attach` call succeeds, and the room transitions to ATTACHED for channel in channelsList { - XCTAssertTrue(channel.attachCallCounter.isNonZero) + #expect(channel.attachCallCounter.isNonZero) } - guard let attachedStatusChange = await attachedStatusChange else { - XCTFail("Expected status change to ATTACHED but didn't get one") - return - } - let currentStatus = await room.status.current - XCTAssertEqual(currentStatus, .attached) - XCTAssertEqual(attachedStatusChange.current, .attached) + #expect(await room.status.current == .attached) + #expect(try #require(await attachedStatusChange).current == .attached) } - func test_attach_attachesAllChannels_andFailsIfOneFails() async throws { + @Test + func attach_attachesAllChannels_andFailsIfOneFails() async throws { // Given: a DefaultRoom instance, with a Realtime client for which `attach(_:)` completes successfully if called on the following channels: // // - basketball::$chat::$chatMessages @@ -65,11 +62,11 @@ class DefaultRoomTests: XCTestCase { } // Then: the room `attach` call fails with the same error as the channel `attach(_:)` call - let roomAttachErrorInfo = try XCTUnwrap(roomAttachError as? ARTErrorInfo) - XCTAssertIdentical(roomAttachErrorInfo, channelAttachError) + #expect(try #require(roomAttachError as? ARTErrorInfo) === channelAttachError) } - func test_detach_detachesAllChannels_andSucceedsIfAllSucceed() async throws { + @Test + func detach_detachesAllChannels_andSucceedsIfAllSucceed() async throws { // Given: a DefaultRoom instance with ID "basketball", with a Realtime client for which `detach(_:)` completes successfully if called on the following channels: // // - basketball::$chat::$chatMessages @@ -92,19 +89,15 @@ class DefaultRoomTests: XCTestCase { // Then: `detach(_:)` is called on each of the channels, the room `detach` call succeeds, and the room transitions to DETACHED for channel in channelsList { - XCTAssertTrue(channel.detachCallCounter.isNonZero) + #expect(channel.detachCallCounter.isNonZero) } - guard let detachedStatusChange = await detachedStatusChange else { - XCTFail("Expected status change to DETACHED but didn't get one") - return - } - let currentStatus = await room.status.current - XCTAssertEqual(currentStatus, .detached) - XCTAssertEqual(detachedStatusChange.current, .detached) + #expect(await room.status.current == .detached) + #expect(try #require(await detachedStatusChange).current == .detached) } - func test_detach_detachesAllChannels_andFailsIfOneFails() async throws { + @Test + func detach_detachesAllChannels_andFailsIfOneFails() async throws { // Given: a DefaultRoom instance, with a Realtime client for which `detach(_:)` completes successfully if called on the following channels: // // - basketball::$chat::$chatMessages @@ -131,7 +124,6 @@ class DefaultRoomTests: XCTestCase { } // Then: the room `detach` call fails with the same error as the channel `detach(_:)` call - let roomDetachErrorInfo = try XCTUnwrap(roomDetachError as? ARTErrorInfo) - XCTAssertIdentical(roomDetachErrorInfo, channelDetachError) + #expect(try #require(roomDetachError as? ARTErrorInfo) === channelDetachError) } } diff --git a/Tests/AblyChatTests/DefaultRoomsTests.swift b/Tests/AblyChatTests/DefaultRoomsTests.swift index 48ec802b..e4e3bd93 100644 --- a/Tests/AblyChatTests/DefaultRoomsTests.swift +++ b/Tests/AblyChatTests/DefaultRoomsTests.swift @@ -1,9 +1,10 @@ @testable import AblyChat -import XCTest +import Testing -class DefaultRoomsTests: XCTestCase { +struct DefaultRoomsTests { // @spec CHA-RC1a - func test_get_returnsRoomWithGivenID() async throws { + @Test + func get_returnsRoomWithGivenID() async throws { // Given: an instance of DefaultRooms let realtime = MockRealtime.create() let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger()) @@ -14,14 +15,15 @@ class DefaultRoomsTests: XCTestCase { let room = try await rooms.get(roomID: roomID, options: options) // Then: It returns a DefaultRoom instance that uses the same Realtime instance, with the given ID and options - let defaultRoom = try XCTUnwrap(room as? DefaultRoom) - XCTAssertIdentical(defaultRoom.realtime, realtime) - XCTAssertEqual(defaultRoom.roomID, roomID) - XCTAssertEqual(defaultRoom.options, options) + let defaultRoom = try #require(room as? DefaultRoom) + #expect(defaultRoom.realtime === realtime) + #expect(defaultRoom.roomID == roomID) + #expect(defaultRoom.options == options) } // @spec CHA-RC1b - func test_get_returnsExistingRoomWithGivenID() async throws { + @Test + func get_returnsExistingRoomWithGivenID() async throws { // Given: an instance of DefaultRooms, on which get(roomID:options:) has already been called with a given ID let realtime = MockRealtime.create() let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger()) @@ -34,11 +36,12 @@ class DefaultRoomsTests: XCTestCase { let secondRoom = try await rooms.get(roomID: roomID, options: options) // Then: It returns the same room object - XCTAssertIdentical(secondRoom, firstRoom) + #expect(secondRoom === firstRoom) } // @spec CHA-RC1c - func test_get_throwsErrorWhenOptionsDoNotMatch() async throws { + @Test + func get_throwsErrorWhenOptionsDoNotMatch() async throws { // Given: an instance of DefaultRooms, on which get(roomID:options:) has already been called with a given ID and options let realtime = MockRealtime.create() let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger()) @@ -59,6 +62,6 @@ class DefaultRoomsTests: XCTestCase { } // Then: It throws an inconsistentRoomOptions error - try assertIsChatError(caughtError, withCode: .inconsistentRoomOptions) + #expect(isChatError(caughtError, withCode: .inconsistentRoomOptions)) } } diff --git a/Tests/AblyChatTests/Helpers/Helpers.swift b/Tests/AblyChatTests/Helpers/Helpers.swift index 669c23ed..bbaff667 100644 --- a/Tests/AblyChatTests/Helpers/Helpers.swift +++ b/Tests/AblyChatTests/Helpers/Helpers.swift @@ -1,15 +1,15 @@ import Ably @testable import AblyChat -import XCTest /** - Asserts that a given optional `Error` is an `ARTErrorInfo` in the chat error domain with a given code. + Tests whether a given optional `Error` is an `ARTErrorInfo` in the chat error domain with a given code. */ -func assertIsChatError(_ maybeError: (any Error)?, withCode code: AblyChat.ErrorCode, file: StaticString = #filePath, line: UInt = #line) throws { - let error = try XCTUnwrap(maybeError, "Expected an error", file: file, line: line) - let ablyError = try XCTUnwrap(error as? ARTErrorInfo, "Expected an ARTErrorInfo", file: file, line: line) +func isChatError(_ maybeError: (any Error)?, withCode code: AblyChat.ErrorCode) -> Bool { + guard let ablyError = maybeError as? ARTErrorInfo else { + return false + } - XCTAssertEqual(ablyError.domain, AblyChat.errorDomain as String, file: file, line: line) - XCTAssertEqual(ablyError.code, code.rawValue, file: file, line: line) - XCTAssertEqual(ablyError.statusCode, code.statusCode, file: file, line: line) + return ablyError.domain == AblyChat.errorDomain as String + && ablyError.code == code.rawValue + && ablyError.statusCode == code.statusCode } diff --git a/Tests/AblyChatTests/InternalLoggerTests.swift b/Tests/AblyChatTests/InternalLoggerTests.swift index 3c91dd33..1dece2be 100644 --- a/Tests/AblyChatTests/InternalLoggerTests.swift +++ b/Tests/AblyChatTests/InternalLoggerTests.swift @@ -1,17 +1,18 @@ @testable import AblyChat -import XCTest +import Testing -class InternalLoggerTests: XCTestCase { - func test_protocolExtension_logMessage_defaultArguments_populatesFileIDAndLine() throws { +struct InternalLoggerTests { + @Test + func protocolExtension_logMessage_defaultArguments_populatesFileIDAndLine() throws { let logger = MockInternalLogger() let expectedLine = #line + 1 logger.log(message: "Here is a message", level: .info) - let receivedArguments = try XCTUnwrap(logger.logArguments) + let receivedArguments = try #require(logger.logArguments) - XCTAssertEqual(receivedArguments.level, .info) - XCTAssertEqual(receivedArguments.message, "Here is a message") - XCTAssertEqual(receivedArguments.codeLocation, .init(fileID: #fileID, line: expectedLine)) + #expect(receivedArguments.level == .info) + #expect(receivedArguments.message == "Here is a message") + #expect(receivedArguments.codeLocation == .init(fileID: #fileID, line: expectedLine)) } } diff --git a/Tests/AblyChatTests/MessageSubscriptionTests.swift b/Tests/AblyChatTests/MessageSubscriptionTests.swift index b002afb6..5babe7b3 100644 --- a/Tests/AblyChatTests/MessageSubscriptionTests.swift +++ b/Tests/AblyChatTests/MessageSubscriptionTests.swift @@ -1,6 +1,6 @@ @testable import AblyChat import AsyncAlgorithms -import XCTest +import Testing private final class MockPaginatedResult: PaginatedResult { var items: [T] { fatalError("Not implemented") } @@ -18,21 +18,20 @@ private final class MockPaginatedResult: PaginatedResult { init() {} } -class MessageSubscriptionTests: XCTestCase { +struct MessageSubscriptionTests { let messages = ["First", "Second"].map { text in Message(timeserial: "", clientID: "", roomID: "", text: text, createdAt: .init(), metadata: [:], headers: [:]) } - func testWithMockAsyncSequence() async { + @Test + func withMockAsyncSequence() async { let subscription = MessageSubscription(mockAsyncSequence: messages.async) { _ in fatalError("Not implemented") } - async let emittedElements = Array(subscription.prefix(2)) - - let awaitedEmittedElements = await emittedElements - XCTAssertEqual(awaitedEmittedElements.map(\.text), ["First", "Second"]) + #expect(await Array(subscription.prefix(2)).map(\.text) == ["First", "Second"]) } - func testEmit() async { + @Test + func emit() async { let subscription = MessageSubscription(bufferingPolicy: .unbounded) async let emittedElements = Array(subscription.prefix(2)) @@ -40,17 +39,17 @@ class MessageSubscriptionTests: XCTestCase { subscription.emit(messages[0]) subscription.emit(messages[1]) - let awaitedEmittedElements = await emittedElements - XCTAssertEqual(awaitedEmittedElements.map(\.text), ["First", "Second"]) + #expect(await emittedElements.map(\.text) == ["First", "Second"]) } - func testMockGetPreviousMessages() async throws { + @Test + func mockGetPreviousMessages() async throws { let mockPaginatedResult = MockPaginatedResult() let subscription = MessageSubscription(mockAsyncSequence: [].async) { _ in mockPaginatedResult } let result = try await subscription.getPreviousMessages(params: .init()) // This dance is to avoid the compiler error "Runtime support for parameterized protocol types is only available in iOS 16.0.0 or newer" — casting back to a concrete type seems to avoid this - let resultAsConcreteType = try XCTUnwrap(result as? MockPaginatedResult) - XCTAssertIdentical(resultAsConcreteType, mockPaginatedResult) + let resultAsConcreteType = try #require(result as? MockPaginatedResult) + #expect(resultAsConcreteType === mockPaginatedResult) } } diff --git a/Tests/AblyChatTests/SubscriptionTests.swift b/Tests/AblyChatTests/SubscriptionTests.swift index a3a80d59..a8f7d434 100644 --- a/Tests/AblyChatTests/SubscriptionTests.swift +++ b/Tests/AblyChatTests/SubscriptionTests.swift @@ -1,18 +1,17 @@ @testable import AblyChat import AsyncAlgorithms -import XCTest +import Testing -class SubscriptionTests: XCTestCase { - func testWithMockAsyncSequence() async { +struct SubscriptionTests { + @Test + func withMockAsyncSequence() async { let subscription = Subscription(mockAsyncSequence: ["First", "Second"].async) - async let emittedElements = Array(subscription.prefix(2)) - - let awaitedEmittedElements = await emittedElements - XCTAssertEqual(awaitedEmittedElements, ["First", "Second"]) + #expect(await Array(subscription.prefix(2)) == ["First", "Second"]) } - func testEmit() async { + @Test + func emit() async { let subscription = Subscription(bufferingPolicy: .unbounded) async let emittedElements = Array(subscription.prefix(2)) @@ -20,7 +19,6 @@ class SubscriptionTests: XCTestCase { subscription.emit("First") subscription.emit("Second") - let awaitedEmittedElements = await emittedElements - XCTAssertEqual(awaitedEmittedElements, ["First", "Second"]) + #expect(await emittedElements == ["First", "Second"]) } } From cfc1fc03ab9ed8dbb48bf9aedd2b7c936ee21eb5 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 24 Sep 2024 10:01:35 -0300 Subject: [PATCH 2/4] Update comment about AsyncSequence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can’t implement some of the #56 improvements that I hoped Swift 6 would enable, due to them requiring OS support. --- Sources/AblyChat/Subscription.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/AblyChat/Subscription.swift b/Sources/AblyChat/Subscription.swift index b07f6a39..2866eab0 100644 --- a/Sources/AblyChat/Subscription.swift +++ b/Sources/AblyChat/Subscription.swift @@ -4,7 +4,7 @@ // // At some point we should define how this thing behaves when you iterate over it from multiple loops, or when you pass it around. I’m not yet sufficiently experienced with `AsyncSequence` to know what’s idiomatic. I tried the same thing out with `AsyncStream` (two tasks iterating over a single stream) and it appears that each element is delivered to precisely one consumer. But we can leave that for later. On a similar note consider whether it makes a difference whether this is a struct or a class. // -// TODO: I wanted to implement this as a protocol (from which `MessageSubscription` would then inherit) but struggled to do so, hence the struct. Try again sometime. We can also revisit our implementation of `AsyncSequence` if we migrate to Swift 6, which adds primary types and typed errors to `AsyncSequence` and should make things easier; see https://github.com/ably-labs/ably-chat-swift/issues/21. +// I wanted to implement this as a protocol (from which `MessageSubscription` would then inherit) but struggled to do so (see https://forums.swift.org/t/struggling-to-create-a-protocol-that-inherits-from-asyncsequence-with-primary-associated-type/73950 where someone suggested it’s a compiler bug), hence the struct. I was also hoping that upon switching to Swift 6 we could use AsyncSequence’s `Failure` associated type to simplify the way in which we show that the subscription is non-throwing, but it turns out this can only be done in macOS 15 etc. So I think that for now we’re stuck with things the way they are. public struct Subscription: Sendable, AsyncSequence { private enum Mode: Sendable { case `default`(stream: AsyncStream, continuation: AsyncStream.Continuation) From 0904bfeeb5a9890ffc19d37dec14351a136a9c7b Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 24 Sep 2024 17:03:31 -0300 Subject: [PATCH 3/4] Remove Swift version check Missed this in 67f4563. --- .../AblyCocoaExtensions/Ably+Dependencies.swift | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/Sources/AblyChat/AblyCocoaExtensions/Ably+Dependencies.swift b/Sources/AblyChat/AblyCocoaExtensions/Ably+Dependencies.swift index 81eeee92..b193a9fe 100644 --- a/Sources/AblyChat/AblyCocoaExtensions/Ably+Dependencies.swift +++ b/Sources/AblyChat/AblyCocoaExtensions/Ably+Dependencies.swift @@ -2,17 +2,9 @@ import Ably // TODO: remove "@unchecked Sendable" once https://github.com/ably/ably-cocoa/issues/1962 done -#if swift(>=6) - // This @retroactive is needed to silence the Swift 6 compiler error "extension declares a conformance of imported type 'ARTRealtimeChannels' to imported protocol 'Sendable'; this will not behave correctly if the owners of 'Ably' introduce this conformance in the future (…) add '@retroactive' to silence this warning". I don’t fully understand the implications of this but don’t really mind since both libraries are in our control. - extension ARTRealtime: RealtimeClientProtocol, @retroactive @unchecked Sendable {} +// This @retroactive is needed to silence the Swift 6 compiler error "extension declares a conformance of imported type 'ARTRealtimeChannels' to imported protocol 'Sendable'; this will not behave correctly if the owners of 'Ably' introduce this conformance in the future (…) add '@retroactive' to silence this warning". I don’t fully understand the implications of this but don’t really mind since both libraries are in our control. +extension ARTRealtime: RealtimeClientProtocol, @retroactive @unchecked Sendable {} - extension ARTRealtimeChannels: RealtimeChannelsProtocol, @retroactive @unchecked Sendable {} +extension ARTRealtimeChannels: RealtimeChannelsProtocol, @retroactive @unchecked Sendable {} - extension ARTRealtimeChannel: RealtimeChannelProtocol, @retroactive @unchecked Sendable {} -#else - extension ARTRealtime: RealtimeClientProtocol, @unchecked Sendable {} - - extension ARTRealtimeChannels: RealtimeChannelsProtocol, @unchecked Sendable {} - - extension ARTRealtimeChannel: RealtimeChannelProtocol, @unchecked Sendable {} -#endif +extension ARTRealtimeChannel: RealtimeChannelProtocol, @retroactive @unchecked Sendable {} From 8184232d07c780f45ea98941f8f643308dd94183 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 26 Sep 2024 09:27:16 -0300 Subject: [PATCH 4/4] Remove use of ARTRealtimePresence query in public API See comment; the problem described there arose when Marat tried to implement a mock of Presence for #4. --- Sources/AblyChat/Presence.swift | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/Sources/AblyChat/Presence.swift b/Sources/AblyChat/Presence.swift index 8d269245..148b571d 100644 --- a/Sources/AblyChat/Presence.swift +++ b/Sources/AblyChat/Presence.swift @@ -5,7 +5,7 @@ public typealias PresenceData = any Sendable public protocol Presence: AnyObject, Sendable, EmitsDiscontinuities { func get() async throws -> [PresenceMember] - func get(params: ARTRealtimePresenceQuery?) async throws -> [PresenceMember] + func get(params: PresenceQuery?) async throws -> [PresenceMember] func isUserPresent(clientID: String) async throws -> Bool func enter() async throws func enter(data: PresenceData) async throws @@ -61,3 +61,24 @@ public struct PresenceEvent: Sendable { self.data = data } } + +// This is a Sendable equivalent of ably-cocoa’s ARTRealtimePresenceQuery type. +// +// Originally, ``Presence.get(params:)`` accepted an ARTRealtimePresenceQuery object, but I’ve changed it to accept this type, because else when you try and write an actor that implements ``Presence``, you get a compiler error like "Non-sendable type 'ARTRealtimePresenceQuery' in parameter of the protocol requirement satisfied by actor-isolated instance method 'get(params:)' cannot cross actor boundary; this is an error in the Swift 6 language mode". +// +// Now, based on my limited understanding, you _should_ be able to send non-Sendable values from one isolation domain to another (the purpose of the "region-based isolation" and "`sending` parameters" features added in Swift 6), but to get this to work I had to mark ``Presence`` as requiring conformance to the `Actor` protocol, and since I didn’t understand _why_ I had to do that, I didn’t want to put it in the public API. +// +// So, for now, let’s just accept this copy (which I don’t think is a big problem anyway); we can always revisit it with more Swift concurrency knowledge in the future. Created https://github.com/ably-labs/ably-chat-swift/issues/64 to revisit. +public struct PresenceQuery: Sendable { + public var limit = 100 + public var clientID: String? + public var connectionID: String? + public var waitForSync = true + + internal init(limit: Int = 100, clientID: String? = nil, connectionID: String? = nil, waitForSync: Bool = true) { + self.limit = limit + self.clientID = clientID + self.connectionID = connectionID + self.waitForSync = waitForSync + } +}