From 21be9e6b6e1892a80082e431dd0e1ab0efcbfe03 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 4 Nov 2026 05:21:38 -0800 Subject: [PATCH] Tests: the opening queue tests wait on the worker's signal instead of the clock The opening tests waited 400 or 500 ms of wall-clock time for files read off the UI thread, and up to 30 s in loops that polled until the queue emptied. Under load a read could take longer, and a_file_that_opens_and_asks_holds_back_the_next_one_ready failed. wait_read now blocks on each pending file's own result channel and keeps the result for poll, and open_ready waits and then polls once. The background save in long_operation_busy_while_a_file_opens_or_a_background_save_runs waits with background::wait_all, and its job waits on the release channel without a timeout. Shipped code is unchanged. --- crates/ui-egui/src/opening/tests.rs | 132 +++++++++------------------- 1 file changed, 40 insertions(+), 92 deletions(-) diff --git a/crates/ui-egui/src/opening/tests.rs b/crates/ui-egui/src/opening/tests.rs index fa90d3cd5..40e2e6009 100644 --- a/crates/ui-egui/src/opening/tests.rs +++ b/crates/ui-egui/src/opening/tests.rs @@ -9,11 +9,7 @@ fn long_operation_busy_while_a_file_opens_or_a_background_save_runs() { assert!(!long_operation_busy(&app)); start(&mut app, "/art/Wait.vectorcraft", vectorcraft_format::save(&vectorcraft_doc::Document::new(10.0, 10.0), false)).unwrap(); assert!(long_operation_busy(&app)); - let t0 = std::time::Instant::now(); - while long_operation_busy(&app) && t0.elapsed().as_secs() < 30 { - poll(&mut app); - std::thread::yield_now(); - } + open_ready(&mut app); assert!(!long_operation_busy(&app)); let (release, held) = std::sync::mpsc::channel::<()>(); let services = Services { write_shared: Some(std::sync::Arc::new(|_: &str, _: &[u8]| Ok(()))), ..Default::default() }; @@ -24,7 +20,7 @@ fn long_operation_busy_while_a_file_opens_or_a_background_save_runs() { "Saving test.vectorcraft".into(), None, move |_| { - let _ = held.recv_timeout(std::time::Duration::from_secs(10)); + let _ = held.recv(); Ok(json!({})) }, |_, r| r, @@ -32,14 +28,28 @@ fn long_operation_busy_while_a_file_opens_or_a_background_save_runs() { .unwrap(); assert!(long_operation_busy(&app)); let _ = release.send(()); - let t0 = std::time::Instant::now(); - while long_operation_busy(&app) && t0.elapsed().as_secs() < 30 { - crate::background::poll(&mut app); - std::thread::yield_now(); - } + crate::background::wait_all(&mut app); assert!(!long_operation_busy(&app)); } +/// Wait until every file being opened has been read, keeping each result for [`poll`]. +fn wait_read(app: &mut VectorcraftApp) { + for p in &mut app.opening.pending { + let (tx, done) = std::sync::mpsc::channel(); + // A reader that stopped without a result leaves the new channel closed too. + if let Ok(r) = p.done.recv() { + let _ = tx.send(r); + } + p.done = done; + } +} + +/// Wait until every file being opened has been read, then open the files that may open now. +fn open_ready(app: &mut VectorcraftApp) { + wait_read(app); + poll(app); +} + /// #1694: a file that finishes opening while a dialog is up waits for it to close, so the /// dialog's document stays the active one. #[test] @@ -49,19 +59,12 @@ fn a_file_ready_while_a_dialog_is_open_waits_for_it_to_close() { let (first, before) = (app.session.doc().unwrap().uid, app.session.documents().len()); app.ui.dialog = Some(crate::state::Dialog::new(crate::dialogs::save_options::KIND, json!({"path": "/art/First.vectorcraft"}))); start(&mut app, "/art/Second.vectorcraft", vectorcraft_format::save(&vectorcraft_doc::Document::new(120.0, 80.0), false)).unwrap(); - let t0 = std::time::Instant::now(); - while t0.elapsed().as_millis() < 400 { - poll(&mut app); - std::thread::yield_now(); - } + open_ready(&mut app); assert_eq!((app.session.documents().len(), app.session.doc().unwrap().uid), (before, first), "still the dialog's document"); assert_eq!(app.opening.pending.len(), 1); app.ui.dialog = None; - let t0 = std::time::Instant::now(); - while !app.opening.pending.is_empty() && t0.elapsed().as_secs() < 30 { - poll(&mut app); - std::thread::yield_now(); - } + poll(&mut app); + assert!(app.opening.pending.is_empty()); assert_eq!(app.session.documents().len(), before + 1); assert_eq!(app.session.doc().unwrap().path.as_deref(), Some("/art/Second.vectorcraft")); } @@ -80,18 +83,10 @@ fn a_file_ready_during_a_drag_waits_for_it_to_end() { app.session.pointer(&PointerEvent::new(kind, x, 20.0), view).unwrap(); } start(&mut app, "/art/Second.vectorcraft", vectorcraft_format::save(&vectorcraft_doc::Document::new(120.0, 80.0), false)).unwrap(); - let t0 = std::time::Instant::now(); - while t0.elapsed().as_millis() < 400 { - poll(&mut app); - std::thread::yield_now(); - } + open_ready(&mut app); assert!(app.session.documents().len() == 1 && app.session.in_interaction(), "the file waits for the drag"); app.session.pointer(&PointerEvent::new(PointerKind::Up, 60.0, 20.0), view).unwrap(); - let t0 = std::time::Instant::now(); - while !app.opening.pending.is_empty() && t0.elapsed().as_secs() < 30 { - poll(&mut app); - std::thread::yield_now(); - } + poll(&mut app); assert_eq!(app.session.documents().len(), 2); let moved = app.session.documents()[0].doc.node(vectorcraft_doc::NodeId(square)).and_then(|n| n.geometric_bounds()); assert_eq!(moved.map(|b| (b.x0, b.y0)), Some((50.0, 10.0)), "the drag was not canceled"); @@ -112,18 +107,10 @@ fn a_file_ready_during_a_place_cursor_drag_waits_for_it_to_end() { crate::canvas::dispatch(&mut app, &PointerEvent::new(kind, x, 20.0), view); } start(&mut app, "/art/Second.vectorcraft", vectorcraft_format::save(&vectorcraft_doc::Document::new(120.0, 80.0), false)).unwrap(); - let t0 = std::time::Instant::now(); - while t0.elapsed().as_millis() < 400 { - poll(&mut app); - std::thread::yield_now(); - } + open_ready(&mut app); assert!(app.session.documents().len() == 1 && app.session.tool_busy(), "the file waits for the drag"); crate::canvas::dispatch(&mut app, &PointerEvent::new(PointerKind::Up, 60.0, 20.0), view); - let t0 = std::time::Instant::now(); - while !app.opening.pending.is_empty() && t0.elapsed().as_secs() < 30 { - poll(&mut app); - std::thread::yield_now(); - } + poll(&mut app); assert_eq!(app.session.documents().len(), 2); let first = &app.session.documents()[0].doc; let names: Vec<_> = first.layers.iter().flat_map(|l| l.children().into_iter().flatten()).filter_map(|n| n.name.clone()).collect(); @@ -158,22 +145,14 @@ fn a_file_ready_during_a_preview_waits_for_it_to_end() { } assert_eq!(app.session.tool_busy(), case != "slider", "{case}"); start(&mut app, "/art/Second.vectorcraft", vectorcraft_format::save(&vectorcraft_doc::Document::new(120.0, 80.0), false)).unwrap(); - let t0 = std::time::Instant::now(); - while t0.elapsed().as_millis() < 400 { - poll(&mut app); - std::thread::yield_now(); - } + open_ready(&mut app); assert!(app.session.documents().len() == 1 && app.session.in_interaction(), "the file waits for the preview: {case}"); if case == "line" { click(&mut app, 50.0); } else { app.session.cancel_interaction().unwrap(); } - let t0 = std::time::Instant::now(); - while !app.opening.pending.is_empty() && t0.elapsed().as_secs() < 30 { - poll(&mut app); - std::thread::yield_now(); - } + poll(&mut app); assert_eq!(app.session.documents().len(), 2, "{case}"); } } @@ -194,24 +173,17 @@ fn a_file_ready_between_a_tools_clicks_opens() { crate::canvas::dispatch(app, &PointerEvent::new(kind, x, 20.0), view); } }; - let opened = |app: &mut VectorcraftApp| { - let t0 = std::time::Instant::now(); - while !app.opening.pending.is_empty() && t0.elapsed().as_secs() < 30 { - poll(app); - std::thread::yield_now(); - } - }; click(&mut app, 20.0); assert!(app.session.tool_busy() && !app.session.in_interaction(), "the first point waits for the second"); start(&mut app, "/art/Second.vectorcraft", vectorcraft_format::save(&vectorcraft_doc::Document::new(400.0, 300.0), false)).unwrap(); - opened(&mut app); + open_ready(&mut app); assert_eq!(app.session.documents().len(), 2, "the file opens after one click"); assert!(!app.session.tool_busy(), "the tool starts afresh"); click(&mut app, 20.0); click(&mut app, 80.0); assert!(app.session.tool_busy() && !app.session.in_interaction(), "the second point waits for the line's place"); start(&mut app, "/art/Third.vectorcraft", vectorcraft_format::save(&vectorcraft_doc::Document::new(120.0, 80.0), false)).unwrap(); - opened(&mut app); + open_ready(&mut app); assert_eq!(app.session.documents().len(), 3, "the file opens after two clicks"); assert!(!app.session.tool_busy() && !app.session.in_interaction(), "the tool starts afresh"); let second = &app.session.documents()[1].doc; @@ -228,11 +200,7 @@ fn a_file_read_off_the_ui_thread_opens_as_one_read_at_once() { let r = start(&mut app, "/art/Poster.vectorcraft", bytes).unwrap(); assert_eq!(r["background"], true); assert_eq!(app.ui.status, "Opening Poster.vectorcraft…"); - let t0 = std::time::Instant::now(); - while !app.opening.pending.is_empty() && t0.elapsed().as_secs() < 30 { - poll(&mut app); - std::thread::yield_now(); - } + open_ready(&mut app); assert_eq!(app.session.documents().len(), before + 1); assert!(!app.ui.status.starts_with("Opening"), "{}", app.ui.status); let st = app.session.doc().unwrap(); @@ -240,11 +208,7 @@ fn a_file_read_off_the_ui_thread_opens_as_one_read_at_once() { assert_eq!(app.ui.recent_files.first().map(String::as_str), Some("/art/Poster.vectorcraft")); // A file that can't be read: reported, nothing opened, not a recent file. start(&mut app, "/art/Broken.vectorcraft", b"{\"format\":\"vectorcraft\"".to_vec()).unwrap(); - let t0 = std::time::Instant::now(); - while !app.opening.pending.is_empty() && t0.elapsed().as_secs() < 30 { - poll(&mut app); - std::thread::yield_now(); - } + open_ready(&mut app); assert_eq!(app.session.documents().len(), before + 1); assert!(app.ui.status.starts_with("Couldn't open Broken.vectorcraft"), "{}", app.ui.status); assert_ne!(app.ui.recent_files.first().map(String::as_str), Some("/art/Broken.vectorcraft")); @@ -298,11 +262,7 @@ fn a_file_read_off_the_ui_thread_has_its_links_and_type_as_one_read_at_once() { assert!(question.as_ref().is_some_and(|(_, f)| f["missing"][0]["name"] == "gone.png"), "{update}: {question:?}"); app.ui.dialog = None; start(&mut app, &path, bytes.clone()).unwrap(); - let t0 = std::time::Instant::now(); - while !app.opening.pending.is_empty() && t0.elapsed().as_secs() < 30 { - poll(&mut app); - std::thread::yield_now(); - } + open_ready(&mut app); let st = app.session.doc().unwrap(); assert!(*st.doc == *at_once, "{update}: the same document"); assert!(st.history.undo.is_empty() && !st.is_dirty(), "{update}"); @@ -341,11 +301,7 @@ fn a_file_asked_for_again_while_it_is_read_opens_once() { start(&mut app, "/art/Poster.vectorcraft", bytes).unwrap(); assert_eq!(crate::io::open_file(&mut app, "/art/Poster.vectorcraft", true).unwrap(), Value::Null); assert_eq!(app.opening.pending.len(), 1); - let t0 = std::time::Instant::now(); - while !app.opening.pending.is_empty() && t0.elapsed().as_secs() < 30 { - poll(&mut app); - std::thread::yield_now(); - } + open_ready(&mut app); assert_eq!(app.session.documents().len(), 1); } @@ -374,12 +330,8 @@ fn a_queued_reopen_that_asks_holds_back_files_ready_to_open() { app.session.execute("file.close", &json!({"index": 1})).unwrap(); app.session.execute("file.close", &json!({"index": 0})).unwrap(); start(&mut app, "/art/Other.vectorcraft", vectorcraft_format::save(&vectorcraft_doc::Document::new(120.0, 80.0), false)).unwrap(); - // Time for the other file to be read; it waits for the dialog meanwhile. - let t0 = std::time::Instant::now(); - while t0.elapsed().as_millis() < 500 { - poll(&mut app); - std::thread::yield_now(); - } + // The other file is read; it waits for the dialog meanwhile. + open_ready(&mut app); assert_eq!(app.opening.pending.len(), 1); app.ui.dialog = None; poll(&mut app); @@ -402,12 +354,8 @@ fn a_file_that_opens_and_asks_holds_back_the_next_one_ready() { app.ui.dialog = Some(crate::state::Dialog::new(crate::dialogs::save_options::KIND, json!({}))); start(&mut app, "/art/Asks.vectorcraft", asks).unwrap(); start(&mut app, "/art/Plain.vectorcraft", vectorcraft_format::save(&vectorcraft_doc::Document::new(120.0, 80.0), false)).unwrap(); - // Time for both files to be read; they wait for the dialog meanwhile. - let t0 = std::time::Instant::now(); - while t0.elapsed().as_millis() < 500 { - poll(&mut app); - std::thread::yield_now(); - } + // Both files are read; they wait for the dialog meanwhile. + open_ready(&mut app); assert_eq!(app.opening.pending.len(), 2); app.ui.dialog = None; poll(&mut app);