[CI] dart_async flake fix - #3
Closed
DenisovAV wants to merge 3 commits into
Closed
Conversation
The `dart_async` timing tests asserted a narrow wall-clock window around each async delay (e.g. `sleep(200ms)` required `> 200 && < 300`). The upper bound measures host/CI scheduling speed rather than binding correctness, so under load it fails on clean runs — the flakiness tracked in Uniffi-Dart#139 (and previously Uniffi-Dart#105, Uniffi-Dart#123). The `sleep` test is the one that trips most often. Root cause (per Uniffi-Dart#139, investigated rather than assumed): - The binding is correct and fast. Locally the whole `dart_async` suite runs in ~9s with every test passing, including `sleep`. - On a loaded CI runner the same suite can take ~200s of wall-clock; the async operations still complete correctly, but the elapsed-time upper bounds are exceeded. So the failure is test design (narrow wall-clock assertions), not an async runtime or generated-binding bug. Fix: for the delay-based tests, keep the lower bound (which proves the async plumbing actually suspended for the expected time) and drop the upper bound. This matches uniffi-rs's own futures fixture (`test_futures.py`), which asserts only `assertGreater(elapsed, expected)` with no upper bound. Scope: this touches only the delay tests that have a lower bound. The immediate- operation checks (`always_ready < 200`, `void <= 10`, sync/constructor `< N`) share the same latent wall-clock fragility but have no lower bound to fall back on; leaving those for a separate decision. Refs Uniffi-Dart#139.
`clippy::useless_borrows_in_formatting` (denied via `-D warnings`) fails the stable Lints job on `main`: `format!`/`println!` arguments that are already `Display`/`Debug` don't need a leading `&`. Removes the redundant `&` at the five sites clippy 1.97 flags (callback_interface, enums, render, stream). Unblocks CI for this branch; unrelated to the timing-assertion change but the two share the stable Lints job.
…pe comment
Review follow-up. Two fixes to the timing-assertion change:
- `concurrent_future` previously relied on its upper bound (`<= 300`) to prove
the two `Future.wait` calls overlapped — dropping it to a lower bound only
would let a regression that serializes the futures (~300ms) pass. Restore the
concurrency check as a *relative* comparison: measure the same two delays
concurrently and sequentially, assert concurrent < sequential. This survives
CI load (both sides dilate) where the old absolute ceiling did not.
- Scope the header comment: only the delay-based assertions became lower-bound-
only; the immediate-operation checks still assert an upper bound only and are
a known, deliberately-deferred fragility. The old wording ("assertions below
check a LOWER bound only") over-generalized. Also mark the ~seconds figures as
illustrative rather than enforced.
Refs Uniffi-Dart#139.
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.
Fork CI run for the flake fix (upstream Uniffi-Dart#157 is action_required-gated). Not for merge.