test(scheduler-sim-reconcile-terminal-guard): task 1.1 - RED tests for terminal guard
Adds two failing tests formalizing bug-1764's reproduction: a job driven to JOB_STAT_DONE (and, separately, JOB_STAT_EXIT) via a real xce_core_push_events() JOB_STATE_CHANGE call correctly has its allocs/accounts nulled by the existing end-of-life cleanup, but sim_reconcile_unaccounted_jobs() then wrongly resurrects both fields when presented with a stray DISPATCH entry for that same jobid, since its trigger checks only job->accounts == NULL with no state check. The pre-existing eager-capture regression test, test_reconcile_captures_non_run_state_job_when_dispatch_recorded, already covers Requirement 1.2's "PEND job's early DISPATCH still captured" case explicitly and needed no duplicate. No production code touched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAWMhfRJFnmmnG8h97yUDd
This commit is contained in:
parent
f854d9facd
commit
ff05c37bca
2 changed files with 192 additions and 2 deletions
|
|
@ -1,7 +1,7 @@
|
||||||
# Implementation Plan
|
# Implementation Plan
|
||||||
|
|
||||||
- [ ] 1. Core: terminal-state guard on job reconciliation
|
- [ ] 1. Core: terminal-state guard on job reconciliation
|
||||||
- [ ] 1.1 Write failing tests for the terminal-state guard
|
- [x] 1.1 Write failing tests for the terminal-state guard
|
||||||
- Cover: a job driven to JOB_STAT_DONE via xce_core_push_events() (confirming allocs/accounts are nulled by the existing cleanup), then presented with a decoded_session carrying a DISPATCH entry for that job id, must leave allocs/accounts NULL after sim_reconcile_unaccounted_jobs() runs; the same scenario with JOB_STAT_EXIT instead of JOB_STAT_DONE; a PEND job's early-recorded DISPATCH entry is still captured exactly as today (explicit regression guard, not just relying on the pre-existing suite)
|
- Cover: a job driven to JOB_STAT_DONE via xce_core_push_events() (confirming allocs/accounts are nulled by the existing cleanup), then presented with a decoded_session carrying a DISPATCH entry for that job id, must leave allocs/accounts NULL after sim_reconcile_unaccounted_jobs() runs; the same scenario with JOB_STAT_EXIT instead of JOB_STAT_DONE; a PEND job's early-recorded DISPATCH entry is still captured exactly as today (explicit regression guard, not just relying on the pre-existing suite)
|
||||||
- Observable: the new test cases for DONE/EXIT fail against today's sim_reconcile_unaccounted_jobs(), which resurrects allocs/accounts on any job with accounts == NULL regardless of state
|
- Observable: the new test cases for DONE/EXIT fail against today's sim_reconcile_unaccounted_jobs(), which resurrects allocs/accounts on any job with accounts == NULL regardless of state
|
||||||
- _Requirements: 1.1, 1.2, 1.3_
|
- _Requirements: 1.1, 1.2, 1.3_
|
||||||
|
|
|
||||||
|
|
@ -3,7 +3,18 @@
|
||||||
* sim_reconcile_unaccounted_jobs() (sim_loop.c, scheduler-sim-decision-
|
* sim_reconcile_unaccounted_jobs() (sim_loop.c, scheduler-sim-decision-
|
||||||
* parity spec, tasks 2.2-2.4, "Recorded-Decision Reconciliation" component,
|
* parity spec, tasks 2.2-2.4, "Recorded-Decision Reconciliation" component,
|
||||||
* design.md; Requirement 1.1, 1.2, 3.1, 3.2, 3.3, 4.1, 4.2). Also covers
|
* design.md; Requirement 1.1, 1.2, 3.1, 3.2, 3.3, 4.1, 4.2). Also covers
|
||||||
* scheduler-sim-eager-reconciliation task 2.1 (Requirement 1.2, 1.3, 1.4):
|
* scheduler-sim-reconcile-terminal-guard task 1.1 (RED phase, Requirement
|
||||||
|
* 1.1, 1.2, 1.3; bug-1764): the two tests at the very bottom of this file
|
||||||
|
* (test_reconcile_never_reallocates_a_{done,exit}_job_despite_stray_
|
||||||
|
* dispatch_entry) prove a job driven to JOB_STAT_DONE/JOB_STAT_EXIT via
|
||||||
|
* xce_core_push_events() must never have its allocs/accounts resurrected by
|
||||||
|
* a later round's stray/duplicate recorded DISPATCH entry -- both are
|
||||||
|
* EXPECTED TO FAIL against today's sim_reconcile_unaccounted_jobs(), whose
|
||||||
|
* trigger (job->accounts == NULL alone) has no job->state check yet (task
|
||||||
|
* 1.2 adds it). test_reconcile_captures_non_run_state_job_when_dispatch_
|
||||||
|
* recorded() above already serves as this feature's non-terminal-case
|
||||||
|
* regression guard (Requirement 1.2) and needed no duplication.
|
||||||
|
* Also covers scheduler-sim-eager-reconciliation task 2.1 (Requirement 1.2, 1.3, 1.4):
|
||||||
* resource-in-use accounting via the REAL sched_account_jobs() (scheduler.c,
|
* resource-in-use accounting via the REAL sched_account_jobs() (scheduler.c,
|
||||||
* unchanged by that spec, verbatim-copied in below -- see this file's own
|
* unchanged by that spec, verbatim-copied in below -- see this file's own
|
||||||
* "White-box #include set" paragraph) correctly defers to a captured job's
|
* "White-box #include set" paragraph) correctly defers to a captured job's
|
||||||
|
|
@ -1585,6 +1596,181 @@ test_reconcile_step_mode_with_repl_state_still_prints_bare_decision_line(
|
||||||
"decision_line: PASS\n");
|
"decision_line: PASS\n");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* ---------------------------------------------------------------------- */
|
||||||
|
/* Test 18 (scheduler-sim-reconcile-terminal-guard task 1.1, RED phase, */
|
||||||
|
/* Requirement 1.1, 1.3; bug-1764): a job driven all the way to */
|
||||||
|
/* JOB_STAT_DONE via xce_core_push_events() -- whose normal end-of-life */
|
||||||
|
/* cleanup (core_events.c's XCE_CORE_EVENT_JOB_STATE_CHANGE case) correctly */
|
||||||
|
/* frees job->allocs/job->accounts, leaving job->accounts == NULL -- must */
|
||||||
|
/* NEVER have its allocation resurrected by sim_reconcile_unaccounted_ */
|
||||||
|
/* jobs(), even when this round's decoded_session carries a stray/ */
|
||||||
|
/* duplicate recorded DISPATCH entry for that same jobid (the live-observed */
|
||||||
|
/* bug-1764 shape: an array job's retried/leftover dispatch attempt still */
|
||||||
|
/* present in the replay stream after the job has already completed). */
|
||||||
|
/* Today's trigger check (sim_loop.c:690) is `job->accounts != NULL` alone, */
|
||||||
|
/* with NO check on job->state -- it cannot distinguish "not yet allocated" */
|
||||||
|
/* from "already completed and released", so THIS TEST IS EXPECTED TO FAIL */
|
||||||
|
/* until task 1.2 adds the terminal-state exclusion */
|
||||||
|
/* (JOB_IS_DONE()/JOB_IS_EXIT()). */
|
||||||
|
/* ---------------------------------------------------------------------- */
|
||||||
|
|
||||||
|
static void
|
||||||
|
test_reconcile_never_reallocates_a_done_job_despite_stray_dispatch_entry(void)
|
||||||
|
{
|
||||||
|
xce_sched_sim_driver_t sim;
|
||||||
|
xce_sched_job_t* job;
|
||||||
|
xce_job_alloc_t* stray_alloc;
|
||||||
|
replay_session_t* decoded;
|
||||||
|
xce_core_event_t ev;
|
||||||
|
int push_rc;
|
||||||
|
|
||||||
|
fixture_init(&sim);
|
||||||
|
|
||||||
|
/* RUN-state job that already has a real allocation -- the "actively
|
||||||
|
* holding resources" shape a terminal transition must release, same
|
||||||
|
* "already accounted" fixture shape add_job()'s own header comment and
|
||||||
|
* test_reconcile_skips_job_that_already_has_accounts() above establish. */
|
||||||
|
job = add_job(&(sim.core), 7070ULL, JOB_STAT_RUN,
|
||||||
|
make_job_alloc(7070ULL, 0, NULL));
|
||||||
|
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||||
|
job->allocs, "fixture setup: job must start with a real allocation");
|
||||||
|
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||||
|
job->accounts, "fixture setup: job must start with real accounts");
|
||||||
|
|
||||||
|
/* Drive the job to JOB_STAT_DONE through the SAME production event path
|
||||||
|
* bug-1764's root cause depends on -- xce_core_push_events()'s
|
||||||
|
* XCE_CORE_EVENT_JOB_STATE_CHANGE case (core_events.c), same technique
|
||||||
|
* test_reconcile_then_release_via_job_state_change_is_asan_clean() above
|
||||||
|
* already establishes. */
|
||||||
|
memset(&ev, 0, sizeof(ev));
|
||||||
|
ev.type = XCE_CORE_EVENT_JOB_STATE_CHANGE;
|
||||||
|
ev.u.job_state_change.jobid = 7070ULL;
|
||||||
|
ev.u.job_state_change.old_state = JOB_STAT_RUN;
|
||||||
|
ev.u.job_state_change.new_state = JOB_STAT_DONE;
|
||||||
|
|
||||||
|
push_rc = xce_core_push_events(&(sim.core), &ev, 1);
|
||||||
|
TEST_ASSERT_EQUAL_INT(0, push_rc);
|
||||||
|
|
||||||
|
/* Precondition check (NOT this test's main assertion): confirm the
|
||||||
|
* existing end-of-life cleanup genuinely nulled allocs/accounts before
|
||||||
|
* blaming reconciliation for anything below. */
|
||||||
|
TEST_ASSERT_TRUE_MESSAGE(JOB_IS_DONE(job->state),
|
||||||
|
"precondition: job must genuinely be DONE "
|
||||||
|
"before this test's real assertion means "
|
||||||
|
"anything");
|
||||||
|
TEST_ASSERT_NULL_MESSAGE(
|
||||||
|
job->allocs, "precondition: xce_core_push_events()'s normal "
|
||||||
|
"end-of-life cleanup must null job->allocs on a "
|
||||||
|
"terminal transition");
|
||||||
|
TEST_ASSERT_NULL_MESSAGE(
|
||||||
|
job->accounts, "precondition: xce_core_push_events()'s normal "
|
||||||
|
"end-of-life cleanup must null job->accounts on a "
|
||||||
|
"terminal transition");
|
||||||
|
|
||||||
|
/* A later replay round's decoded_session still carries a stray/
|
||||||
|
* duplicate DISPATCH entry for this now-terminal job's jobid -- e.g. a
|
||||||
|
* retried dispatch attempt for one element of an array job (bug-1764's
|
||||||
|
* live-observed shape). */
|
||||||
|
stray_alloc = make_test_job_alloc_with_host(7070ULL, "worker-stray",
|
||||||
|
"slots", 9.0f);
|
||||||
|
decoded = make_decoded_session(
|
||||||
|
2000ULL, make_dispatch_entry_from_alloc(stray_alloc));
|
||||||
|
|
||||||
|
sim_reconcile_unaccounted_jobs(&sim, decoded, NULL);
|
||||||
|
|
||||||
|
/* THE ASSERTION THIS TEST EXISTS TO PROVE (expected to FAIL today): a
|
||||||
|
* DONE job's allocation must never be resurrected, no matter what the
|
||||||
|
* replay stream still contains for it. */
|
||||||
|
TEST_ASSERT_NULL_MESSAGE(
|
||||||
|
job->allocs,
|
||||||
|
"expected allocs NULL after reconciling a terminal (DONE) job, but "
|
||||||
|
"allocs was populated -- today's trigger (job->accounts == NULL "
|
||||||
|
"alone) cannot distinguish 'not yet allocated' from 'already "
|
||||||
|
"completed and released' (bug-1764)");
|
||||||
|
TEST_ASSERT_NULL_MESSAGE(
|
||||||
|
job->accounts,
|
||||||
|
"expected accounts NULL after reconciling a terminal (DONE) job, "
|
||||||
|
"but accounts was populated -- same root cause as the allocs "
|
||||||
|
"assertion above");
|
||||||
|
|
||||||
|
printf("test_reconcile_never_reallocates_a_done_job_despite_stray_"
|
||||||
|
"dispatch_entry: PASS\n");
|
||||||
|
}
|
||||||
|
|
||||||
|
/* ---------------------------------------------------------------------- */
|
||||||
|
/* Test 19 (scheduler-sim-reconcile-terminal-guard task 1.1, RED phase, */
|
||||||
|
/* Requirement 1.1, 1.3; bug-1764): identical scenario to Test 18 above but */
|
||||||
|
/* with JOB_STAT_EXIT instead of JOB_STAT_DONE -- both terminal states must */
|
||||||
|
/* be covered, not just one (Requirement 1.3's "treat a job's terminal */
|
||||||
|
/* state as permanent" applies to EXIT exactly as much as DONE). Also */
|
||||||
|
/* EXPECTED TO FAIL today, for the identical reason as Test 18. */
|
||||||
|
/* ---------------------------------------------------------------------- */
|
||||||
|
|
||||||
|
static void
|
||||||
|
test_reconcile_never_reallocates_an_exit_job_despite_stray_dispatch_entry(
|
||||||
|
void)
|
||||||
|
{
|
||||||
|
xce_sched_sim_driver_t sim;
|
||||||
|
xce_sched_job_t* job;
|
||||||
|
xce_job_alloc_t* stray_alloc;
|
||||||
|
replay_session_t* decoded;
|
||||||
|
xce_core_event_t ev;
|
||||||
|
int push_rc;
|
||||||
|
|
||||||
|
fixture_init(&sim);
|
||||||
|
|
||||||
|
job = add_job(&(sim.core), 7071ULL, JOB_STAT_RUN,
|
||||||
|
make_job_alloc(7071ULL, 0, NULL));
|
||||||
|
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||||
|
job->allocs, "fixture setup: job must start with a real allocation");
|
||||||
|
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||||
|
job->accounts, "fixture setup: job must start with real accounts");
|
||||||
|
|
||||||
|
memset(&ev, 0, sizeof(ev));
|
||||||
|
ev.type = XCE_CORE_EVENT_JOB_STATE_CHANGE;
|
||||||
|
ev.u.job_state_change.jobid = 7071ULL;
|
||||||
|
ev.u.job_state_change.old_state = JOB_STAT_RUN;
|
||||||
|
ev.u.job_state_change.new_state = JOB_STAT_EXIT;
|
||||||
|
|
||||||
|
push_rc = xce_core_push_events(&(sim.core), &ev, 1);
|
||||||
|
TEST_ASSERT_EQUAL_INT(0, push_rc);
|
||||||
|
|
||||||
|
TEST_ASSERT_TRUE_MESSAGE(JOB_IS_EXIT(job->state),
|
||||||
|
"precondition: job must genuinely be EXIT "
|
||||||
|
"before this test's real assertion means "
|
||||||
|
"anything");
|
||||||
|
TEST_ASSERT_NULL_MESSAGE(
|
||||||
|
job->allocs, "precondition: xce_core_push_events()'s normal "
|
||||||
|
"end-of-life cleanup must null job->allocs on a "
|
||||||
|
"terminal transition");
|
||||||
|
TEST_ASSERT_NULL_MESSAGE(
|
||||||
|
job->accounts, "precondition: xce_core_push_events()'s normal "
|
||||||
|
"end-of-life cleanup must null job->accounts on a "
|
||||||
|
"terminal transition");
|
||||||
|
|
||||||
|
stray_alloc = make_test_job_alloc_with_host(7071ULL, "worker-stray-exit",
|
||||||
|
"slots", 9.0f);
|
||||||
|
decoded = make_decoded_session(
|
||||||
|
2100ULL, make_dispatch_entry_from_alloc(stray_alloc));
|
||||||
|
|
||||||
|
sim_reconcile_unaccounted_jobs(&sim, decoded, NULL);
|
||||||
|
|
||||||
|
TEST_ASSERT_NULL_MESSAGE(
|
||||||
|
job->allocs,
|
||||||
|
"expected allocs NULL after reconciling a terminal (EXIT) job, but "
|
||||||
|
"allocs was populated -- today's trigger (job->accounts == NULL "
|
||||||
|
"alone) cannot distinguish 'not yet allocated' from 'already "
|
||||||
|
"completed and released' (bug-1764)");
|
||||||
|
TEST_ASSERT_NULL_MESSAGE(
|
||||||
|
job->accounts,
|
||||||
|
"expected accounts NULL after reconciling a terminal (EXIT) job, "
|
||||||
|
"but accounts was populated -- same root cause as the allocs "
|
||||||
|
"assertion above");
|
||||||
|
|
||||||
|
printf("test_reconcile_never_reallocates_an_exit_job_despite_stray_"
|
||||||
|
"dispatch_entry: PASS\n");
|
||||||
|
}
|
||||||
|
|
||||||
int main(void)
|
int main(void)
|
||||||
{
|
{
|
||||||
UNITY_BEGIN();
|
UNITY_BEGIN();
|
||||||
|
|
@ -1612,5 +1798,9 @@ int main(void)
|
||||||
test_reconcile_continue_mode_with_repl_state_suppresses_bare_decision_line);
|
test_reconcile_continue_mode_with_repl_state_suppresses_bare_decision_line);
|
||||||
RUN_TEST(
|
RUN_TEST(
|
||||||
test_reconcile_step_mode_with_repl_state_still_prints_bare_decision_line);
|
test_reconcile_step_mode_with_repl_state_still_prints_bare_decision_line);
|
||||||
|
RUN_TEST(
|
||||||
|
test_reconcile_never_reallocates_a_done_job_despite_stray_dispatch_entry);
|
||||||
|
RUN_TEST(
|
||||||
|
test_reconcile_never_reallocates_an_exit_job_despite_stray_dispatch_entry);
|
||||||
return UNITY_END();
|
return UNITY_END();
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue