Repository navigation
Migrate swift xctests to Swift testing tests - #187801
Conversation
github-actions
Bot
added
platform-ios
| import Testing | ||
|
|
||
| class TaskRunnerTests: XCTestCase { | ||
| @MainActor |
There was a problem hiding this comment.
@MainActor otherwise the tests here hang. Potentially because Swift testing threads don't actively pump their runloops?
There was a problem hiding this comment.
Can you put this in comment?
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates several iOS platform tests (SplashScreenManagerTests, TaskRunnerTests, and VSyncClientTests) from XCTest to the Swift Testing framework, updating assertions to #expect and converting asynchronous tests to use withCheckedContinuation. Additionally, the iOS deployment target for the test targets is updated to 15.0. Feedback suggests simplifying the tests by removing redundant #available(iOS 15.0, *) checks now that the deployment target is 15.0, and adding .timeLimit traits to asynchronous tests to prevent potential indefinite hangs in CI.
| } | ||
|
|
||
| class SplashScreenManagerTests: XCTestCase { | ||
| @Suite @MainActor |
There was a problem hiding this comment.
Can you add comment on why @mainactor is needed? What problem this is trying to solve?
There was a problem hiding this comment.
This one should be obvious once the SplashScreenManager is @MainActor so I left out the comment.
| import Testing | ||
|
|
||
| class TaskRunnerTests: XCTestCase { | ||
| @MainActor |
There was a problem hiding this comment.
Can you put this in comment?
| let expectation = self.expectation(description: "Task should be executed") | ||
| taskRunner.postTask { | ||
| expectation.fulfill() | ||
| await withCheckedContinuation { continuation in |
There was a problem hiding this comment.
Can you use the confirmation API which is designed for this kind of usage?
https://developer.apple.com/documentation/testing/testing-asynchronous-code
There was a problem hiding this comment.
Can't use this because taskRunner isn't an async API. It uses callbacks.
There was a problem hiding this comment.
So it looks like withCheckedContinuation does not have the same semantics as confirmation, as the following test fails:
@Test func proofConfirmationDoesNotWait() async {
await confirmation { confirm in
DispatchQueue.global().asyncAfter(deadline: .now() + 0.1) {
confirm()
}
}
}It seems confirmation doesn't really wait for the callback to be called.
| override func setUp() { | ||
| super.setUp() | ||
| init() { | ||
| threadTaskRunner = TaskRunnerTestHelper.makeTaskRunner(withLabel: "VSyncClientTest") |
There was a problem hiding this comment.
the new code reuses task runner, will it cause side effect down the road?
There was a problem hiding this comment.
Ah interesting, I think the tests in this file will be run concurrently so multiple tests can schedule work on that task runner concurrently. But as long as the tests themselves are hermetic it shouldn't cause problems. IOW to the tests, the shared task runner doesn't have mutable shared states.
| var client: VSyncClient? | ||
| autoreleasepool { | ||
| let client = VSyncClient( | ||
| client = VSyncClient( |
There was a problem hiding this comment.
Here strong reference outside of autoreleasepool retains the client.
There was a problem hiding this comment.
Unfortunately we need to hold a strong reference outside of the first autoreleasepool. The reference will be niled out in the autoreleasepool and that's where deallocation actually. Added a comment.
LongCatIsLooong
added
the
autosubmit
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/187801, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
f45e246 to
d01ddc9
Compare
LongCatIsLooong
added
the
autosubmit
flutter-dashboard
Bot
removed
the
autosubmit
…12401) Manual roll Flutter from 2757a77a73df to e52f01c920ad (47 revisions) Manual roll requested by [email protected] flutter/flutter@2757a77...e52f01c 2026-07-31 [email protected] [Impeller] Execute PipelineCompileQueue jobs in the order that they were added (flutter/flutter#190262) 2026-07-31 [email protected] Roll Packages from 7d17fc8 to 5351d8c (5 revisions) (flutter/flutter#190351) 2026-07-31 [email protected] Roll Skia from dcbd015c9646 to 7ef86a5b0eb9 (4 revisions) (flutter/flutter#190347) 2026-07-31 [email protected] [Impeller] Do not acquire an unneeded image in KHRSwapchainVK if the swapchain must be recreated due to a size change (flutter/flutter#190116) 2026-07-31 [email protected] Remove the --enable-serial-gc engine flag (flutter/flutter#190310) 2026-07-31 [email protected] Migrate platform_channel_swift example to material_ui (flutter/flutter#190308) 2026-07-31 [email protected] macOS: Set static text accessibility role for read-only text fields (flutter/flutter#190330) 2026-07-31 [email protected] Roll Dart SDK from 96f49fc43f59 to c3acfc2479f6 (1 revision) (flutter/flutter#190336) 2026-07-31 [email protected] Roll Skia from 07f08414dd66 to dcbd015c9646 (1 revision) (flutter/flutter#190333) 2026-07-31 [email protected] [web] Autofill all AutofillGroup fields in semantics mode (flutter/flutter#187108) 2026-07-31 [email protected] iOS: Configure test task runner thread at display QoS (flutter/flutter#190283) 2026-07-31 [email protected] Roll Skia from a1109355de78 to 07f08414dd66 (5 revisions) (flutter/flutter#190328) 2026-07-31 [email protected] Support reduceMotion accessibility feature in MediaQuery (flutter/flutter#190287) 2026-07-30 [email protected] Roll Dart SDK from 43f7de5f4977 to 96f49fc43f59 (4 revisions) (flutter/flutter#190312) 2026-07-30 [email protected] fix: text selection two handles directionality. (flutter/flutter#179928) 2026-07-30 [email protected] Add java/gradle check when gradle crashes (flutter/flutter#189997) 2026-07-30 [email protected] [Impeller] Add filterQuality support to ImageFilter.shader (flutter/flutter#188544) 2026-07-30 [email protected] [flutter/flutter] Migrate multiple_windows example to material_ui (flutter/flutter#190313) 2026-07-30 [email protected] Roll Skia from 50cf67910462 to a1109355de78 (6 revisions) (flutter/flutter#190306) 2026-07-30 [email protected] Migrate swift xctests to Swift testing tests (flutter/flutter#187801) 2026-07-30 [email protected] Migrates the platform_channel example to material_ui (flutter/flutter#190253) 2026-07-30 [email protected] iOS: Drop unnecessary @mainactor from VSyncClientTests (flutter/flutter#190285) 2026-07-30 [email protected] Add nullptr check to isOpaque (flutter/flutter#189796) 2026-07-30 [email protected] [Flutter GPU] Bind uniforms and textures by cached reflection index (flutter/flutter#189820) 2026-07-30 [email protected] Migrate layers example to material_ui (flutter/flutter#190237) 2026-07-30 [email protected] Add `ShapeBorder` hit test fast paths (flutter/flutter#187962) 2026-07-30 [email protected] ci: mac-must-wait part duex (flutter/flutter#190295) 2026-07-30 [email protected] Roll Skia from 491301f393d1 to 50cf67910462 (3 revisions) (flutter/flutter#190294) 2026-07-30 [email protected] Roll Packages from dd11626 to 7d17fc8 (1 revision) (flutter/flutter#190293) 2026-07-30 [email protected] Roll Skia from 177de1940bf2 to 491301f393d1 (1 revision) (flutter/flutter#190279) 2026-07-30 [email protected] Roll Skia from b8bda91f02db to 177de1940bf2 (5 revisions) (flutter/flutter#190272) 2026-07-30 [email protected] Roll Fuchsia Linux SDK from J8rVTlBnjnUpuHYk3... to _J8wM3kyQpLN9wvRD... (flutter/flutter#190273) 2026-07-30 [email protected] Roll Skia from 78afc18c9ba0 to b8bda91f02db (3 revisions) (flutter/flutter#190268) 2026-07-30 [email protected] Revert: Split the platform and cpuArch part of TargetPlatform (flutter/flutter#190258) 2026-07-30 [email protected] Return null from RenderProxyBoxMixin.computeDryBaseline when the child has no baseline (flutter/flutter#189723) 2026-07-30 [email protected] Roll Skia from 6fea713d32ec to 78afc18c9ba0 (4 revisions) (flutter/flutter#190257) 2026-07-30 [email protected] Roll pub packages (flutter/flutter#190259) 2026-07-29 [email protected] [cross imports] Check examples cross imports (flutter/flutter#187662) 2026-07-29 [email protected] ci: mac verify binaries still has to wait on linux_host_engine (flutter/flutter#190239) 2026-07-29 [email protected] Adds analytics to macos impeller opt in/out (flutter/flutter#190220) 2026-07-29 [email protected] Roll Skia from 0f35bba4945c to 6fea713d32ec (6 revisions) (flutter/flutter#190230) 2026-07-29 [email protected] iOS: Fix Swift warning in TracingTests (flutter/flutter#190203) 2026-07-29 [email protected] Convert all gradle-wrapper.properties in /dev to use distribution 'bin' (flutter/flutter#190226) 2026-07-29 [email protected] Roll Packages from 3e63635 to dd11626 (11 revisions) (flutter/flutter#190216) ...
3 participants
Continuation of #185712. The reason for doing that is I was getting cryptic dynamic actor isolation crashes when trying to turn on Swift 6 language mode for the test target, and switching to swift testing seems to fix that.
Most changes are trivial / verbatim, but since Swift Testing by default doesn't run tests on the main thread I had to add
@MainActorto a few tests to make sure the test has a runningCFRunLoopPre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.