c0re: the cancel tests read what cancel left behind
`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.
This commit is contained in:
parent
a039a10e40
commit
5f5898d167
1 changed files with 69 additions and 58 deletions
|
|
@ -55,10 +55,8 @@ fn stop_online(
|
||||||
/// need the payload next to the id.
|
/// need the payload next to the id.
|
||||||
#[derive(Debug, Clone)]
|
#[derive(Debug, Clone)]
|
||||||
struct Claimed {
|
struct Claimed {
|
||||||
dag_id: u64,
|
|
||||||
node_id: NodeId,
|
node_id: NodeId,
|
||||||
kind: NodeKind,
|
kind: NodeKind,
|
||||||
agent: String,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Drive one settle wave and report every node that started.
|
/// Drive one settle wave and report every node that started.
|
||||||
|
|
@ -80,12 +78,7 @@ impl ClaimReady for JobQueue {
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.filter_map(|node_id| {
|
.filter_map(|node_id| {
|
||||||
let kind = sched.graph().node(node_id)?.payload.clone();
|
let kind = sched.graph().node(node_id)?.payload.clone();
|
||||||
Some(Claimed {
|
Some(Claimed { node_id, kind })
|
||||||
dag_id: sched.graph().root_of(node_id)?.get(),
|
|
||||||
node_id,
|
|
||||||
agent: kind.agent().to_owned(),
|
|
||||||
kind,
|
|
||||||
})
|
|
||||||
})
|
})
|
||||||
.collect()
|
.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
|
/// 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.
|
/// blocked on a dep is not ready but is very much still alive.
|
||||||
fn pending_kinds(q: &JobQueue, dag: u64) -> Vec<&'static str> {
|
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<NodeKind> {
|
||||||
let sched = q.sched().lock().expect("job_queue mutex poisoned");
|
let sched = q.sched().lock().expect("job_queue mutex poisoned");
|
||||||
let graph = sched.graph();
|
let graph = sched.graph();
|
||||||
let root = graph.resolve_id(dag).expect("dag id is a real node id");
|
let root = graph.resolve_id(dag).expect("dag id is a real node id");
|
||||||
graph
|
graph
|
||||||
.nodes()
|
.nodes()
|
||||||
.filter(|n| n.id != root && graph.root_of(n.id) == Some(root) && n.state == State::Pending)
|
.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())
|
.map(|n| n.payload.as_str())
|
||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
@ -259,28 +284,8 @@ fn claim_one(q: &JobQueue) -> Claimed {
|
||||||
claims.pop().expect("one claim")
|
claims.pop().expect("one claim")
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Claim an approval DAG's `ResolveApproval` tail and complete it, asserting it
|
/// Claim the `EmitRebuilt` tail the graph let run and assert it's the `ok` one
|
||||||
/// is the one built for `expect`.
|
/// expected.
|
||||||
///
|
|
||||||
/// 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.
|
|
||||||
fn settle_rebuild_tail(q: &JobQueue, agent: &str, expect_ok: bool) {
|
fn settle_rebuild_tail(q: &JobQueue, agent: &str, expect_ok: bool) {
|
||||||
let tail = claim_one(q);
|
let tail = claim_one(q);
|
||||||
assert!(
|
assert!(
|
||||||
|
|
@ -1137,12 +1142,13 @@ fn cancel_clears_queued_dag() {
|
||||||
assert_eq!(state_of(&q, id), State::Cancelled, "no stale Queued gap");
|
assert_eq!(state_of(&q, id), State::Cancelled, "no stale Queued gap");
|
||||||
// Neither `EmitRebuilt` tail accepts a *dropped* dependency — the ok one is
|
// Neither `EmitRebuilt` tail accepts a *dropped* dependency — the ok one is
|
||||||
// `AFTER_OK`, the failure one keys on elimination — so both are cancelled
|
// `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!(
|
assert!(
|
||||||
q.claim_ready().is_empty(),
|
pending_kinds(&q, id).is_empty(),
|
||||||
"a dropped rebuild reports nothing"
|
"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
|
/// `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");
|
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.
|
// agent-a's subgraph is gone; agent-b's is untouched and still alive.
|
||||||
let claims = q.claim_ready();
|
|
||||||
assert!(
|
assert!(
|
||||||
!claims.is_empty() && claims.iter().all(|c| c.agent == "agent-b"),
|
pending_kinds_for(&q, id, "agent-a").is_empty(),
|
||||||
"only agent-b remains runnable, got {:?}",
|
"agent-a's branch was dropped whole, got {:?}",
|
||||||
claims.iter().map(|c| c.agent.as_str()).collect::<Vec<_>>()
|
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
|
/// 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
|
/// 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
|
/// 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`
|
/// to claim. `cancel` also refuses unless every work node is still `Pending`
|
||||||
/// (`cancel_refuses_running_dag`), so a `Cancelled` DAG provably never executed
|
/// (`hive_jobq`'s `cancel_node_refuses_a_group_with_anything_running`), so a
|
||||||
/// a node: its `SetWanted` never ran and the agent's intent still reads whatever
|
/// `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*
|
/// the operator last set. A "revert" instead writes the agent's *observed*
|
||||||
/// state, which for a down-but-`wanted = Up` agent (crashed, or caught
|
/// state, which for a down-but-`wanted = Up` agent (crashed, or caught
|
||||||
/// mid-bounce) flips the intent to `Offline` and leaves it
|
/// 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");
|
assert!(q.cancel(id), "fully-queued dag cancels");
|
||||||
// The `Cancelled` tail is the only node whose edge accepts a dropped
|
// 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*
|
// dependency, so it is the only one `cancel` spares — and *which* tail
|
||||||
// the assertion that the approval gets resolved as cancelled.
|
// survives is the whole assertion: the template emits one per outcome and
|
||||||
settle_approval_tail(&q, 7, TerminalState::Cancelled);
|
// 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);
|
assert_eq!(state_of(&q, id), State::Cancelled);
|
||||||
// Unrelated later activity doesn't disturb the settled DAG.
|
// An unrelated DAG landing in the same graph doesn't disturb this one's
|
||||||
let other = submit(&q, rebuild("agent-b", "r"));
|
// roll-up — the snapshot is per-DAG, not a global state machine.
|
||||||
let c = claim_one(&q);
|
let _other = submit(&q, rebuild("agent-b", "r"));
|
||||||
assert_eq!(c.dag_id, other);
|
|
||||||
q.complete_node(c.node_id, Err("boom".to_owned()));
|
|
||||||
assert_eq!(state_of(&q, id), State::Cancelled);
|
assert_eq!(state_of(&q, id), State::Cancelled);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue