Wait for a preview to be ready instead of a fixed delay - #175
Open
BarredEwe wants to merge 2 commits into
Open
Conversation
`.snapshot(delay:)` is both flaky and slow: it is either too short for the
preview or multiplied by hundreds of tests on CI. Two opt-in alternatives:
- `.snapshot(waitForIdle: true, timeout: 2)` renders the preview and compares
frames captured at least 50 ms apart, capturing once the same frame comes
back twice in a row.
- `.snapshotWait(until: { ... }, timeout: 2)` waits for a condition the preview
itself knows about.
Both fail with a message naming the preview when the timeout runs out, and
`snapshot_wait_for_idle` / `snapshot_wait_timeout` set the project wide
defaults. `delay` keeps working unchanged.
On macOS the wait runs on the hosted view that is captured afterwards. On
iOS/tvOS the snapshot strategy builds its own copy of the view, so the wait
runs on a probe window and its duration is replayed as the delay.
Closes #28
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94e4b2ab1a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two review findings on the waiting mechanism: - `delay` was documented as the floor of the wait, but the idle check ran before the snapshot strategy applied it. A preview that stays unchanged for the first 100 ms and starts working later was declared idle right away. The wait now spends `delay` first and, on macOS where it runs on the view that is captured, the strategy no longer spends it a second time. - A generated suite only assigned `SnapshotWaitDefaults` when the corresponding key was configured, so a test target with suites from several `.prefire.yml` files inherited whatever the previously run suite had set. Every suite now restores the library defaults before applying its own configuration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #28
.snapshot(delay:)is currently the only tool for a preview that is not ready at once. A fixed delay is both flaky (sometimes too short) and slow — on CI it is multiplied by hundreds of tests.The existing
.snapshot(delay:precision:perceptualPrecision:record:)is untouched —snapshot(waitForIdle:)is a separate overload, so a bare.snapshot()stays unambiguous. The modifiers write to different preference keys and compose:delayremains the floor of the wait.How idle is determined
SnapshotWaiterspins the main run loop with a 50 ms minimum interval between captures and compares frames —layer.render(in:)on iOS/tvOS,cacheDisplay(in:to:)on macOS. Idle means two consecutive matches, i.e. roughly 100 ms without a change. An explicit condition is polled by the same loop on the main thread.On macOS the wait runs on the very
NSViewthat SnapshotTesting later captures, so no extra delay is needed. On iOS/tvOS the strategy re-creates the view fromAnyViewand its state is reset, so the wait runs on a probe window and the measured time is replayed aspreferences.resolvedDelay = max(delay, settleDelay).Timeout
Timing out does not throw. It records
waitFailure, which the template turns intoXCTFail:The condition's call site comes from
#fileID/#line.Tests
make test59/0 andmake test-cli25/0, including eight real-render tests on macOS: idle returns exactly when a timer stops changing the frame, an endlessly animating preview times out, a condition that does and does not resolve in time, global defaults, and the opt-out.The repository's test target is macOS-only, so the iOS path is verified by
make buildand aswiftc -typecheckagainst the iphonesimulator SDK rather than by a runtime test — worth a look during review.