[shared_preferences_foundation] Migrate XCTest to Swift Testing #10766

jmagman · 2026-01-10T05:57:10Z

Use parameterized tests instead of testing in a for loop. I had to make the tests serializable because this shared preferences API is very un-threadsafe, and running them in parallel has many test failures where values are stomping on each other.

Adding CHANGELOG override per #10761 (comment)

macOS tests

``` ◇ Test run started. ↳ Testing Library Version: 102 (arm64e-apple-macos13.0) ◇ Suite RunnerTests started. ◇ Test setAndGet(prefix:) started. ◇ Passing 1 argument prefix → "aPrefix" to setAndGet(prefix:) ◇ Passing 1 argument prefix → "" to setAndGet(prefix:) ✔ Test setAndGet(prefix:) passed after 0.001 seconds. ◇ Test getWithAllowList(prefix:) started. ◇ Passing 1 argument prefix → "aPrefix" to getWithAllowList(prefix:) ◇ Passing 1 argument prefix → "" to getWithAllowList(prefix:) ✔ Test getWithAllowList(prefix:) passed after 0.001 seconds. ◇ Test remove(prefix:) started. ◇ Passing 1 argument prefix → "aPrefix" to remove(prefix:) ◇ Passing 1 argument prefix → "" to remove(prefix:) ✔ Test remove(prefix:) passed after 0.030 seconds. ◇ Test clearWithNoAllowlist(prefix:) started. ◇ Passing 1 argument prefix → "aPrefix" to clearWithNoAllowlist(prefix:) ◇ Passing 1 argument prefix → "" to clearWithNoAllowlist(prefix:) ✔ Test clearWithNoAllowlist(prefix:) passed after 0.002 seconds. ◇ Test clearWithAllowlist(prefix:) started. ◇ Passing 1 argument prefix → "aPrefix" to clearWithAllowlist(prefix:) ◇ Passing 1 argument prefix → "" to clearWithAllowlist(prefix:) ✔ Test clearWithAllowlist(prefix:) passed after 0.001 seconds. ◇ Test asyncSetAndGet() started. ✔ Test asyncSetAndGet() passed after 0.001 seconds. ◇ Test asyncGetAll() started. ✔ Test asyncGetAll() passed after 0.001 seconds. ◇ Test asyncGetAllWithAndWithoutSuiteName() started. ✔ Test asyncGetAllWithAndWithoutSuiteName() passed after 0.001 seconds. ◇ Test asyncGetAllWithAllowList() started. ✔ Test asyncGetAllWithAllowList() passed after 0.001 seconds. ◇ Test asyncRemove() started. ✔ Test asyncRemove() passed after 0.001 seconds. ◇ Test asyncClearWithNoAllowlist() started. ✔ Test asyncClearWithNoAllowlist() passed after 0.001 seconds. ◇ Test asyncClearWithAllowlist() started. ✔ Test asyncClearWithAllowlist() passed after 0.001 seconds. ✔ Suite RunnerTests passed after 0.044 seconds. ✔ Test run with 12 tests passed after 0.044 seconds. ``` https://ci.chromium.org/ui/p/flutter/builders/try/Mac_arm64%20macos_platform_tests%20master%20-%20packages/26300/overview

iOS tests

``` Testing started Test suite 'RunnerTests' started on 'Clone 1 of Flutter-iPhone - Runner (85304)' Test case 'RunnerTests/setAndGet(prefix:)' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.003 seconds) Test case 'RunnerTests/setAndGet(prefix:)' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.002 seconds) Test case 'RunnerTests/getWithAllowList(prefix:)' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.000 seconds) Test case 'RunnerTests/getWithAllowList(prefix:)' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.000 seconds) Test case 'RunnerTests/remove(prefix:)' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.001 seconds) Test case 'RunnerTests/remove(prefix:)' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.001 seconds) Test case 'RunnerTests/clearWithNoAllowlist(prefix:)' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.003 seconds) Test case 'RunnerTests/clearWithNoAllowlist(prefix:)' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.013 seconds) Test case 'RunnerTests/clearWithAllowlist(prefix:)' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.000 seconds) Test case 'RunnerTests/clearWithAllowlist(prefix:)' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.000 seconds) Test case 'RunnerTests/asyncSetAndGet()' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.001 seconds) Test case 'RunnerTests/asyncGetAll()' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.000 seconds) Test case 'RunnerTests/asyncGetAllWithAndWithoutSuiteName()' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.002 seconds) Test case 'RunnerTests/asyncGetAllWithAllowList()' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.019 seconds) Test case 'RunnerTests/asyncRemove()' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.001 seconds) Test case 'RunnerTests/asyncClearWithNoAllowlist()' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.005 seconds) Test case 'RunnerTests/asyncClearWithAllowlist()' passed on 'Clone 1 of Flutter-iPhone - Runner (85304)' (0.002 seconds) Successfully ran iOS xctest for packages/shared_preferences/shared_preferences_foundation/example ``` https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8693085104608886001/+/u/Run_package_tests/native_test/stdout

## Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot 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.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩² ↩³

gemini-code-assist

Code Review

This pull request successfully migrates the tests for shared_preferences_foundation from XCTest to the modern Swift Testing framework. The changes are well-executed, utilizing parameterized tests to reduce boilerplate and improving test robustness by avoiding force unwraps. The decision to serialize tests is a good catch for the non-thread-safe API. I have one minor suggestion to improve consistency.

packages/shared_preferences/shared_preferences_foundation/darwin/Tests/RunnerTests.swift

jmagman · 2026-01-10T06:15:12Z

packages/shared_preferences/shared_preferences_foundation/example/ios/RunnerTests/.gitkeep

Xcode is expecting this directory:

macOS gets away with it because there's a (probably unnecessary) Info.plist in the equivalent directory:
https://github.com/flutter/packages/tree/fb3e9081ca58b1cb3e54b8bd471c57116a72585e/packages/shared_preferences/shared_preferences_foundation/example/macos/RunnerTests

jmagman · 2026-01-10T06:20:53Z

packages/shared_preferences/shared_preferences_foundation/darwin/Tests/RunnerTests.swift

 #endif

-class RunnerTests: XCTestCase {
+@Suite(.serialized)


Lots of test failures when these tests run in parallel. For example, there's no locking in the update methods like:

packages/packages/shared_preferences/shared_preferences_foundation/darwin/shared_preferences_foundation/Sources/shared_preferences_foundation/SharedPreferencesPlugin.swift

Lines 133 to 144 in fb3e908

func clear(allowList: [String]?, options: SharedPreferencesPigeonOptions) throws {

let defaults = try SharedPreferencesPlugin.getUserDefaults(options: options)

if let allowList = allowList {

for (key) in allowList {

defaults.removeObject(forKey: key)

}

} else {

for key in defaults.dictionaryRepresentation().keys {

defaults.removeObject(forKey: key)

}

}

}

Should probably be migrated to actor?

Surprisingly I don't see any open issues that look related.

Should probably be migrated to actor?

We now have toooo many options!

Actor

GCD

the new Synchronization framework introduced last year

Locks (os_unfair_lock is likely the most performant)

Worth adding a TODO to remove .serialized?

Come to think of it, this isn't actually a bug. The defaults themselves are a threadsafe API, so if calling code, like these tests, are relying on another thread not mutating the defaults, then it would up to that calling code to handle it. I actually now think @Suite(.serialized) is correct. I'll add a comment.

I was going to say something like this. My understanding when I wrote this code is that behavior is expected. I can see changing it though, if we wanted to make the plugin work that way instead.

There's still a few pieces here that look a little thread unsafe, for example:

packages/packages/shared_preferences/shared_preferences_foundation/darwin/shared_preferences_foundation/Sources/shared_preferences_foundation/SharedPreferencesPlugin.swift

Lines 140 to 142 in fb3e908

for key in defaults.dictionaryRepresentation().keys {

defaults.removeObject(forKey: key)

}

~~That's looping over a collection while mutating it, which is generally not a good pattern. There's no guarantee the defaults wouldn't be added by another thread while it's being cleared.~~ correction: no it's not, it's looping over the dictionary representation.
But it's way less dire than I made it out to be in the initial description. Sorry.

Had me worried there for a second :)

If there are others, I'd be happy to hear about them though. I'm always happy to learn from my mistakes.

Sorry for the noise and panic, @tarrinneal 🙂

hellohuanlin · 2026-01-12T19:31:05Z

packages/shared_preferences/shared_preferences_foundation/darwin/Tests/RunnerTests.swift

 #endif

-class RunnerTests: XCTestCase {
+@Suite(.serialized)


Should probably be migrated to actor?

We now have toooo many options!

Actor

GCD

the new Synchronization framework introduced last year

Locks (os_unfair_lock is likely the most performant)

hellohuanlin · 2026-01-12T19:35:51Z

packages/shared_preferences/shared_preferences_foundation/darwin/Tests/RunnerTests.swift

 #endif

-class RunnerTests: XCTestCase {
+@Suite(.serialized)


Worth adding a TODO to remove .serialized?

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

…ting (flutter/packages#10766)

jmagman self-assigned this Jan 10, 2026

github-actions bot added platform-ios platform-macos p: shared_preferences labels Jan 10, 2026

gemini-code-assist bot reviewed Jan 10, 2026

View reviewed changes

packages/shared_preferences/shared_preferences_foundation/darwin/Tests/RunnerTests.swift Outdated Show resolved Hide resolved

jmagman mentioned this pull request Jan 10, 2026

Migrate iOS plugin tests from XCTest to Swift Testing flutter/flutter#180787

Open

19 tasks

jmagman commented Jan 10, 2026

View reviewed changes

jmagman marked this pull request as ready for review January 10, 2026 06:56

jmagman requested a review from tarrinneal as a code owner January 10, 2026 06:56

jmagman requested a review from hellohuanlin January 10, 2026 06:56

hellohuanlin approved these changes Jan 12, 2026

View reviewed changes

tarrinneal approved these changes Jan 12, 2026

View reviewed changes

jmagman and others added 4 commits January 26, 2026 16:36

[shared_preferences_foundation] Migrate XCTest to Swift Testing

86a73fe

#require

ed97579

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

format

3312f2a

Revert CHANGELOG

ce0147f

jmagman force-pushed the shared-preferences-st branch from c570175 to ce0147f Compare January 27, 2026 00:37

jmagman added the override: no changelog needed Override the check requiring CHANGELOG updates for most changes label Jan 27, 2026

Add serialized comment

6d1814d

jmagman added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 27, 2026

auto-submit bot merged commit 7253b0f into flutter:main Jan 27, 2026
81 checks passed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 28, 2026

7253b0f1e [shared_preferences_foundation] Migrate XCTest to Swift Tes…

e69521d

…ting (flutter/packages#10766)

engine-flutter-autoroll mentioned this pull request Jan 28, 2026

Roll Packages from e37af1148f45 to 1cb2148ade16 (5 revisions) flutter/flutter#181598

Open

	func clear(allowList: [String]?, options: SharedPreferencesPigeonOptions) throws {
	let defaults = try SharedPreferencesPlugin.getUserDefaults(options: options)
	if let allowList = allowList {
	for (key) in allowList {
	defaults.removeObject(forKey: key)
	}
	} else {
	for key in defaults.dictionaryRepresentation().keys {
	defaults.removeObject(forKey: key)
	}
	}
	}

[shared_preferences_foundation] Migrate XCTest to Swift Testing #10766

[shared_preferences_foundation] Migrate XCTest to Swift Testing #10766

Conversation

jmagman commented Jan 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmagman Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jmagman commented Jan 10, 2026 •

edited

Loading

jmagman Jan 27, 2026 •

edited

Loading