diff --git a/.swiftlint.yml b/.swiftlint.yml index f27ca7f1..e50be7c2 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -92,17 +92,21 @@ file_header: # // forbidden_pattern: // Created by -identifier_name: +type_name: &no_length_checks # We disable the length checks, for the same reason we disable the rules of type "metrics". min_length: warning: 1 max_length: warning: 10000 -type_name: *no_length_checks - generic_type_name: *no_length_checks +identifier_name: + <<: *no_length_checks + allowed_symbols: + # Allow underscore; it can be handy for adding a prefix to another identifier (e.g. our testsOnly_ convention) + - _ + # For compatibility with SwiftFormat trailing_comma: mandatory_comma: true diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1771ced8..3f9ed413 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,3 +28,24 @@ 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.) + +### Exposing test-only APIs + +When writing unit tests, there are times that we need to access internal state of a type. To enable this, we might expose this state at an `internal` access level so that it can be used by the unit tests. However, we want to make it clear that this state is being exposed _purely_ for the purposes of testing that class, and that it should not be used by other components of the SDK. + +So, when writing an API which has `internal` access level purely to enable it to be called by the tests, prefix this API’s name with `testOnly_`. For example: + +```swift +private nonisolated let realtime: RealtimeClient + +#if DEBUG + internal nonisolated var testsOnly_realtime: RealtimeClient { + realtime + } +#endif +``` + +A couple of notes: + +- Using a naming convention will allow us to verify that APIs marked `testsOnly` are not being used inside the SDK; we’ll do this in #70. +- I believe that we should be able to eliminate the boilerplate of re-exposing a `private` member as a `testsOnly` member (as exemplified above) using a macro (called e.g. `@ExposedToTests`), but my level of experience with macros is insufficient to be confident about being able to do this quickly, so have deferred it to #71. diff --git a/Sources/AblyChat/Logging.swift b/Sources/AblyChat/Logging.swift index fe95943a..dbfd41e1 100644 --- a/Sources/AblyChat/Logging.swift +++ b/Sources/AblyChat/Logging.swift @@ -44,9 +44,21 @@ extension InternalLogger { } internal final class DefaultInternalLogger: InternalLogger { - // Exposed for testing. - internal let logHandler: LogHandler - internal let logLevel: LogLevel + private let logHandler: LogHandler + + #if DEBUG + internal var testsOnly_logHandler: LogHandler { + logHandler + } + #endif + + private let logLevel: LogLevel + + #if DEBUG + internal var testsOnly_logLevel: LogLevel { + logLevel + } + #endif internal init(logHandler: LogHandler?, logLevel: LogLevel?) { self.logHandler = logHandler ?? DefaultLogHandler() diff --git a/Sources/AblyChat/Room.swift b/Sources/AblyChat/Room.swift index e7209319..e4660c8d 100644 --- a/Sources/AblyChat/Room.swift +++ b/Sources/AblyChat/Room.swift @@ -22,7 +22,13 @@ internal actor DefaultRoom: Room { internal nonisolated let options: RoomOptions // Exposed for testing. - internal nonisolated let realtime: RealtimeClient + private nonisolated let realtime: RealtimeClient + + #if DEBUG + internal nonisolated var testsOnly_realtime: RealtimeClient { + realtime + } + #endif private let _status: DefaultRoomStatus private let logger: InternalLogger diff --git a/Sources/AblyChat/Rooms.swift b/Sources/AblyChat/Rooms.swift index 12fd6ef1..87f1d7c6 100644 --- a/Sources/AblyChat/Rooms.swift +++ b/Sources/AblyChat/Rooms.swift @@ -7,8 +7,14 @@ public protocol Rooms: AnyObject, Sendable { } internal actor DefaultRooms: Rooms { - /// Exposed so that we can test it. - internal nonisolated let realtime: RealtimeClient + private nonisolated let realtime: RealtimeClient + + #if DEBUG + internal nonisolated var testsOnly_realtime: RealtimeClient { + realtime + } + #endif + internal nonisolated let clientOptions: ClientOptions private let logger: InternalLogger diff --git a/Tests/AblyChatTests/DefaultChatClientTests.swift b/Tests/AblyChatTests/DefaultChatClientTests.swift index 1f207ae2..569322c4 100644 --- a/Tests/AblyChatTests/DefaultChatClientTests.swift +++ b/Tests/AblyChatTests/DefaultChatClientTests.swift @@ -23,7 +23,7 @@ struct DefaultChatClientTests { let rooms = client.rooms let defaultRooms = try #require(rooms as? DefaultRooms) - #expect(defaultRooms.realtime === realtime) + #expect(defaultRooms.testsOnly_realtime === realtime) #expect(defaultRooms.clientOptions.isEqualForTestPurposes(options)) } } diff --git a/Tests/AblyChatTests/DefaultInternalLoggerTests.swift b/Tests/AblyChatTests/DefaultInternalLoggerTests.swift index a40bf223..9e8f80ea 100644 --- a/Tests/AblyChatTests/DefaultInternalLoggerTests.swift +++ b/Tests/AblyChatTests/DefaultInternalLoggerTests.swift @@ -6,8 +6,8 @@ struct DefaultInternalLoggerTests { func defaults() { let logger = DefaultInternalLogger(logHandler: nil, logLevel: nil) - #expect(logger.logHandler is DefaultLogHandler) - #expect(logger.logLevel == .error) + #expect(logger.testsOnly_logHandler is DefaultLogHandler) + #expect(logger.testsOnly_logLevel == .error) } @Test diff --git a/Tests/AblyChatTests/DefaultRoomsTests.swift b/Tests/AblyChatTests/DefaultRoomsTests.swift index e4e3bd93..adf9fafd 100644 --- a/Tests/AblyChatTests/DefaultRoomsTests.swift +++ b/Tests/AblyChatTests/DefaultRoomsTests.swift @@ -16,7 +16,7 @@ struct DefaultRoomsTests { // Then: It returns a DefaultRoom instance that uses the same Realtime instance, with the given ID and options let defaultRoom = try #require(room as? DefaultRoom) - #expect(defaultRoom.realtime === realtime) + #expect(defaultRoom.testsOnly_realtime === realtime) #expect(defaultRoom.roomID == roomID) #expect(defaultRoom.options == options) }