test(analysis): assert Cancelled event is published; correct cron-test rationale (0.87.21)
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) <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
||||
|
||||
[[package]]
|
||||
name = "mangalord"
|
||||
version = "0.87.20"
|
||||
version = "0.87.21"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.87.20"
|
||||
version = "0.87.21"
|
||||
edition = "2021"
|
||||
default-run = "mangalord"
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user