From 5f5898d167541953cf25fce47d57a2a8f7e6de6f Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 2 Aug 2026 21:20:56 +0200 Subject: [PATCH] c0re: the cancel tests read what cancel left behind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cancel_refuses_running_dag` is deleted outright: c0re's `cancel` is a delegate and hive_jobq already owns that guarantee in `cancel_node_refuses_a_group_with_anything_running`. Claiming a node here to prove it was testing the library through the wrapper. The other three claimed only to ask "what could still run?", which the graph answers directly. `cancel_clears_queued_dag` and `cancel_drops_one_agents_branch_leaving_the_rest` now read pending kinds (the second per-agent, since the point is that one branch died and its sibling didn't). `cancelled_dag_still_runs_its_approval_tail` reads the spared *payload* rather than claiming it: the approval template emits one tail per outcome and which one survives the cancel is the entire assertion. Its trailing "unrelated activity doesn't disturb it" half no longer fails a node in the other DAG — the DAG merely existing is enough to show roll-up is per-DAG. `Claimed` loses `dag_id` and `agent`; nothing reads them any more. --- hive-c0re/src/job_queue/tests.rs | 127 +++++++++++++++++-------------- 1 file changed, 69 insertions(+), 58 deletions(-) diff --git a/hive-c0re/src/job_queue/tests.rs b/hive-c0re/src/job_queue/tests.rs index 1d478b1d..791a97f1 100644 --- a/hive-c0re/src/job_queue/tests.rs +++ b/hive-c0re/src/job_queue/tests.rs @@ -55,10 +55,8 @@ fn stop_online( /// need the payload next to the id. #[derive(Debug, Clone)] struct Claimed { - dag_id: u64, node_id: NodeId, kind: NodeKind, - agent: String, } /// Drive one settle wave and report every node that started. @@ -80,12 +78,7 @@ impl ClaimReady for JobQueue { .into_iter() .filter_map(|node_id| { let kind = sched.graph().node(node_id)?.payload.clone(); - Some(Claimed { - dag_id: sched.graph().root_of(node_id)?.get(), - node_id, - agent: kind.agent().to_owned(), - kind, - }) + Some(Claimed { node_id, kind }) }) .collect() } @@ -225,12 +218,44 @@ fn payload_of(q: &JobQueue, dag: u64, kind: &str) -> NodeKind { /// run. Stronger than asking the scheduler what is *ready right now*: a node /// blocked on a dep is not ready but is very much still alive. fn pending_kinds(q: &JobQueue, dag: u64) -> Vec<&'static str> { + pending_kinds_filtered(q, dag, &|_| true) +} + +/// [`pending_kinds`] restricted to the nodes whose payload names `agent`. +fn pending_kinds_for(q: &JobQueue, dag: u64, agent: &str) -> Vec<&'static str> { + pending_kinds_filtered(q, dag, &|kind: &NodeKind| kind.agent() == agent) +} + +/// The payloads of every node under `dag` still `Pending`, for the cases where +/// *which* of a family of same-kind nodes survived is the assertion — a +/// template emits one tail per outcome and they differ only in what they carry. +fn pending_payloads(q: &JobQueue, dag: u64) -> Vec { let sched = q.sched().lock().expect("job_queue mutex poisoned"); let graph = sched.graph(); let root = graph.resolve_id(dag).expect("dag id is a real node id"); graph .nodes() .filter(|n| n.id != root && graph.root_of(n.id) == Some(root) && n.state == State::Pending) + .map(|n| n.payload.clone()) + .collect() +} + +fn pending_kinds_filtered( + q: &JobQueue, + dag: u64, + keep: &dyn Fn(&NodeKind) -> bool, +) -> Vec<&'static str> { + let sched = q.sched().lock().expect("job_queue mutex poisoned"); + let graph = sched.graph(); + let root = graph.resolve_id(dag).expect("dag id is a real node id"); + graph + .nodes() + .filter(|n| { + n.id != root + && graph.root_of(n.id) == Some(root) + && n.state == State::Pending + && keep(&n.payload) + }) .map(|n| n.payload.as_str()) .collect() } @@ -259,28 +284,8 @@ fn claim_one(q: &JobQueue) -> Claimed { claims.pop().expect("one claim") } -/// Claim an approval DAG's `ResolveApproval` tail and complete it, asserting it -/// is the one built for `expect`. -/// -/// A template emits one tail per outcome and the graph runs exactly one, so the -/// assertion is on *which node was claimed* — that alone says what the approval -/// row is about to be resolved as. Nothing computes it. -fn settle_approval_tail(q: &JobQueue, approval_id: i64, expect: TerminalState) { - let tail = claim_one(q); - assert!( - matches!( - tail.kind, - NodeKind::ResolveApproval { approval_id: got, outcome } - if got == approval_id && outcome == expect - ), - "expected the {expect:?} ResolveApproval tail for #{approval_id}, got {:?}", - tail.kind - ); - q.complete_node(tail.node_id, Ok(())); -} - -/// The `EmitRebuilt` counterpart of [`settle_approval_tail`] — claim the tail the -/// graph let run and assert it's the `ok` one expected. +/// Claim the `EmitRebuilt` tail the graph let run and assert it's the `ok` one +/// expected. fn settle_rebuild_tail(q: &JobQueue, agent: &str, expect_ok: bool) { let tail = claim_one(q); assert!( @@ -1137,12 +1142,13 @@ fn cancel_clears_queued_dag() { assert_eq!(state_of(&q, id), State::Cancelled, "no stale Queued gap"); // Neither `EmitRebuilt` tail accepts a *dropped* dependency — the ok one is // `AFTER_OK`, the failure one keys on elimination — so both are cancelled - // with the work and **nothing is emitted** for a rebuild that never ran. + // with the work and **nothing is left that could still run**: no node is + // spared, so a rebuild that never ran emits nothing. assert!( - q.claim_ready().is_empty(), - "a dropped rebuild reports nothing" + pending_kinds(&q, id).is_empty(), + "a dropped rebuild leaves nothing alive, got {:?}", + pending_kinds(&q, id) ); - assert_eq!(state_of(&q, id), State::Cancelled); } /// `cancel` takes a **node** id, not a DAG id — so an interior node can be @@ -1168,22 +1174,16 @@ fn cancel_drops_one_agents_branch_leaving_the_rest() { assert!(q.cancel(a_root.id), "an interior/group root cancels alone"); - // agent-b's work is untouched and still claimable; agent-a's is not. - let claims = q.claim_ready(); + // agent-a's subgraph is gone; agent-b's is untouched and still alive. assert!( - !claims.is_empty() && claims.iter().all(|c| c.agent == "agent-b"), - "only agent-b remains runnable, got {:?}", - claims.iter().map(|c| c.agent.as_str()).collect::>() + pending_kinds_for(&q, id, "agent-a").is_empty(), + "agent-a's branch was dropped whole, got {:?}", + pending_kinds_for(&q, id, "agent-a") + ); + assert!( + !pending_kinds_for(&q, id, "agent-b").is_empty(), + "agent-b's branch survives its sibling's cancel" ); -} - -#[test] -fn cancel_refuses_running_dag() { - let q = JobQueue::new(1); - let id = submit(&q, rebuild("agent-a", "r")); - let _ = claim_one(&q); - assert!(!q.cancel(id)); - assert_eq!(state_of(&q, id), State::Running); } /// A cancelled power op must run **no** compensating node — not even one that @@ -1192,8 +1192,9 @@ fn cancel_refuses_running_dag() { /// Now structural rather than a property of a hook enum: a power op emits no /// tail node at all, so once its work nodes cancel there is simply nothing left /// to claim. `cancel` also refuses unless every work node is still `Pending` -/// (`cancel_refuses_running_dag`), so a `Cancelled` DAG provably never executed -/// a node: its `SetWanted` never ran and the agent's intent still reads whatever +/// (`hive_jobq`'s `cancel_node_refuses_a_group_with_anything_running`), so a +/// `Cancelled` DAG provably never executed a node: its `SetWanted` never ran +/// and the agent's intent still reads whatever /// the operator last set. A "revert" instead writes the agent's *observed* /// state, which for a down-but-`wanted = Up` agent (crashed, or caught /// mid-bounce) flips the intent to `Offline` and leaves it @@ -1274,15 +1275,25 @@ fn cancelled_dag_still_runs_its_approval_tail() { ); assert!(q.cancel(id), "fully-queued dag cancels"); // The `Cancelled` tail is the only node whose edge accepts a dropped - // dependency, so it is the only one `cancel` spares — and claiming it *is* - // the assertion that the approval gets resolved as cancelled. - settle_approval_tail(&q, 7, TerminalState::Cancelled); + // dependency, so it is the only one `cancel` spares — and *which* tail + // survives is the whole assertion: the template emits one per outcome and + // the spared one names how the approval row is about to be resolved. + // Nothing computes it, so reading the survivor is reading the answer. + let spared = pending_payloads(&q, id); + assert!( + matches!( + spared.as_slice(), + [NodeKind::ResolveApproval { + approval_id: 7, + outcome: TerminalState::Cancelled + }] + ), + "only the cancelled-outcome tail is spared, got {spared:?}" + ); assert_eq!(state_of(&q, id), State::Cancelled); - // Unrelated later activity doesn't disturb the settled DAG. - let other = submit(&q, rebuild("agent-b", "r")); - let c = claim_one(&q); - assert_eq!(c.dag_id, other); - q.complete_node(c.node_id, Err("boom".to_owned())); + // An unrelated DAG landing in the same graph doesn't disturb this one's + // roll-up — the snapshot is per-DAG, not a global state machine. + let _other = submit(&q, rebuild("agent-b", "r")); assert_eq!(state_of(&q, id), State::Cancelled); }