From a62a5f155bf1873f6ff3e7ab333612b0e12934bf Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Tue, 23 Jun 2026 20:46:08 +0200 Subject: [PATCH] test(analysis): assert Cancelled event is published; correct cron-test rationale (0.87.21) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 0.87.13 followup. The Cancelled publish had no asserting test — mechanical revert of the publish block shipped green. New sqlx test subscribes before spawn, drives SlowDispatcher into flight, cancels, asserts both Started and Cancelled frames with breadcrumb fields. Mutation-confirmed. Also corrects: cron-test rationale comment (inverted unlock-axis), Cancelled docstring (self-contradicting clear-triggers sentence). Co-Authored-By: Claude Opus 4.7 (1M context) --- backend/Cargo.lock | 2 +- backend/Cargo.toml | 2 +- backend/src/analysis/events.rs | 9 ++-- backend/tests/analysis_worker.rs | 93 ++++++++++++++++++++++++++++++++ backend/tests/crawler_daemon.rs | 11 ++-- frontend/package.json | 2 +- 6 files changed, 107 insertions(+), 12 deletions(-) diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 5de3fae..791b483 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.87.20" +version = "0.87.21" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 145f90d..ada8b8d 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.87.20" +version = "0.87.21" edition = "2021" default-run = "mangalord" diff --git a/backend/src/analysis/events.rs b/backend/src/analysis/events.rs index 354c75d..c3d7cda 100644 --- a/backend/src/analysis/events.rs +++ b/backend/src/analysis/events.rs @@ -56,10 +56,11 @@ pub enum AnalysisEvent { }, /// The worker cancelled an in-flight dispatch (daemon shutdown or /// settings reload toggling analysis off). The lease was released, - /// not failed — but the dashboard banner already saw `Started` and - /// only clears on `Completed`/`Failed`/`Cancelled`. Without this - /// variant the banner stuck on the cancelled page forever (until a - /// later page kicked off and overwrote it). + /// not failed. The dashboard's "now analyzing" banner clears on + /// `Completed`/`Failed`/`Cancelled`; before this variant existed it + /// listened only to the first two, so a cancel-mid-dispatch left + /// the banner stuck on the cancelled page until a later one + /// kicked off and overwrote it. Cancelled { page_id: Uuid, manga_id: Uuid, diff --git a/backend/tests/analysis_worker.rs b/backend/tests/analysis_worker.rs index ec3d07a..95dc3d9 100644 --- a/backend/tests/analysis_worker.rs +++ b/backend/tests/analysis_worker.rs @@ -489,3 +489,96 @@ async fn worker_shutdown_does_not_block_on_in_flight_dispatch(pool: PgPool) { "cancellation must not burn a job attempt (release, not ack_failed)" ); } + +/// Cancel-mid-dispatch must publish a `Cancelled` event so the admin +/// dashboard's "now analyzing" banner clears — it listens only for +/// `Completed`/`Failed`/`Cancelled` and would otherwise stick on the +/// in-flight page until a later one overwrites it. 0.87.13 added the +/// publish but no test asserted the frame; mechanical revert of the +/// publish block ships green under +/// `worker_shutdown_does_not_block_on_in_flight_dispatch` because that +/// test builds `AnalysisEvents::new()` but never subscribes. +#[sqlx::test(migrations = "./migrations")] +async fn worker_publishes_cancelled_event_on_cancel_mid_dispatch(pool: PgPool) { + let page_id = seed_page(&pool).await; + page_analysis::enqueue_for_page(&pool, page_id, false) + .await + .unwrap(); + + // Subscribe BEFORE spawning so we never miss a frame. + let events = + std::sync::Arc::new(mangalord::analysis::events::AnalysisEvents::new()); + let mut rx = events.subscribe(); + + let dispatcher = SlowDispatcher::new(Duration::from_secs(30)); + let cancel = CancellationToken::new(); + let handle = daemon::spawn( + pool.clone(), + cancel.clone(), + AnalysisDaemonConfig { + dispatcher: dispatcher.clone(), + workers: 1, + job_timeout: Duration::from_secs(30), + events: events.clone(), + readiness: None, + }, + ); + + // Wait for the worker to lease the job and enter the dispatch so + // it has already published `Started`. + wait_for_state(&pool, page_id, "running").await; + + // Cancel. + cancel.cancel(); + tokio::time::timeout(Duration::from_secs(5), handle.shutdown()) + .await + .expect("shutdown must observe cancel and return promptly"); + + // Drain the broadcaster. We must observe `Started` (for our page_id) + // followed by `Cancelled` (for the same page_id). Drop any other + // frames (Enqueued from the initial enqueue path, frames for other + // pages a sibling test might leak via the shared schema). + let mut saw_started = false; + let mut saw_cancelled = false; + let _ = tokio::time::timeout(Duration::from_secs(2), async { + loop { + match rx.try_recv() { + Ok(ev) => { + let v = serde_json::to_value(&ev).unwrap(); + let kind = v["kind"].as_str().unwrap_or(""); + let ev_page = v["page_id"].as_str().map(str::to_string); + if ev_page.as_deref() != Some(&page_id.to_string()) { + continue; + } + if kind == "started" { + saw_started = true; + } else if kind == "cancelled" { + saw_cancelled = true; + // Sanity-check the labelled breadcrumb fields + // the dashboard banner needs to render. + assert!(v["manga_id"].as_str().is_some()); + assert!(v["manga_title"].as_str().is_some()); + assert!(v["chapter_id"].as_str().is_some()); + assert!(v["chapter_number"].is_number()); + assert!(v["page_number"].is_number()); + break; + } + } + Err(tokio::sync::broadcast::error::TryRecvError::Empty) => { + tokio::time::sleep(Duration::from_millis(20)).await; + } + Err(e) => panic!("subscribe error: {e:?}"), + } + } + }) + .await; + + assert!( + saw_started, + "worker should publish a Started frame for the leased page before the cancel" + ); + assert!( + saw_cancelled, + "worker must publish a Cancelled frame on cancel-mid-dispatch so the dashboard banner clears" + ); +} diff --git a/backend/tests/crawler_daemon.rs b/backend/tests/crawler_daemon.rs index f865715..4dad184 100644 --- a/backend/tests/crawler_daemon.rs +++ b/backend/tests/crawler_daemon.rs @@ -896,11 +896,12 @@ async fn cron_shutdown_does_not_block_on_in_flight_metadata_pass(pool: PgPool) { ); // The advisory lock must be released so another replica's cron can - // pick up the slot. Without this assertion, a refactor that moved - // the `pg_advisory_unlock` under the cancel arm (instead of running - // unconditionally below the select!) would ship green — only to - // wedge multi-replica deployments where the second replica's cron - // would block on `pg_try_advisory_lock` forever. `pg_try_advisory_lock` + // pick up the slot. The 0.87.4 fix runs `pg_advisory_unlock` + // UNCONDITIONALLY below the `tokio::select!` — both the + // cancel-arm and the body-completed paths converge there. Without + // this assertion, a refactor that moved the unlock INTO the + // body-completed arm only (skipping the cancel path) would ship + // green under shutdown-latency-only coverage. `pg_try_advisory_lock` // returns true iff the lock is free, so we use it as the probe. let acquired: bool = sqlx::query_scalar("SELECT pg_try_advisory_lock($1)") .bind(CRON_LOCK_KEY) diff --git a/frontend/package.json b/frontend/package.json index 3867eb1..d0e25c8 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.87.20", + "version": "0.87.21", "private": true, "type": "module", "scripts": {