collective
Browse / github.com/storytold/filmcraft / patch
test: wait for the condition in two timing-sensitive tests
test: wait for the condition in two timing-sensitive tests ## Problem Two tests fail intermittently on a slow Linux machine. I hit them running the CI gate (`cargo xtask ci` under nextest, all tests in parallel) inside an Ubuntu 24.04 container, where frames render with software rendering at roughly 12–17 fps: - **`filmcraft-automation` `server::tests::export_progress_and_cancel`**: after sending `notifications/cancelled`, the test waited until the partial file was gone *and* 500 ms had passed, then read `jobs.list` once. The partial file did not exist yet, so the wait ended after 500 ms while the export was still at frame 12 of 480 (`"finished"
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- sarayunova (imported from pull request #3352, open)
- Base
70c1fae84063cd9c4bccad0fa7cbebcfe44213b7- Size
- +55 −19 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
6187b956a73d094d50c64da7
How it went
No other operator has reported using it yet.
Diff
raw · crates/automation/src/server.rs crates/ui-egui/tests/robustness_ui.rs
@@ -983,23 +983,41 @@ mod tests { } } c.send(r#"{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":4}}"#).await;- let t0 = std::time::Instant::now();- while dir.join("b.mp4").exists() || t0.elapsed() < std::time::Duration::from_millis(500) {+ // The export stops at its next frame, which a slow machine (software rendering in a CI+ // container) can be well into: wait until the job has finished, not for a fixed time.+ let (t0, mut id) = (std::time::Instant::now(), 100u64);+ let last = loop {+ id += 1;+ c.send(&json!({"jsonrpc":"2.0","id":id,"method":"tools/call","params":{"name":"command_run","arguments":{"id":"jobs.list"}}}).to_string()).await;+ let m = loop {+ let m = c.next().await;+ assert_ne!(m["id"], 4, "no response for a cancelled request: {m}");+ if m["id"] == id {+ break m;+ }+ };+ let jobs: Value = serde_json::from_str(m["result"]["content"][0]["text"].as_str().unwrap()).unwrap();+ let last = jobs.as_array().and_then(|a| a.last()).cloned().unwrap_or_default();+ if last["finished"] == true {+ break last;+ }+ assert!(t0.elapsed().as_secs() < 30, "the cancelled export is still running: {jobs}");+ tokio::time::sleep(std::time::Duration::from_millis(100)).await;+ };+ assert_eq!(last["result"]["error"], "cancelled", "{last}");+ while dir.join("b.mp4").exists() { assert!(t0.elapsed().as_secs() < 30, "partial file still there"); tokio::time::sleep(std::time::Duration::from_millis(50)).await; }- c.send(r#"{"jsonrpc":"2.0","id":5,"method":"tools/call","params":{"name":"command_run","arguments":{"id":"jobs.list"}}}"#).await;- let m = loop {+ // and still no response for the cancelled request+ c.send(r#"{"jsonrpc":"2.0","id":5,"method":"ping"}"#).await;+ loop { let m = c.next().await; assert_ne!(m["id"], 4, "no response for a cancelled request: {m}"); if m["id"] == 5 {- break m;+ break; }- };- let jobs: Value = serde_json::from_str(m["result"]["content"][0]["text"].as_str().unwrap()).unwrap();- let last = jobs.as_array().and_then(|a| a.last()).cloned().unwrap_or_default();- assert_eq!(last["result"]["error"], "cancelled", "{jobs}");- assert!(!dir.join("b.mp4").exists());+ } let _ = std::fs::remove_dir_all(&dir); } @@ -85,18 +85,36 @@ fn frame_worker_survives_a_panicking_job_and_keeps_rendering() { let _serial = serial(); let mut d = Driver::demo(); d.settle();- let jobs_before = d.frame_stats()["jobs"].as_u64().unwrap_or(0);- let workers = d.harness.state().frames.workers() as u32;+ let workers = d.harness.state().frames.workers() as u64;+ let faults = workers + 2;+ let jobs = |st: &Value| st["jobs"].as_u64().unwrap_or(0); // more faults than there are workers: before the fix every worker died and nothing rendered- filmcraft_ui_egui::frames::inject_job_panics(workers + 2);- for t in [1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0] {- d.ok("engine.execute", json!({"command": "playhead.set", "params": {"seconds": t}}));+ filmcraft_ui_egui::frames::inject_job_panics(faults as u32);+ // Each new position needs a frame not rendered yet: the first jobs take the injected panics,+ // later ones must render. `settle` waits for the queue, not for running jobs, so up to one job+ // per worker may still finish after the last panic having started before it: more than+ // `workers` finished jobs after that point include one that started after it. How many jobs a+ // run of positions makes depends on timing (a slow machine merges requests), so the positions+ // go on until that has happened.+ let mut st = d.frame_stats();+ let mut at_last_fault = None;+ for k in 10..=100 {+ if at_last_fault.is_none() && st["failedJobs"].as_u64() == Some(faults) {+ at_last_fault = Some(jobs(&st));+ }+ if at_last_fault.is_some_and(|n| jobs(&st) > n + workers) {+ break;+ }+ d.ok("engine.execute", json!({"command": "playhead.set", "params": {"seconds": f64::from(k) * 0.1}})); d.settle();+ st = d.frame_stats();+ if d.harness.state().frames.queue_len() > 0 {+ break; // no worker is taking jobs: the assertions below say so+ } }- let st = d.frame_stats();- assert_eq!(st["failedJobs"].as_u64(), Some(workers as u64 + 2), "{st}");- let done_after_faults = st["jobs"].as_u64().unwrap_or(0);- assert!(done_after_faults > jobs_before, "workers still finish jobs after the panics: {st}");+ assert_eq!(st["failedJobs"].as_u64(), Some(faults), "{st}");+ let at_last_fault = at_last_fault.unwrap_or(u64::MAX);+ assert!(jobs(&st) > at_last_fault.saturating_add(workers), "workers still finish jobs after the panics (from {at_last_fault}): {st}"); assert_eq!(d.harness.state().frames.queue_len(), 0, "no job stuck in the queue"); }