Automattic / Automattic/wordpress-rs

Add Swift/Kotlin binding-layer tests for the connectivity predicates

Open
#1,496 0 comments 0 reactions 0 assignees View on GitHub
Error Handling Kotlin Swift
Dominant language
Rust
Stars
36
Forks
5
Avg merge
17h 30m
Merged PRs (30d)
43

Description

The connectivity predicates added in [#1488]() (`isSiteUnreachable` / `isDeviceOffline`) have thorough **Rust** unit coverage (every `RequestExecutionErrorReason` variant, plus the exported UniFFI functions), but the **Swift and Kotlin binding layer has no tests**. The untested glue is exactly the part most likely to break silently:

* Swift `CarriesRequestExecutionErrorReason` — the `executionErrorReason` `if case .RequestExecutionFailed(…)` extraction on `WpApiError` and `RequestExecutionError`, and the `?? false` fallback for non-execution errors.
* Kotlin — the extension-property delegation to the exported functions.

## Why it matters

A UniFFI regen that renamed the `reason:` associated-value label (or reshaped the `RequestExecutionFailed` payload) would make the Swift `if case` stop matching — `executionErrorReason` returns `nil`, both predicates silently return `false`, and nothing catches it. The structurally identical `WpApiError.isCancellationError` *is* covered (integration tests); these new predicates are not.

## Sequencing — do this after the behaviour-changing PRs land

The stable, value-level assertions below (NonExistentSite → unreachable, DeviceIsOffline → offline, the protocol extraction, the fallback) are safe to write now. But the classification of **timeouts** (Automattic/wordpress-rs#1491), **cancellation** (Automattic/wordpress-rs#1492), and **refused connections** (Automattic/wordpress-rs#1495) is still in flux, so a test asserting "what a timeout/cancellation/refused maps to" should wait for those to resolve — otherwise the assertions get rewritten. Landing the whole test pass *after* those PRs lets it assert final behaviour and extend naturally to cover the new classifications in one go.

## Draft tests

Grounded against the real generated API (`Exports.swift` re-exports; the `.RequestExecutionFailed(statusCode:redirects:reason:requestUrl:requestMethod:)` labels and `requestMethod: .get` from `SafeRequestExecutor.swift`/`Extensions.swift`; Kotlin construction from `WpRequestExecutor.kt`) but **not yet compiled** — CI is the validator. The most likely thing needing a tweak is an exact associated-value label.

### Swift — `native/swift/Tests/wordpress-api/ConnectivityPredicatesTests.swift`

```swift
import Foundation
import Testing
import WordPressAPI
import WordPressAPIInternal

@Suite("Connectivity predicates")
struct ConnectivityPredicatesTests {

private func nonExistentSite() -> RequestExecutionErrorReason {
.nonExistentSiteError(errorMessage: nil, suggestedAction: nil)
}

private func deviceOffline() -> RequestExecutionErrorReason {
.deviceIsOfflineError(errorMessage: "offline")
}

// MARK: RequestExecutionErrorReason (delegates to the exported UniFFI fns)

@Test("NonExistentSite is unreachable, not offline")
func nonExistentSiteReason() {
let reason = nonExistentSite()
#expect(reason.isSiteUnreachable)
#expect(!reason.isDeviceOffline)
}

@Test("DeviceIsOffline is offline, not unreachable")
func deviceOfflineReason() {
let reason = deviceOffline()
#expect(reason.isDeviceOffline)
#expect(!reason.isSiteUnreachable)
}

@Test("A non-connectivity reason is neither")
func otherReason() {
let reason = RequestExecutionErrorReason.cancellationError
#expect(!reason.isSiteUnreachable)
#expect(!reason.isDeviceOffline)
}

// MARK: CarriesRequestExecutionErrorReason on WpApiError

@Test("WpApiError.RequestExecutionFailed surfaces the reason + predicates")
func wpApiErrorCarriesReason() {
let error = WpApiError.RequestExecutionFailed(
statusCode: nil,
redirects: nil,
reason: nonExistentSite(),
requestUrl: "https://example.com",
requestMethod: .get
)
#expect(error.executionErrorReason != nil)
#expect(error.isSiteUnreachable)
#expect(!error.isDeviceOffline)
}

@Test("A non-execution WpApiError has no reason and falls back to false")
func wpApiErrorFallback() {
let error = WpApiError.SiteUrlParsingError(reason: "bad url")
#expect(error.executionErrorReason == nil)
#expect(!error.isSiteUnreachable) // exercises `executionErrorReason?... ?? false`
#expect(!error.isDeviceOffline)
}

// MARK: CarriesRequestExecutionErrorReason on RequestExecutionError

@Test("RequestExecutionError.RequestExecutionFailed surfaces the reason + predicates")
func requestExecutionErrorCarriesReason() {
let error = RequestExecutionError.RequestExecutionFailed(
statusCode: nil,
redirects: nil,
reason: deviceOffline(),
requestUrl: "https://example.com",
requestMethod: .get
)
#expect(error.executionErrorReason != nil)
#expect(error.isDeviceOffline)
#expect(!error.isSiteUnreachable)
}

@Test("A RequestExecutionError.MediaFileNotFound has no reason")
func requestExecutionErrorFallback() {
let error = RequestExecutionError.MediaFileNotFound(filePath: "/tmp/x")
#expect(error.executionErrorReason == nil)
#expect(!error.isSiteUnreachable)
#expect(!error.isDeviceOffline)
}
}
```

### Kotlin — `.../src/integrationTest/kotlin/RequestExecutionErrorReasonExtensionsTest.kt`

Kotlin exposes the predicates only on the reason (no error-type accessors — that convenience is Swift-only), so there's less surface. Same package as the extensions, so no import needed for them:

```kotlin
package rs.wordpress.api.kotlin

import org.junit.jupiter.api.Test
import uniffi.wp_api.RequestExecutionErrorReason
import kotlin.test.assertFalse
import kotlin.test.assertTrue

class RequestExecutionErrorReasonExtensionsTest {

@Test
fun `NonExistentSite is unreachable, not offline`() {
val reason = RequestExecutionErrorReason.NonExistentSiteError(
errorMessage = null,
suggestedAction = null,
)
assertTrue(reason.isSiteUnreachable)
assertFalse(reason.isDeviceOffline)
}

@Test
fun `DeviceIsOffline is offline, not unreachable`() {
val reason = RequestExecutionErrorReason.DeviceIsOfflineError(errorMessage = "offline")
assertTrue(reason.isDeviceOffline)
assertFalse(reason.isSiteUnreachable)
}

@Test
fun `A refused connection (HttpError) is neither`() {
val reason = RequestExecutionErrorReason.HttpError(reason = "connection failed")
assertFalse(reason.isSiteUnreachable)
assertFalse(reason.isDeviceOffline)
}
}
```

## Placement notes

* **Swift** goes in the existing `Tests/wordpress-api` unit target — no infra needed.
* **Kotlin** has no `src/test` unit source set (only `src/main` and `src/integrationTest`), so the draft is parked in `integrationTest` alongside `WpRequestExecutorTest`. It's a pure value test needing no server; if a real unit source set is preferred, that's a small `build.gradle.kts` addition.

## Provenance

F5 from the review of [#1488](). The predicates themselves are documented in [#1493](); the behavioural gaps to resolve first are Automattic/wordpress-rs#1491 (Swift timeouts), Automattic/wordpress-rs#1492 (Kotlin cancellation), and Automattic/wordpress-rs#1495 (refused-connection classification).

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with native/swift/Tests/wordpress-api/ConnectivityPredicatesTests.swift and the Kotlin integration-test source set alongside WpRequestExecutorTest. Run the existing Swift and Kotlin test commands first, then validate the draft constructions against the generated API. Done means both suites compile and cover the listed connectivity reasons, error extraction, and false fallbacks after the referenced behavior changes land.

Written by the indexing model from the issue text.

Assessment

Tech stack
kotlin, rust, swift
Domain
mobile-dev, testing
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
67/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.