diff --git a/src/browser_retirement.rs b/src/browser_retirement.rs index 7090dfa..d20d7da 100644 --- a/src/browser_retirement.rs +++ b/src/browser_retirement.rs @@ -1127,6 +1127,12 @@ async fn retire( let _ = browser .close_owned_target(id, remaining(operation_deadline)?, cancel) .await; + #[cfg(test)] + let operation_deadline = crate::browser_target::TEST_CLOSE_WAIT + .get() + .map_or(operation_deadline, |wait| { + operation_deadline.min(Instant::now() + wait) + }); loop { if targets(&browser, operation_deadline) .await? diff --git a/src/browser_runner.rs b/src/browser_runner.rs index 7ee8c26..4a56393 100644 --- a/src/browser_runner.rs +++ b/src/browser_runner.rs @@ -4186,10 +4186,22 @@ fn capture_image_summary( let capture = capture_outcome(store, id, witness, outcome, &text, None, cancel)?; Ok((capture, text)) } +const ATTACHMENT_BUDGET_FLOOR: Duration = Duration::from_secs(45); +#[cfg(test)] +thread_local! { + static TEST_ATTACHMENT_BUDGET_FLOOR: std::cell::Cell> = const { std::cell::Cell::new(None) }; +} +fn attachment_budget_floor() -> Duration { + #[cfg(test)] + if let Some(floor) = TEST_ATTACHMENT_BUDGET_FLOOR.get() { + return floor; + } + ATTACHMENT_BUDGET_FLOOR +} /// Reference attachment completion budget: max(inputTimeoutMs, 45 s) plus 20 s for each /// further file, raised to `attachmentTimeoutMs`. All of a batch's uploads share it. fn attachment_budget(config: &Value, limits: &RunnerLimits, files: usize) -> Duration { - let budget = limits.command.max(Duration::from_secs(45)) + let budget = limits.command.max(attachment_budget_floor()) + Duration::from_secs(20).saturating_mul(files.saturating_sub(1) as u32); budget.max(Duration::from_millis( config["attachmentTimeoutMs"].as_u64().unwrap_or(0), diff --git a/src/browser_runner_tests.rs b/src/browser_runner_tests.rs index f4b8756..1b119c4 100644 --- a/src/browser_runner_tests.rs +++ b/src/browser_runner_tests.rs @@ -2063,6 +2063,11 @@ fn limits() -> RunnerLimits { bringup: Duration::from_secs(5), } } +/// For uploads that wait out their budget by design: the fixture's command budget, in place of +/// the 45 s production floor. `attachment_budget` still takes the larger of the two. +const WAITED_OUT_ATTACHMENT_FLOOR: Duration = Duration::from_secs(5); +/// For closes the fixture refuses, which production watches for its whole budget. +const REFUSED_CLOSE_WAIT: Duration = Duration::from_secs(1); /// Limits for cases whose commit or answer deadline expires before a bounded provider read. /// That read's whole budget is min(commit, answer), so the fixture's 500 ms leaves too little /// for its identity checks and receipts once a parallel suite stalls native publication. @@ -5670,6 +5675,7 @@ async fn a_lock_held_through_cleanup_and_warning_keeps_the_completed_answer() { #[tokio::test] async fn completed_close_retries_resume_only_the_exact_durable_marker() { + crate::browser_target::TEST_CLOSE_WAIT.set(Some(REFUSED_CLOSE_WAIT)); for explicit in [false, true] { for fault in [ Fault::CloseRefusedTwice, @@ -5740,10 +5746,12 @@ async fn completed_close_retries_resume_only_the_exact_durable_marker() { stop(f).await; } } + crate::browser_target::TEST_CLOSE_WAIT.set(None); } #[tokio::test] async fn same_process_reconciliation_never_closes_running_or_uncertain_requests() { + crate::browser_target::TEST_CLOSE_WAIT.set(Some(REFUSED_CLOSE_WAIT)); for case in [ "running", "send-uncertain", @@ -5793,6 +5801,7 @@ async fn same_process_reconciliation_never_closes_running_or_uncertain_requests( assert_eq!(f.sends.load(Ordering::SeqCst), 1); stop(f).await; } + crate::browser_target::TEST_CLOSE_WAIT.set(None); } #[tokio::test] @@ -6724,6 +6733,9 @@ async fn source_mismatch_historical_echo_incomplete_answer_and_lost_ack_never_re #[tokio::test] async fn owned_upload_must_reach_readiness_and_cleanup_before_send() { for fault in [Fault::None, Fault::UploadFailed] { + // The failure waits out the budget by design; the successful upload keeps the real one. + TEST_ATTACHMENT_BUDGET_FLOOR + .set(matches!(fault, Fault::UploadFailed).then_some(WAITED_OUT_ATTACHMENT_FLOOR)); let f = fixture(fault, true).await; let result = run(&f).await; assert_eq!(result.is_ok(), matches!(fault, Fault::None)); @@ -6758,6 +6770,7 @@ async fn owned_upload_must_reach_readiness_and_cleanup_before_send() { assert!(f.home.path().join("public.txt").exists()); stop(f).await; } + TEST_ATTACHMENT_BUDGET_FLOOR.set(None); } #[tokio::test] async fn unconfirmed_upload_commands_or_cleanup_never_authorise_close_or_send() { @@ -6770,6 +6783,18 @@ async fn unconfirmed_upload_commands_or_cleanup_never_authorise_close_or_send() Fault::UploadCleanupWrongOwner, Fault::UploadCleanupWrongNavigation, ] { + // These wait out the budget by design; the wrong receipts refuse on their first read and + // keep the real budget, so expiry cannot stand in for the refusal they test. + TEST_ATTACHMENT_BUDGET_FLOOR.set( + matches!( + fault, + Fault::UploadObjectUnacknowledged + | Fault::UploadDescribeUnacknowledged + | Fault::UploadCommandUnacknowledged + | Fault::UploadCleanupFailed + ) + .then_some(WAITED_OUT_ATTACHMENT_FLOOR), + ); let f = fixture(fault, true).await; assert!(run(&f).await.is_err(), "{fault:?}"); assert_eq!(f.sends.load(Ordering::SeqCst), 0, "{fault:?}"); @@ -6789,6 +6814,7 @@ async fn unconfirmed_upload_commands_or_cleanup_never_authorise_close_or_send() ); stop(f).await; } + TEST_ATTACHMENT_BUDGET_FLOOR.set(None); } #[tokio::test] async fn a_cleaned_second_upload_failure_retires_the_owned_target_with_its_first_upload() { diff --git a/src/browser_target.rs b/src/browser_target.rs index 22033a8..f35b787 100644 --- a/src/browser_target.rs +++ b/src/browser_target.rs @@ -10,6 +10,13 @@ use tokio::{ use tokio_util::sync::CancellationToken; const LATE_CLEANUP: Duration = Duration::from_millis(750); +#[cfg(test)] +thread_local! { + /// Shortens the watch after a close the fixture refuses, not the close command itself. The + /// watch only reads the target list, so unlike the command budget it needs no room for a + /// durable write. + pub(crate) static TEST_CLOSE_WAIT: std::cell::Cell> = const { std::cell::Cell::new(None) }; +} fn allocation_remaining(deadline: std::time::Instant) -> Result { let remaining = deadline.saturating_duration_since(std::time::Instant::now()); anyhow::ensure!( @@ -487,6 +494,10 @@ impl OwnedTarget { .browser .close_owned_target(&self.id, timeout, &cancel) .await; + #[cfg(test)] + let deadline = TEST_CLOSE_WAIT + .get() + .map_or(deadline, |wait| deadline.min(Instant::now() + wait)); loop { let remaining = deadline.saturating_duration_since(Instant::now()); anyhow::ensure!( @@ -565,6 +576,10 @@ impl OwnedTarget { .browser .close_owned_target(&self.id, remaining, &CancellationToken::new()) .await; + #[cfg(test)] + let deadline = TEST_CLOSE_WAIT.get().map_or(deadline, |wait| { + deadline.min(std::time::Instant::now() + wait) + }); loop { if self.observe_absence_until(deadline).await? { return Ok(());