job_queue: drop the erased-recipe test infra
ErasedRecipe and erase() existed for one test, which put three power-op cases in a single array. Three recipe closures have distinct types and cannot share an array element type, so all three were boxed. The array was the reason, not the specs. The per-case assertion is now a helper taking the already-submitted DAG id, so each case submits its own spec at its own concrete type. Nothing is erased and nothing is boxed; production never needed either. The final assertion is also stronger than the one it replaces. claim_ready().is_empty() asks what is runnable at this instant, which a node that is alive but blocked on a dependency passes -- exactly what a leftover compensating node would look like. It now asserts the DAG has no Pending nodes at all. It was also a mutating call inside an assertion: claim_ready settles the graph.
This commit is contained in:
parent
d879d3e67a
commit
96a0679934
1 changed files with 61 additions and 76 deletions
|
|
@ -13,26 +13,6 @@ fn submit<F: FnOnce(&Job)>(q: &JobQueue, spec: DagSpec<F>) -> u64 {
|
||||||
q.submit(spec).expect("valid spec")
|
q.submit(spec).expect("valid spec")
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A spec recipe with its concrete closure type erased. **Test-only** — the
|
|
||||||
/// module used to export this alias for the executor's growth path too, which
|
|
||||||
/// is exactly what a node declaring onto its own builder removed: nothing in
|
|
||||||
/// production stores a recipe to replay later, so nothing needs to box one.
|
|
||||||
type ErasedRecipe = Box<dyn FnOnce(&Job) + Send>;
|
|
||||||
|
|
||||||
/// Erase a spec's recipe so specs of *different* shapes can share one type —
|
|
||||||
/// e.g. a table of `(name, spec)` cases.
|
|
||||||
///
|
|
||||||
/// Production never needs this: each submit path builds one spec and hands it
|
|
||||||
/// straight to `submit`, so the concrete closure type is known end to end. A
|
|
||||||
/// test table is the one case where several shapes must be one type.
|
|
||||||
fn erase<F: FnOnce(&Job) + Send + 'static>(spec: DagSpec<F>) -> DagSpec<ErasedRecipe> {
|
|
||||||
DagSpec {
|
|
||||||
source: spec.source,
|
|
||||||
reason: spec.reason,
|
|
||||||
declare: Box::new(spec.declare),
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
fn ident(s: &str) -> hive_types::Ident {
|
fn ident(s: &str) -> hive_types::Ident {
|
||||||
hive_types::Ident::parse(s).expect("valid test ident")
|
hive_types::Ident::parse(s).expect("valid test ident")
|
||||||
}
|
}
|
||||||
|
|
@ -237,6 +217,20 @@ fn declared_shape(q: &JobQueue, dag: u64) -> Vec<Declared> {
|
||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Kinds of every node under `dag` still `Pending` — the nodes that could yet
|
||||||
|
/// 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> {
|
||||||
|
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.as_str())
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
/// Shorthand for one expected row, so the tables below read as a shape.
|
/// Shorthand for one expected row, so the tables below read as a shape.
|
||||||
fn row(
|
fn row(
|
||||||
kind: &'static str,
|
kind: &'static str,
|
||||||
|
|
@ -1439,66 +1433,57 @@ fn cancel_refuses_running_dag() {
|
||||||
/// deliberately-stopped as far as reconcile and crash-watch are concerned.
|
/// deliberately-stopped as far as reconcile and crash-watch are concerned.
|
||||||
#[test]
|
#[test]
|
||||||
fn cancelled_power_op_runs_no_compensating_node() {
|
fn cancelled_power_op_runs_no_compensating_node() {
|
||||||
|
/// Submit-cancel-assert for one power op. Taking the already-submitted DAG
|
||||||
|
/// id is what removes the need to put three differently-typed recipes in
|
||||||
|
/// one array: each caller submits its own spec, so no closure type has to
|
||||||
|
/// be erased to a boxed one.
|
||||||
|
fn assert_cancels_clean(q: &JobQueue, id: u64, writes_intent: bool, case: &str) {
|
||||||
|
// Read the intent head off the submitted DAG rather than out of the
|
||||||
|
// spec: a declared job holds its own nodes and inserts them.
|
||||||
|
let has_intent = declared_shape(q, id).iter().any(|d| d.kind == "set_wanted");
|
||||||
|
assert_eq!(has_intent, writes_intent, "{case}: intent head");
|
||||||
|
assert!(q.cancel(id), "{case}: cancelled while queued");
|
||||||
|
assert_eq!(state_of(q, id), State::Cancelled);
|
||||||
|
// Nothing is left that *could* run. Asserting on the pending set rather
|
||||||
|
// than on "what is ready this instant" also covers a node that is alive
|
||||||
|
// but blocked — which is exactly what a leftover compensating node
|
||||||
|
// would look like.
|
||||||
|
assert_eq!(
|
||||||
|
pending_kinds(q, id),
|
||||||
|
Vec::<&str>::new(),
|
||||||
|
"{case}: a power op emits no tail node, so a cancelled one leaves nothing"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
for graceful in [false, true] {
|
for graceful in [false, true] {
|
||||||
for running in [false, true] {
|
for running in [false, true] {
|
||||||
let targets = vec![("agent-a".to_owned(), running)];
|
let targets = vec![("agent-a".to_owned(), running)];
|
||||||
// Erased to one boxed recipe type: three different recipe types
|
let case = format!("graceful={graceful} running={running}");
|
||||||
// have to sit in one array.
|
|
||||||
let cases = [
|
let q = JobQueue::new(1);
|
||||||
(
|
let id = submit(
|
||||||
"restart",
|
&q,
|
||||||
false,
|
submit::restart_spec(&targets, graceful, Source::Manual, "bounce".to_owned()),
|
||||||
erase(submit::restart_spec(
|
);
|
||||||
&targets,
|
assert_cancels_clean(&q, id, false, &format!("restart {case}"));
|
||||||
graceful,
|
|
||||||
Source::Manual,
|
let q = JobQueue::new(1);
|
||||||
"bounce".to_owned(),
|
let id = submit(
|
||||||
)),
|
&q,
|
||||||
|
submit::stop_spec(&targets, graceful, Source::Manual, "stop".to_owned()),
|
||||||
|
);
|
||||||
|
assert_cancels_clean(&q, id, true, &format!("stop {case}"));
|
||||||
|
|
||||||
|
let q = JobQueue::new(1);
|
||||||
|
let id = submit(
|
||||||
|
&q,
|
||||||
|
submit::start_spec(
|
||||||
|
&[("agent-a".to_owned(), running, false)],
|
||||||
|
Source::Manual,
|
||||||
|
"start".to_owned(),
|
||||||
),
|
),
|
||||||
(
|
);
|
||||||
"stop",
|
assert_cancels_clean(&q, id, true, &format!("start {case}"));
|
||||||
true,
|
|
||||||
erase(submit::stop_spec(
|
|
||||||
&targets,
|
|
||||||
graceful,
|
|
||||||
Source::Manual,
|
|
||||||
"stop".to_owned(),
|
|
||||||
)),
|
|
||||||
),
|
|
||||||
(
|
|
||||||
"start",
|
|
||||||
true,
|
|
||||||
erase(submit::start_spec(
|
|
||||||
&[("agent-a".to_owned(), running, false)],
|
|
||||||
Source::Manual,
|
|
||||||
"start".to_owned(),
|
|
||||||
)),
|
|
||||||
),
|
|
||||||
];
|
|
||||||
for (name, writes_intent, spec) in cases {
|
|
||||||
let q = JobQueue::new(1);
|
|
||||||
let id = submit(&q, spec);
|
|
||||||
// Read the intent head off the submitted DAG rather than out of
|
|
||||||
// the spec: a declared job holds its own nodes and inserts them.
|
|
||||||
assert_eq!(
|
|
||||||
q.snapshot()
|
|
||||||
.iter()
|
|
||||||
.find(|d| d.id == id)
|
|
||||||
.expect("submitted dag")
|
|
||||||
.nodes
|
|
||||||
.iter()
|
|
||||||
.any(|n| n.kind == "set_wanted"),
|
|
||||||
writes_intent,
|
|
||||||
"{name} intent head (graceful={graceful}, running={running})"
|
|
||||||
);
|
|
||||||
assert!(q.cancel(id), "cancelled while queued");
|
|
||||||
assert_eq!(state_of(&q, id), State::Cancelled);
|
|
||||||
assert!(
|
|
||||||
q.claim_ready().is_empty(),
|
|
||||||
"cancelled {name} (graceful={graceful}, running={running}) must \
|
|
||||||
leave nothing to run — a power op emits no tail node"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue