diff --git a/fixtures/dart_async/test/futures_test.dart b/fixtures/dart_async/test/futures_test.dart index 3af14e06..7921e9d8 100644 --- a/fixtures/dart_async/test/futures_test.dart +++ b/fixtures/dart_async/test/futures_test.dart @@ -8,6 +8,26 @@ Future measureTime(Future Function() action) async { return end.difference(start); } +// The DELAY-based timing assertions below check a LOWER bound only: that an +// async operation waited at least its expected delay (proving the async +// plumbing actually suspends). They deliberately do not assert an upper bound — +// wall-clock upper bounds measure host/CI scheduling speed, not binding +// correctness, and are the source of the flaky failures tracked in #139. This +// mirrors uniffi-rs's own futures fixture (`test_futures.py`), which uses +// `assertGreater` with no upper bound. (Illustrative, not enforced: locally the +// whole suite runs in a few seconds with correct results; on a heavily loaded +// CI runner it can take vastly longer, which is what tripped the old two-sided +// per-operation bounds like `< 300ms`.) +// +// Two groups are intentionally different: +// - `concurrent_future` verifies concurrency structurally (completion order), +// not by comparing durations — see the note in that test for why a timing +// comparison can't reliably catch a serialization regression. +// - The immediate-operation checks (`always_ready`, `void`, sync methods, +// constructors) still assert an upper bound only. They have no lower bound +// to fall back on and share the same latent wall-clock fragility; tightening +// those is deliberately left for a separate change. + class ErroringAsyncParser extends AsyncParser { @override Future asString(int delayMs, int value) async => value.toString(); @@ -73,7 +93,7 @@ void main() { await sleep(ms: 200); }); - expect(time.inMilliseconds > 200 && time.inMilliseconds < 300, true); + expect(time.inMilliseconds > 200, true); }); test('sequential_future', () async { @@ -83,11 +103,22 @@ void main() { expect(resultAlice, 'Hello, Alice!'); expect(resultBob, 'Hello, Bob!'); }); - expect(time.inMilliseconds > 300 && time.inMilliseconds < 400, true); + expect(time.inMilliseconds > 300, true); }); test('concurrent_future', () async { - final time = await measureTime(() async { + // Comparing concurrent vs. sequential wall-clock time cannot reliably detect + // a serialization regression: a serialized `Future.wait` is structurally the + // same work as running the calls sequentially (100+200 either way), so the + // comparison becomes a coin flip, while any fixed margin or ratio re-adds the + // load-sensitive flakiness #139 is about (the Rust-side `thread::sleep` + // delays don't dilate under load, but scheduling jitter adds unbounded time). + // Instead assert two load-robust properties. + + // (1) Correct positional results + a lower bound. The lower bound is + // jitter-immune — sleeps only ever lengthen a run — and catches a future + // resolving before its delay elapsed. + final concurrentTime = await measureTime(() async { final results = await Future.wait([ sayAfter(ms: 100, who: 'Alice'), sayAfter(ms: 200, who: 'Bob'), @@ -96,8 +127,25 @@ void main() { expect(results[0], 'Hello, Alice!'); expect(results[1], 'Hello, Bob!'); }); - - expect(time.inMilliseconds >= 200 && time.inMilliseconds <= 300, true); + expect(concurrentTime.inMilliseconds, greaterThanOrEqualTo(200)); + + // (2) Completion ORDER, not duration: start a slow future, then a fast one, + // and require the fast one to finish while the slow one is still pending. If + // the bindings serialized FFI futures, the slow call would have to complete + // before the fast one could even start, failing this deterministically — + // with ~2000ms of structural headroom (not a tuned threshold), so it can't + // reflake on a loaded runner. + var slowDone = false; + final slow = + sayAfter(ms: 2000, who: 'Slow').whenComplete(() => slowDone = true); + await sayAfter(ms: 1, who: 'Fast'); + expect( + slowDone, + isFalse, + reason: 'a 1ms future completed only after a 2000ms future that started ' + 'earlier — FFI futures appear to be serialized', + ); + await slow; // let the fixture call finish before the test ends }); test('with_tokio_runtime', () async { @@ -105,7 +153,7 @@ void main() { final resultAlice = await sayAfterWithTokio(ms: 200, who: 'Alice'); expect(resultAlice, 'Hello, Alice (with Tokio)!'); }); - expect(time.inMilliseconds > 200 && time.inMilliseconds < 300, true); + expect(time.inMilliseconds > 200, true); }); test('fallible_function_and_method', () async { @@ -155,7 +203,7 @@ void main() { ); // calls the waker a second time after 1s await sleep(ms: 200); // wait for possible failure }); - expect(time.inMilliseconds >= 400 && time.inMilliseconds <= 600, true); + expect(time.inMilliseconds >= 400, true); }); test('udl_async_function', () async { @@ -190,7 +238,7 @@ void main() { final result = await megaphone.sayAfter(ms: 100, who: 'Alice'); expect(result, 'HELLO, ALICE!'); }); - expect(time.inMilliseconds >= 100 && time.inMilliseconds < 200, true); + expect(time.inMilliseconds >= 100, true); // Test async silence method final silenceTime = await measureTime(() async { @@ -218,7 +266,7 @@ void main() { final result = await megaphone.sayAfterWithTokio(ms: 100, who: 'Charlie'); expect(result, 'HELLO, CHARLIE (WITH TOKIO)!'); }); - expect(time.inMilliseconds >= 100 && time.inMilliseconds < 200, true); + expect(time.inMilliseconds >= 100, true); }); test('proc_macro_megaphone_fallible_method', () async { @@ -260,7 +308,7 @@ void main() { final result = await udlMegaphone.sayAfter(ms: 100, who: 'Dave'); expect(result, 'HELLO, DAVE (FROM UDL MEGAPHONE)!'); }); - expect(time.inMilliseconds >= 100 && time.inMilliseconds < 200, true); + expect(time.inMilliseconds >= 100, true); }); test('async_object_creation_functions', () async { @@ -291,7 +339,7 @@ void main() { ); expect(result, 'HELLO, EVE!'); }); - expect(time.inMilliseconds >= 100 && time.inMilliseconds < 200, true); + expect(time.inMilliseconds >= 100, true); }); test('fallible_struct_creation', () async { diff --git a/src/gen/callback_interface.rs b/src/gen/callback_interface.rs index b65b95be..357fd803 100644 --- a/src/gen/callback_interface.rs +++ b/src/gen/callback_interface.rs @@ -359,7 +359,7 @@ pub fn generate_callback_functions( // Generate the function body let callback_method_name = - &format!("{}{}", &DartCodeOracle::fn_name(callback_name), &DartCodeOracle::class_name(m.name())); + &format!("{}{}", DartCodeOracle::fn_name(callback_name), DartCodeOracle::class_name(m.name())); if m.is_async() { let completion_base = foreign_future_completion_name(m); diff --git a/src/gen/enums.rs b/src/gen/enums.rs index c181d4ce..3ca0c575 100644 --- a/src/gen/enums.rs +++ b/src/gen/enums.rs @@ -36,7 +36,7 @@ impl CodeType for EnumCodeType { } fn ffi_converter_name(&self) -> String { - format!("FfiConverter{}", &DartCodeOracle::class_name(&self.id)) + format!("FfiConverter{}", DartCodeOracle::class_name(&self.id)) } } diff --git a/src/gen/render/mod.rs b/src/gen/render/mod.rs index 93bf68c8..1ee2d60b 100644 --- a/src/gen/render/mod.rs +++ b/src/gen/render/mod.rs @@ -57,7 +57,7 @@ pub trait Renderable { }; if !type_helper.include_once_check(&ty.as_codetype().canonical_name(), ty) { - println!("{} Added", &ty.as_codetype().canonical_name()); + println!("{} Added", ty.as_codetype().canonical_name()); } type_name diff --git a/src/gen/stream/mod.rs b/src/gen/stream/mod.rs index d5d2b7ee..b2a41d9b 100644 --- a/src/gen/stream/mod.rs +++ b/src/gen/stream/mod.rs @@ -8,7 +8,7 @@ pub fn generate_stream(obj: &Object, _type_helper: &dyn TypeHelperRenderer) -> d let obj_name = obj.name(); let fn_name = DartCodeOracle::fn_name(&obj_name.replace("StreamExt", "")); let obj_var_name = &DartCodeOracle::var_name(&fn_name); - let create_obj_fn_name = format!("createStream{}", &obj_name.replace("StreamExt", "")); + let create_obj_fn_name = format!("createStream{}", obj_name.replace("StreamExt", "")); quote! { $fn_name() async* {