Skip to content

Migrate unit tests from XCTest to Swift Testing#404

Draft
eseay wants to merge 21 commits into
mainfrom
eddie/swift-testing
Draft

Migrate unit tests from XCTest to Swift Testing#404
eseay wants to merge 21 commits into
mainfrom
eddie/swift-testing

Conversation

@eseay

@eseay eseay commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Migrates all 15 XCTest suites under Tests/UnitTests (both ConnectLibraryTests and ConnectPluginUtilitiesTests) to Swift Testing, one suite per commit for reviewability.

  • XCTestCase classes → struct suites; XCTAssert*#expect, XCTUnwrap#require, XCTFailIssue.record, error-inspection closures → typed #expect(throws:).
  • The four near-identical FilePathComponentsTests methods are collapsed into a single parameterized @Test(arguments:).
  • Every @Test function carries @available(iOS 13, *) because the package's iOS floor is 12, below Swift Testing's iOS 13 minimum. The annotation cannot live on the suite type: the @Test macro rejects members of @available types ("Attribute 'Test' cannot be applied to this function because it has been marked '@available(iOS 13, *)'"), so per-function is the only supported form.
  • InterceptorIntegrationTests is @Suite(.serialized) (documented in-source): Swift Testing parallelizes by default, and these tests share the live conformance reference server and assert timing-sensitive URLSession metrics. Its sleep(1) is now a cooperative Task.sleep so it no longer blocks a pool thread.
  • Removes the now-dead XCTest lint rules (empty_xctest_method, xct_specific_matcher, custom xctestcase) and adds a swift_testing_over_xctest rule to prevent drift back to XCTest.

No Package.swift, CI, or Makefile changes were needed: swift test discovers Swift Testing tests under the existing swift-tools-version:5.6 manifest.

Test plan

  • Full swift test against a running conformance reference server (the make testunit flow): 69 tests in 15 suites pass — the original 72 methods minus the 4 collapsed into 1 parameterized test, so no test was lost in discovery.
  • xcodebuild build-for-testing for iOS Simulator succeeds, verifying the iOS 12 deployment target compiles with the availability annotations.
  • swiftlint lint --strict is clean for all touched files.
  • Multi-angle review of the full diff; findings were either remediated (non-blocking sleep, XCTest lint guard) or refuted with compiler/runtime evidence.

🤖 Generated with Claude Code

@eseay
eseay marked this pull request as draft July 19, 2026 02:08
@eseay
eseay force-pushed the eddie/swift-testing branch from a25411a to 86f3541 Compare July 19, 2026 15:18
eseay added a commit that referenced this pull request Jul 20, 2026
…-resend hang) (#407)

## Summary

Fixes the long-standing flakiness in the `run-conformance-tests` CI job,
where a run would emit a nameless `timed out waiting for result from
client` line and then, ~5s later, a burst of 80–100 `FAILED: … failed to
get result from client` lines spanning unrelated test categories.

**Root cause: the conformance client was crashing/hanging *between* test
cases, not failing individual tests.**
`Tests/ConformanceClient/Sources/main.swift` reads length-prefixed
requests from stdin and runs them **serially** — one process, one test
at a time. When the process dies or wedges between tests, the runner
times out on the in-flight test and then fails every test still queued
behind it at once. That is why the "failing" tests looked random and
unrelated each run — they were never actually run.

Two distinct underlying bugs produced this same signature; both are
fixed here.

### 1. `NIOHTTPClient` crash on deinit (SIGTRAP)

Reproduced under load with matching `.ips` crash reports. If the last
strong reference to a `NIOHTTPClient` is dropped while an
`EventLoopFuture` callback (e.g. the connect callback in
`connectChannelAndMultiplexerIfNeeded()`) holds a temporary strong
reference upgraded from `[weak self]`, the client is deallocated **on
its own event loop thread**. `deinit` then called
`syncShutdownGracefully()`, which traps with a precondition failure when
invoked from an event loop — killing the process with SIGTRAP, silently,
between test cases.

**Fix:** use the asynchronous `shutdownGracefully(_:)`, which is safe to
call from any thread including the group's own event loops.

### 2. `URLSessionStream` hang on request-body resend

Reproduced locally and in CI (runs `29702874348`, `29702882058`). Every
hung test was a streaming RPC over `URLSessionHTTPClient`; in one case
even a 2-second `TimeoutTimer` calling `task.cancel()` could not produce
a completion callback. When a connection is dropped and retried,
URLSession asks for the request body again via
`urlSession(_:task:needNewBodyStream:)`. A bound stream pair can't be
replayed, so `requestBodyStream` vends `nil` (#399). Contrary to the
assumption there, CFNetwork does **not** fail the task when given a
`nil` body stream — it can leave the task parked indefinitely with no
delegate callbacks, wedging the serial client.

**Fix:** cancel the task when a resend is requested, guaranteeing
`didCompleteWithError` is delivered, and surface the failure as
`.unavailable` with a descriptive message (rather than a misleading
`canceled`) since it is a retriable connection-level failure.

### 3. Diagnosability

The conformance client now logs `starting <test> …` / `finished <test>`
to stderr before/after each case. The runner surfaces client stderr, so
a future wedge shows exactly which test/protocol/codec/streamType was in
flight — the piece of information missing from every prior CI failure
log, and what pinpointed both bugs here.

## Validation

- **NIO crash:** 1,071 suite runs with the fix while the machine was
verifiably awake (534 nio / 537 urlsession) produced **zero NIO crashes
and zero NIO bursts**; ~17 crashes were expected at the pre-fix rate.
Unit tests: 69/69 pass.
- **URLSession hang:** validated on the real macos-26 CI runners (in
progress on this branch).
- Note for reproduction: laptop sleep fires the runner's wall-clock
timeouts and produces fake "burst" failures on wake — all local runs
were cross-checked against `pmset -g log` and sleep-window runs
discarded.

## Notes

This branch was previously stacked on #404 (Swift Testing migration); it
has been rebased directly onto `main` so this PR contains only the 3
conformance-fix commits.

---------

Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
eseay and others added 21 commits July 21, 2026 10:09
Replaces the deprecated NIOHTTP2.HTTP2StreamMultiplexer with
NIOHTTP2Handler.StreamMultiplexer (the inline stream multiplexer) and
moves pipeline configuration onto ChannelPipeline.SynchronousOperations.
This eliminates all Sendable/deprecation warnings emitted when building
ConnectNIO against recent swift-nio-http2 releases, and allows dropping
the @preconcurrency imports of NIOCore and NIOSSL.

Fixes #408.

Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Converts the ConnectPluginUtilitiesTests target from XCTest to Swift
Testing, collapsing four near-identical test methods into a single
parameterized @test. Validates that Swift Testing discovery works with
the existing swift-tools-version and that the iOS 12 deployment target
builds with @available(iOS 13, *) on test functions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Mechanical conversion from XCTest: struct suite, #expect/#require
assertions, and @available(iOS 13, *) on test functions to satisfy the
package's iOS 12 minimum deployment target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Mechanical conversion from XCTest: struct suite, #expect/#require
assertions, and @available(iOS 13, *) on test functions to satisfy the
package's iOS 12 minimum deployment target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Mechanical conversion from XCTest: struct suite, #expect/#require
assertions, and @available(iOS 13, *) on test functions to satisfy the
package's iOS 12 minimum deployment target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Mechanical conversion from XCTest: struct suite, #expect/#require
assertions, and @available(iOS 13, *) on test functions to satisfy the
package's iOS 12 minimum deployment target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Mechanical conversion from XCTest: struct suite, #expect/#require
assertions, and @available(iOS 13, *) on test functions to satisfy the
package's iOS 12 minimum deployment target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Mechanical conversion from XCTest: struct suite, #expect/#require
assertions, and @available(iOS 13, *) on test functions to satisfy the
package's iOS 12 minimum deployment target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Mechanical conversion from XCTest: struct suite, #expect/#require
assertions, and @available(iOS 13, *) on test functions to satisfy the
package's iOS 12 minimum deployment target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Mechanical conversion from XCTest: struct suite, #expect/#require
assertions, and @available(iOS 13, *) on test functions to satisfy the
package's iOS 12 minimum deployment target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Mechanical conversion from XCTest: struct suite, #expect/#require
assertions, and @available(iOS 13, *) on test functions to satisfy the
package's iOS 12 minimum deployment target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Mechanical conversion from XCTest: struct suite, #expect/#require
assertions, and @available(iOS 13, *) on test functions to satisfy the
package's iOS 12 minimum deployment target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Mechanical conversion from XCTest: struct suite, #expect/#require
assertions, and @available(iOS 13, *) on test functions to satisfy the
package's iOS 12 minimum deployment target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Mechanical conversion from XCTest: struct suite, #expect assertions, and
@available(iOS 13, *) on test functions to satisfy the package's iOS 12
minimum deployment target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Mechanical conversion from XCTest: struct suite, #expect assertions,
Issue.record in place of XCTFail, and @available(iOS 13, *) on test
functions to satisfy the package's iOS 12 minimum deployment target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Converts the suite from XCTest using struct suites and #expect
assertions. The suite is marked .serialized because Swift Testing runs
tests in parallel by default, and these tests all hit the same local
conformance reference server and validate timing-sensitive URLSession
metrics behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
All unit tests now use Swift Testing, so the empty_xctest_method and
xct_specific_matcher opt-in rules and the custom xctestcase rule no
longer apply. Also updates the UnitTests README accordingly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
sleep(1) inside an async test blocks a cooperative-pool thread for the
full second under Swift Testing's default parallel execution. Task.sleep
suspends cooperatively instead, freeing the thread for other suites,
with identical timing behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
All unit tests now use Swift Testing; this custom rule keeps new tests
from silently drifting back to XCTest.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
Bare @test (no macro arguments) now reads as one line, matching how
@IBAction and similar attributes are conventionally styled in this
codebase. The one parameterized test, @test(arguments:), keeps its own
line with a targeted swiftlint:disable since it's a different case:
it carries a macro argument list, not "simple."

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
@eseay
eseay force-pushed the eddie/swift-testing branch from 86f3541 to 660bac6 Compare July 21, 2026 16:50
@eseay
eseay changed the base branch from main to eddie/niohttpclient-warnings July 21, 2026 16:50
Base automatically changed from eddie/niohttpclient-warnings to main July 21, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant