lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

test(scheduler-sim-reconcile-terminal-guard): task 2.2 - final REPL-level verification

Browse source 
Adds test_reconcile_info_worker_and_summary_hide_completed_job_after_stray_dispatch,
mirroring bug-1764's actual live-observed shape: a worker with one job
driven to a terminal state and two jobs genuinely still RUN, all real
allocations to the SAME worker. After reconciling a stray DISPATCH
entry for the terminal job's id, info worker excludes it while still
listing the two running jobs, and info summary's running count agrees.

Round 1 review rejected an initial version for using a mismatched host
string on the stray entry ("workerarr-stray" instead of "workerarr"),
which made the info worker/info summary assertions pass vacuously
regardless of whether the fix existed, since repl_info_worker_one()
filters assigned jobs by an exact host-string match. Round 2 fixed the
host string and confirmed via an independently-reproduced mutation
test (fix reverted, the test's own accounting-level pre-check
bypassed) that the REPL-level assertions now genuinely, independently
detect the bug.

design.md's Testing Strategy is corrected: it originally said no new
test was needed, reasoning the existing display-path suites already
covered this; tasks.md's own task 2.2 correctly required a
purpose-built test for this exact scenario instead.

Full scheduler test suite: 63/64 OK, sole failure the pre-existing,
unrelated bug-1758. This is the final task of the
scheduler-sim-reconcile-terminal-guard spec; all implementation tasks
(1.1-2.2) are now complete.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAWMhfRJFnmmnG8h97yUDd
This commit is contained in:
Rongsong Shen 2026-09-03 08:31:37 +00:00
parent 4b26120fc0
commit 9dcffce8d8
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
3 changed files with 256 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

6
.kiro/specs/scheduler-sim-reconcile-terminal-guard/design.md
View file

@ -132,6 +132,8 @@ No new logging — this is a silent skip, matching the function's existing silen
- The same scenario with `JOB_STAT_EXIT` instead of `JOB_STAT_DONE` (1.1, 1.3 — both terminal states must be covered, not just one).
- A PEND job's early-recorded DISPATCH entry is still captured exactly as before (regression guard for 1.2 — must continue passing unmodified from the existing suite, and a new test if the existing suite doesn't already cover this exact "not yet RUN, gets captured" shape explicitly).
### Integration Tests (existing suites, re-run, no new tests required)
- `test_sim_e2e_original_motivation.c` / `test_repl_resource_table.c`'s `info worker` assigned-jobs tests: re-run to confirm no regression (Requirement 2's observable outcome; no new test needed since these already exercise the real display path against real job state).
### Integration Tests
**Correction (post-review, task 2.2)**: this section originally read "existing suites, re-run, no new tests required," reasoning that `test_sim_e2e_original_motivation.c`/`test_repl_resource_table.c` already exercise `info worker`'s real display path closely enough. `tasks.md`'s own task 2.2 text (approved at the same time as this document) correctly went further and required a new, purpose-built test for the exact bug-1764 scenario — those existing suites exercise the general display path, not this specific stray-dispatch-after-terminal-state case. That new test now lives in `test_sim_reconcile_unaccounted.c` (`test_reconcile_info_worker_and_summary_hide_completed_job_after_stray_dispatch`), alongside the unit tests below rather than in a separate REPL-focused file, since this file already carries every fixture primitive the scenario needs. Review's first round rejected an initial version of this test for using a mismatched host string on the stray entry (`"workerarr-stray"` instead of `"workerarr"`), which made its `info worker`/`info summary` assertions pass vacuously regardless of whether the fix existed; round 2 confirmed via an independently-reproduced mutation test (fix reverted, test's own accounting-level pre-check bypassed) that the corrected test's REPL-level assertions now genuinely, independently detect the bug.
- `test_reconcile_info_worker_and_summary_hide_completed_job_after_stray_dispatch` (`test_sim_reconcile_unaccounted.c`, new): a worker with one job driven to a terminal state and two jobs genuinely still RUN, all real allocations to the SAME worker (mirroring bug-1764's actual live shape — a stray dispatch entry for the SAME worker the job was originally sent to); after reconciling a stray DISPATCH entry for the terminal job's id, `info worker <host>` excludes it while still listing the two running jobs, and `info summary`'s running count is consistent with what `info worker` shows (2.1, 2.2).
- `test_sim_e2e_original_motivation.c` / `test_repl_resource_table.c`'s `info worker` assigned-jobs tests: re-run to confirm no regression to the general display path (unrelated to bug-1764's specific scenario, but a useful sanity check).
- Full scheduler test suite: confirm no regression anywhere else, matching the discipline already established in this session's other specs.

2
.kiro/specs/scheduler-sim-reconcile-terminal-guard/tasks.md
View file

@ -19,7 +19,7 @@
- Observable: full test suite result showing no regression, with the pre-existing eager-capture and resync-merge test names explicitly confirmed still passing
- _Requirements: 1.2_
- [ ] 2.2 Re-verify against a realistic array-job scenario that info worker's assigned-jobs listing no longer shows a completed job
- [x] 2.2 Re-verify against a realistic array-job scenario that info worker's assigned-jobs listing no longer shows a completed job
- Using the same capture_dispatch()-driven REPL test harness already established in test_sim_e2e_original_motivation.c/test_repl_resource_table.c, construct a multi-job scenario mirroring bug-1764's live-observed shape: an array job where some elements are driven to JOB_STAT_DONE/JOB_STAT_EXIT via xce_core_push_events() (as in 1.1's tests) while others remain running, then advance the sim driver through a further round whose decoded_session carries a stray/duplicate DISPATCH entry (built the same way as 1.1's regression tests) for one of the now-terminal job ids. Issue info worker <host> and info summary through capture_dispatch() and assert the completed job is absent from the assigned-jobs listing, the still-running jobs are present, and the two commands' counts agree
- Observable: a captured info worker transcript showing the completed job absent from the assigned-jobs listing, with the still-running jobs present and info summary's counts consistent with it
- _Requirements: 2.1, 2.2_

251
src/scheduler/testsuite/test_sim_reconcile_unaccounted.c
View file

@ -1771,6 +1771,255 @@ test_reconcile_never_reallocates_an_exit_job_despite_stray_dispatch_entry(
"dispatch_entry: PASS\n");
}
/* ---------------------------------------------------------------------- */
/* REPL-capture helper (scheduler-sim-reconcile-terminal-guard task 2.2): */
/* duplicated from test_repl_resource_table.c's/test_repl_breakpoints_ */
/* inspection.c's own identical capture_dispatch()/capture_stdout_around() */
/* helpers, per this file family's established "duplicate rather than share */
/* via a new header" convention (design.md's File Structure Plan; those two */
/* files' own header comments document the same choice). This file already */
/* white-box #includes the full repl.c/repl_fmt.c/repl_rollup.c/repl_break. */
/* c/repl_info.c/repl_params.c stack every OTHER test here needs for */
/* sim_reconcile_unaccounted_jobs()'s own xce_repl_check_decision() call -- */
/* only the stdout-redirect capture helpers themselves are new here, not a */
/* new #include. */
/* ---------------------------------------------------------------------- */
static char* capture_stdout_around(void (*fn)(void))
{
char tmp[] = "/tmp/xce_simreconcile_replcap_XXXXXX";
int fd = mkstemp(tmp);
int saved_stdout;
off_t len;
char* buf;
TEST_ASSERT_TRUE_MESSAGE(fd >= 0, "mkstemp for stdout capture must "
"succeed");
fflush(stdout);
saved_stdout = dup(STDOUT_FILENO);
TEST_ASSERT_TRUE_MESSAGE(saved_stdout >= 0, "dup(STDOUT_FILENO) must "
"succeed");
TEST_ASSERT_TRUE_MESSAGE(dup2(fd, STDOUT_FILENO) >= 0,
"dup2 redirect of STDOUT_FILENO must succeed");
fn();
fflush(stdout);
dup2(saved_stdout, STDOUT_FILENO);
close(saved_stdout);
len = lseek(fd, 0, SEEK_CUR);
TEST_ASSERT_TRUE(len >= 0);
buf = (char*)calloc(1, (size_t)len + 1);
TEST_ASSERT_NOT_NULL(buf);
TEST_ASSERT_TRUE(lseek(fd, 0, SEEK_SET) == 0);
if (len > 0) {
ssize_t nread = read(fd, buf, (size_t)len);
TEST_ASSERT_TRUE(nread == len);
}
close(fd);
unlink(tmp);
return buf;
}
static xce_sched_sim_driver_t* g_dispatch_sim;
static const char* g_dispatch_line;
static void run_dispatch_for_capture(void)
{
(void)repl_dispatch(g_dispatch_sim, g_dispatch_line);
}
static char* capture_dispatch(xce_sched_sim_driver_t* sim, const char* line)
{
g_dispatch_sim = sim;
g_dispatch_line = line;
return capture_stdout_around(run_dispatch_for_capture);
}
/* ---------------------------------------------------------------------- */
/* Test 20 (scheduler-sim-reconcile-terminal-guard task 2.2, Requirement */
/* 2.1, 2.2 -- FINAL task of this spec): REPL-visible confirmation of tasks */
/* 1.1/1.2's fix, mirroring bug-1764's live-observed shape -- an array job */
/* where some elements are DONE/EXIT while others remain running -- at the */
/* `info worker`/`info summary` display level Requirement 2 actually cares */
/* about, not just at the job->allocs/job->accounts level Tests 18/19 above */
/* already pin. */
/* */
/* Three sibling jobs, all dispatched onto the SAME worker ("workerarr"): */
/* - jobid 7080: driven to JOB_STAT_DONE via xce_core_push_events() */
/* (XCE_CORE_EVENT_JOB_STATE_CHANGE, the exact production path Test 18 */
/* uses), which correctly frees its real starting allocation. */
/* - jobids 7081/7082: remain genuinely JOB_STAT_RUN with their own real, */
/* distinct allocations to the same worker throughout. */
/* A later round's decoded_session then carries a stray/duplicate DISPATCH */
/* entry for jobid 7080 -- bug-1764's own live-observed cause (a retried/ */
/* leftover dispatch attempt for one array element still present in the */
/* replay stream after that element already completed) -- built with the */
/* exact same make_test_job_alloc_with_host()/make_dispatch_entry_from_ */
/* alloc()/make_decoded_session() helpers Tests 18/19 use. */
/* sim_reconcile_unaccounted_jobs() is called directly, the same convention */
/* design.md's Testing Strategy establishes and every other test in this */
/* file follows (the function is `static`, not driven through a full */
/* round-boundary path by any test in this suite). */
/* */
/* "info worker workerarr" and "info summary" are then captured through */
/* capture_dispatch() (above). Requirement 2.1: the completed job's id must */
/* be ABSENT from the assigned-jobs listing while both still-running jobs' */
/* ids remain present. Requirement 2.2 (display consistency): info */
/* summary's own running count -- computed purely from job->state via */
/* JOB_IS_RUN(), entirely independent of job->allocs/job->accounts -- must */
/* agree with exactly those two still-running jobs, proving the assigned- */
/* jobs listing and the running count tell the SAME story once the guard is */
/* in place (bug-1764's actual symptom was these two views disagreeing: */
/* the assigned-jobs listing showed 3 while running was correctly only 2). */
/* ---------------------------------------------------------------------- */
static void
test_reconcile_info_worker_and_summary_hide_completed_job_after_stray_dispatch(
void)
{
xce_sched_sim_driver_t sim;
xce_sched_worker_t* w;
xce_sched_job_t* job_done;
xce_sched_job_t* job_run1;
xce_sched_job_t* job_run2;
xce_job_alloc_t* alloc_done_initial;
xce_job_alloc_t* alloc_run1;
xce_job_alloc_t* alloc_run2;
xce_job_alloc_t* stray_alloc;
replay_session_t* decoded;
xce_core_event_t ev;
int residx;
int push_rc;
char* captured_worker;
char* captured_summary;
char done_jobid_str[64];
char run1_jobid_str[64];
char run2_jobid_str[64];
fixture_init(&sim);
residx = register_slots_resource(&(sim.core));
w = add_worker(&(sim.core), "workerarr", residx, 20.0f);
TEST_ASSERT_NOT_NULL(w);
/* jobid 7080 starts genuinely RUN with a real allocation -- the
* "actively holding resources" shape a terminal transition must
* release, same fixture shape Test 18/19 above establish. */
alloc_done_initial
= make_test_job_alloc_with_host(7080ULL, "workerarr", "slots", 3.0f);
job_done = add_job(&(sim.core), 7080ULL, JOB_STAT_RUN, alloc_done_initial);
TEST_ASSERT_NOT_NULL_MESSAGE(
job_done->allocs,
"fixture setup: job_done must start with a real allocation");
TEST_ASSERT_NOT_NULL_MESSAGE(
job_done->accounts,
"fixture setup: job_done must start with real accounts");
/* jobids 7081/7082: the "some elements remain running" half of the
* array-job scenario -- genuinely RUN, each with its own real, distinct
* allocation to the SAME worker, untouched for the rest of this test. */
alloc_run1 = make_test_job_alloc_with_host(7081ULL, "workerarr", "slots",
2.0f);
job_run1 = add_job(&(sim.core), 7081ULL, JOB_STAT_RUN, alloc_run1);
TEST_ASSERT_NOT_NULL(job_run1->allocs);
alloc_run2 = make_test_job_alloc_with_host(7082ULL, "workerarr", "slots",
4.0f);
job_run2 = add_job(&(sim.core), 7082ULL, JOB_STAT_RUN, alloc_run2);
TEST_ASSERT_NOT_NULL(job_run2->allocs);
/* Drive jobid 7080 to JOB_STAT_DONE through the SAME production event
* path bug-1764's root cause depends on (Test 18's own technique). */
memset(&ev, 0, sizeof(ev));
ev.type = XCE_CORE_EVENT_JOB_STATE_CHANGE;
ev.u.job_state_change.jobid = 7080ULL;
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);
TEST_ASSERT_TRUE_MESSAGE(JOB_IS_DONE(job_done->state),
"precondition: job_done must genuinely be DONE "
"before this test's real assertions mean "
"anything");
TEST_ASSERT_NULL_MESSAGE(
job_done->allocs, "precondition: normal end-of-life cleanup must "
"null job_done->allocs on its terminal "
"transition");
TEST_ASSERT_NULL_MESSAGE(
job_done->accounts, "precondition: normal end-of-life cleanup must "
"null job_done->accounts on its terminal "
"transition");
/* A later replay round's decoded_session still carries a stray/
* duplicate DISPATCH entry for jobid 7080 -- bug-1764's live-observed
* shape (a retried dispatch attempt for one array element still present
* in the replay stream after that element already completed). */
stray_alloc = make_test_job_alloc_with_host(7080ULL, "workerarr",
"slots", 9.0f);
decoded = make_decoded_session(
9000ULL, make_dispatch_entry_from_alloc(stray_alloc));
sim_reconcile_unaccounted_jobs(&sim, decoded, NULL);
/* Sanity re-check (the accounting-level half of this task's own
* Observable, already covered by Tests 18/19 -- repeated here only to
* localize a failure before trusting the REPL-level assertions below). */
TEST_ASSERT_NULL_MESSAGE(
job_done->allocs,
"job_done's allocation must still be NULL after reconciling this "
"round's stray DISPATCH entry for its own jobid");
TEST_ASSERT_NULL_MESSAGE(job_done->accounts,
"job_done's accounts must likewise still be "
"NULL");
/* ---- The REPL-visible assertions this task actually exists for. ---- */
fmt_jobid(done_jobid_str, sizeof(done_jobid_str), 7080ULL);
fmt_jobid(run1_jobid_str, sizeof(run1_jobid_str), 7081ULL);
fmt_jobid(run2_jobid_str, sizeof(run2_jobid_str), 7082ULL);
captured_worker = capture_dispatch(&sim, "info worker workerarr");
TEST_ASSERT_NULL_MESSAGE(
strstr(captured_worker, done_jobid_str),
"Requirement 2.1: the completed job must be ABSENT from 'info "
"worker's assigned-jobs listing, even though this round's "
"decoded_session carried a stray DISPATCH entry for its jobid "
"(bug-1764)");
TEST_ASSERT_NOT_NULL_MESSAGE(
strstr(captured_worker, run1_jobid_str),
"the first still-running job must be present in the assigned-jobs "
"listing");
TEST_ASSERT_NOT_NULL_MESSAGE(
strstr(captured_worker, run2_jobid_str),
"the second still-running job must be present in the assigned-jobs "
"listing");
free(captured_worker);
captured_summary = capture_dispatch(&sim, "info summary");
TEST_ASSERT_NOT_NULL_MESSAGE(
strstr(captured_summary, "3 jobs (0 pending, 2 running)"),
"Requirement 2.2: 'info summary's own running count (computed "
"purely from job->state, independent of job->allocs/accounts) must "
"agree with the assigned-jobs listing above -- exactly the two "
"still-running jobs, never inflated by the completed one");
free(captured_summary);
printf(
"test_reconcile_info_worker_and_summary_hide_completed_job_after_"
"stray_dispatch: PASS\n");
}
int main(void)
{
UNITY_BEGIN();
@ -1802,5 +2051,7 @@ int main(void)
test_reconcile_never_reallocates_a_done_job_despite_stray_dispatch_entry);
RUN_TEST(
test_reconcile_never_reallocates_an_exit_job_despite_stray_dispatch_entry);
RUN_TEST(
test_reconcile_info_worker_and_summary_hide_completed_job_after_stray_dispatch);
return UNITY_END();
}