Skip to content

Commit

Permalink
Add a convention for exposing APIs to tests
Browse files Browse the repository at this point in the history
I had noticed that I was writing lots of comments like "Exposed for
testing" and thought it would be useful to do something more consistent
and lintable.
  • Loading branch information
lawrence-forooghian committed Sep 30, 2024
1 parent b35b114 commit 78fd1be
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 13 deletions.
10 changes: 7 additions & 3 deletions .swiftlint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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_<identifier> convention)
- _

# For compatibility with SwiftFormat
trailing_comma:
mandatory_comma: true
21 changes: 21 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 whose `internal` access level is 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 `testOnly` are not being used inside the SDK; we’ll do this in #70.
- I believe that we should be able to implement the pattern of re-exposing a `private` member as a `testOnly` 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.
18 changes: 15 additions & 3 deletions Sources/AblyChat/Logging.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
8 changes: 7 additions & 1 deletion Sources/AblyChat/Room.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions Sources/AblyChat/Rooms.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Tests/AblyChatTests/DefaultChatClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
4 changes: 2 additions & 2 deletions Tests/AblyChatTests/DefaultInternalLoggerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Tests/AblyChatTests/DefaultRoomsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit 78fd1be

Please sign in to comment.