From 0c237f601a323bb90b9996bef9043185be043dca Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 22 Aug 2026 09:07:24 +0000 Subject: [PATCH] feat(scheduler-sim-decision-parity): report reconciled decisions through the existing channel (task 2.4) Feeds a reconciled job's decision (task 2.3) through the exact same recording/print/REPL-check calls the existing decision-drain loop uses for a freshly-drained decision, via make_dispatch_decision() -- the same DISPATCH-construction idiom the live allocator path already uses. Dedup (Requirement 3.3) needs no new mechanism: the job->accounts gate from tasks 2.2/2.3 already prevents double-reporting, now proven at the reporting layer too (zero recorded decisions / no console output for an already-accounted job). Required extending sim_reconcile_unaccounted_jobs()'s signature with a third parameter, a nullable recording-context handle -- design.md's Service Interface updated to match, with the confirmed reasoning: the handle only exists as a stack-local inside the round loop, and task 3's still-unwired call site sits in that same scope, so this is necessary plumbing, not scope creep. 46/47 scheduler tests pass; the 1 failure is pre-existing bug-1560 (unrelated). Co-Authored-By: Claude Sonnet 5 --- .../scheduler-sim-decision-parity/design.md | 16 +- .../scheduler-sim-decision-parity/tasks.md | 4 +- src/scheduler/src/sim_loop.c | 96 ++++++-- .../test_sim_reconcile_unaccounted.c | 223 +++++++++++++++--- 4 files changed, 278 insertions(+), 61 deletions(-) diff --git a/.kiro/specs/scheduler-sim-decision-parity/design.md b/.kiro/specs/scheduler-sim-decision-parity/design.md index 74665247..e23aa6a1 100644 --- a/.kiro/specs/scheduler-sim-decision-parity/design.md +++ b/.kiro/specs/scheduler-sim-decision-parity/design.md @@ -196,11 +196,23 @@ int sched_account_jobs(xce_sched_core_t* core); * this is deliberate, not an oversight: the caller's own subsequent * --sim-verify positional walk over the same decoded_session depends on * consumed entries already being gone by the time it runs. + * + * record_ctx is nullable and mirrors the existing decision-drain loop's + * own `if (record_ctx != NULL)` gate (this reporting path must recording- + * capture identically to a freshly-drained decision, and recording is + * optional per run) -- CONFIRMED during implementation this third + * parameter is necessary, not avoidable: record_ctx exists only as a + * stack-local inside sched_sim_driver_run()'s per-ReplaySession block + * (no field on xce_sched_sim_driver_t can supply it), and the round- + * boundary call site this function is wired into (task 3) sits in that + * exact same lexical scope, so passing it through is the natural, + * unambiguous plumbing rather than a workaround. */ void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim, - replay_session_t* decoded_session); + replay_session_t* decoded_session, + sched_record_session_ctx_t* record_ctx); ``` -- Preconditions: `sim`/`decoded_session` non-NULL (NULL is a safe no-op, matching this codebase's established "never load-bearing" REPL-hook convention). +- Preconditions: `sim`/`decoded_session` non-NULL (NULL is a safe no-op, matching this codebase's established "never load-bearing" REPL-hook convention). `record_ctx` may be NULL (recording not active for this run). - Postconditions: every RUN-state job either has `job->accounts` set (reconstructed) or is left NULL (genuinely unavailable this round). `decoded_session->decisions`/`n_decisions` reflect the removal of every consumed entry (freed, not merely uncounted -- see memory-ownership note below), leaving the remaining entries in their original relative order. - Invariants: never double-applies (idempotent per job, per the `job->accounts` gate); never fabricates allocation data not present in the recording; never leaves a freed pointer reachable through `job->allocs` or through the remaining `decoded_session->decisions` entries. diff --git a/.kiro/specs/scheduler-sim-decision-parity/tasks.md b/.kiro/specs/scheduler-sim-decision-parity/tasks.md index 3549b58d..22669b00 100644 --- a/.kiro/specs/scheduler-sim-decision-parity/tasks.md +++ b/.kiro/specs/scheduler-sim-decision-parity/tasks.md @@ -45,7 +45,7 @@ - _Depends: 2.2_ - _Boundary: Recorded-Decision Reconciliation_ -- [ ] 2.4 Make a reconciled job's decision visible the same way a fresh one is +- [x] 2.4 Make a reconciled job's decision visible the same way a fresh one is - Feed a reconstructed decision (task 2.3) through the exact same decision-reporting path a freshly-computed decision already uses, preserving its original decision type and resource-allocation detail - If simulation mode's own live decision-making independently produces a fresh decision for a job in the same round it would otherwise be reconciled, report only the fresh one — never both - Observable completion: replaying a fixture where a job was already dispatched before the replay's live decision-making reaches it now shows that job's dispatch decision in step/continue output, exactly once, with the same resource-allocation detail the original recording captured @@ -94,3 +94,5 @@ - (task 2.3) Do NOT check "not found" via `i == decoded_session->n_decisions` after the lookup loop once the loop's array can be mutated mid-iteration (e.g. once `n_decisions` gets decremented by a compaction) -- that check goes stale for a last-index match. Use an explicit `found` flag instead. This bug would only have surfaced later (task 2.3 introduced the mutation that makes the old check wrong) but was caught and fixed here. - (task 2.3) `src/lib/misc.c`'s `free_host_account()` never frees the `strdup`'d `host` field `make_host_account()` (`src/lib/xcealloc.c:119`) allocates -- a real, pre-existing leak, equally present on the live `scheduler.c` dispatch/release path, previously masked because this test suite is registered with `ASAN_OPTIONS=detect_leaks=0`. Logged as bug-1621, intentionally NOT fixed here (out of this feature's boundary -- misc.c isn't owned by scheduler-sim-decision-parity). If a later task's ASAN run surfaces this same leak, it's not a new regression -- don't waste time re-diagnosing it, it's bug-1621. - (task 2.3) `make_job_account()`'s return isn't NULL-checked before use, matching the live-path precedent (`scheduler.c`'s `sched_commit_decision()` has the same gap) -- informational only, not a defect to fix in this feature unless the live path itself gets hardened first. +- (task 2.4) The reconciliation function ended up needing a third parameter (a nullable recording-context handle) beyond the two design.md originally specified, because that handle only exists as a stack-local inside the round-loop and nothing on the driver struct can supply it -- design.md's Service Interface has been updated to match. Task 3 (wiring): the call site sits in the exact same lexical scope where that recording-context variable is already declared, so pass it straight through -- no new plumbing needed. +- (task 2.4) `xce_decision_t` objects built via `make_dispatch_decision()`/`make_pend_decision()` are never freed anywhere in this codebase on any live path (verified by grep across `scheduler/src/*.c`) -- this is an established, accepted pattern here, not something to "fix" if you see it under `detect_leaks=1`. Don't add a free for it; that would deviate from the codebase's own convention without a broader cleanup pass. diff --git a/src/scheduler/src/sim_loop.c b/src/scheduler/src/sim_loop.c index cad09e5c..00c715b9 100644 --- a/src/scheduler/src/sim_loop.c +++ b/src/scheduler/src/sim_loop.c @@ -490,15 +490,15 @@ static void* sim_picker_thread_fn(void* arg) /* * sim_reconcile_unaccounted_jobs - Recorded-Decision Reconciliation * (scheduler-sim-decision-parity spec, design.md's "Recorded-Decision - * Reconciliation" component; tasks 2.2-2.4). THIS function, as it stands - * right now, implements tasks 2.2 AND 2.3: the once-per-round trigger, the - * jobid-based lookup, the "not found" path (Requirement 4.1, 4.2), and (task - * 2.3) the FOUND path's allocation reconstruction, ownership transfer, and - * recorded-array compaction (Requirement 1.1, 1.2, 3.2). Task 2.4 (feeding - * the reconstructed decision through the decision-reporting channel) is - * deliberately NOT built here yet -- see the comment at the end of the FOUND - * branch below. Not yet called from anywhere (task 3 wires this into - * sched_sim_driver_run()'s round-boundary sequence, before + * Reconciliation" component; tasks 2.2-2.4). THIS function now implements + * tasks 2.2, 2.3, AND 2.4: the once-per-round trigger, the jobid-based + * lookup, the "not found" path (Requirement 4.1, 4.2), the FOUND path's + * allocation reconstruction/ownership transfer/recorded-array compaction + * (Requirement 1.1, 1.2, 3.2), and (task 2.4) feeding the reconstructed + * decision through the exact same decision-reporting channel a + * freshly-drained decision uses -- see the end of the FOUND branch below + * (Requirement 3.1, 3.2). Not yet called from anywhere (task 3 wires this + * into sched_sim_driver_run()'s round-boundary sequence, before * sched_account_jobs() -- see that call site's own comment above) -- * standalone and directly unit-testable for now, per design.md's own * Testing Strategy. @@ -509,7 +509,12 @@ static void* sim_picker_thread_fn(void* arg) * guard, per design.md's Implementation Notes. A RUN-state job in * `sim->core.jobs` with `job->accounts == NULL` is the signature of a job * already resolved earlier in the original recording, before this replay - * run's own live decision-making ever considered it. + * run's own live decision-making ever considered it. Requirement 3.3's + * "report only the fresh one" guarantee needs no separate mechanism here: + * a job that independently receives a fresh live decision in this same + * round already has job->accounts set by sched_commit_decision() + * (scheduler.c, unchanged) by the time this trigger check runs, so this + * function simply finds nothing to do for it. * * For each such job, searches `decoded_session->decisions` for a matching * DISPATCH entry by jobid -- a job can only reach RUN-state via a recorded @@ -529,7 +534,18 @@ static void* sim_picker_thread_fn(void* arg) * by shift-down (memmove), not swap-with-last, keeping the remaining * entries in their original relative order. `job->accounts != NULL` * afterward permanently prevents this job from being reconsidered here on a - * later round. + * later round. Finally (task 2.4) a fresh `xce_decision_t` is built from + * `job->allocs` via the same make_dispatch_decision() constructor + * scheduler.c's allocator thread uses, and fed through the identical + * three-call reporting sequence sched_sim_driver_run()'s own decision-drain + * loop uses for a freshly-drained decision: sched_record_session_add_decision() + * (only when `record_ctx` is non-NULL, i.e. --sim-output recording is + * active), sim_print_decision() (the unconditional step/continue console + * output), and xce_repl_check_decision() (the REPL breakpoint/inspection + * hook, itself a safe no-op when not interactive). Like the drain loop's own + * `d`, this reconstructed decision wrapper is intentionally not freed here -- + * matching the codebase's existing, unchanged treatment of every + * freshly-drained decision object. * * If no match is found: `job->accounts`/`job->allocs` are left exactly as * they were (still NULL) -- not an error, not fabricated. Nothing here @@ -543,10 +559,14 @@ static void* sim_picker_thread_fn(void* arg) * * NULL `sim`/`decoded_session` is a safe no-op, matching this codebase's * established "never load-bearing" REPL-hook convention (design.md's - * Service Interface preconditions). + * Service Interface preconditions). `record_ctx` may legitimately be NULL + * (no --sim-output recording active this run) -- that is not an error + * either, it simply gates the recording call the same way the main + * decision-drain loop's own `if (record_ctx != NULL)` check already does. */ static void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim, - replay_session_t* decoded_session) + replay_session_t* decoded_session, + sched_record_session_ctx_t* record_ctx) { iterator_t it; int rc; @@ -638,15 +658,38 @@ static void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim, * sizeof(*(decoded_session->decisions))); decoded_session->n_decisions--; - /* Task 2.4 (NOT this task) feeds this reconstructed decision - * through the same decision-reporting channel a freshly-drained - * decision uses. This task's job ends here: job->allocs/ - * job->accounts are now correctly reconstructed and safe to - * release later, and job->accounts != NULL now permanently - * prevents this job from being reconsidered by this mechanism - * again (Requirement 3.3's double-application guard, reused as - * this task's own "never reconsidered on a later round" - * guarantee). */ + /* Task 2.4: feed the reconstructed decision through the exact + * same reporting channel sched_sim_driver_run()'s own + * decision-drain loop uses for a freshly-drained decision + * (Requirement 3.1, 3.2) -- see this function's own header + * comment above for the full three-call sequence and dedup + * (Requirement 3.3) reasoning. make_dispatch_decision()'s + * return isn't NULL-checked before the report calls below only + * because it IS checked here first: a calloc() failure building + * the wrapper is treated the same "never load-bearing" way as a + * not-yet-allocated repl_state (repl.h) -- job->allocs/ + * job->accounts above are already correctly reconstructed + * either way, so skipping the report on this rare OOM edge case + * loses visibility for one round, not correctness. */ + { + xce_decision_t* d = make_dispatch_decision(job->allocs); + + if (d != NULL) { + if (record_ctx != NULL) { + sched_record_session_add_decision( + record_ctx, xce_core_now(&(sim->core)), d); + } + sim_print_decision(d); + xce_repl_check_decision(sim, d); + } + } + + /* job->allocs/job->accounts are now correctly reconstructed + * and safe to release later, and job->accounts != NULL now + * permanently prevents this job from being reconsidered by + * this mechanism again (Requirement 3.3's double-application + * guard, reused as this task's own "never reconsidered on a + * later round" guarantee). */ break; } @@ -880,10 +923,11 @@ int sched_sim_driver_run(xce_sched_sim_driver_t* sim, continue; } - /* Recorded-Decision Reconciliation (tasks 2.2-2.4, not yet - * implemented) belongs here, before sched_account_jobs() - * below -- see design.md's "Recorded-Decision Reconciliation" - * component. */ + /* Recorded-Decision Reconciliation (sim_reconcile_unaccounted_ + * jobs(), tasks 2.2-2.4 -- implemented above but not yet wired + * in here; task 3 wires this call site) belongs here, before + * sched_account_jobs() below -- see design.md's "Recorded- + * Decision Reconciliation" component. */ sched_account_jobs(&(sim->core)); xce_core_signal_go(&(sim->core)); diff --git a/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c b/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c index 18e290f0..bc5e5510 100644 --- a/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c +++ b/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c @@ -1,21 +1,21 @@ /* * test_sim_reconcile_unaccounted.c - contract test for * sim_reconcile_unaccounted_jobs() (sim_loop.c, scheduler-sim-decision- - * parity spec, task 2.2, "Recorded-Decision Reconciliation" component, - * design.md; Requirement 4.1, 4.2). + * 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). * - * Scope (task 2.2 only, per design.md's Implementation Notes and this - * task's own text): the once-per-round trigger (RUN-state job with - * job->accounts == NULL), the jobid-based search through this round's - * decoded_session->decisions, and the "not found" path -- leave the job + * Scope: the once-per-round trigger (RUN-state job with job->accounts == + * NULL), the jobid-based search through this round's + * decoded_session->decisions, the "not found" path -- leave the job * unaccounted, report usage unavailable (== job->accounts staying NULL, * which sched_account_jobs() already tolerates safely), never treat this as - * an error, and stay safe to re-check next round. Allocation reconstruction - * for a FOUND match (ownership transfer, array compaction, decision - * reporting) is deliberately NOT exercised here -- that is tasks 2.3/2.4's - * own test coverage; this file's "found" test (see below) only asserts - * that a match does not corrupt an already-accounted job, not that a match - * gets reconstructed. + * an error, and stay safe to re-check next round -- AND (task 2.3/2.4) a + * FOUND match's allocation reconstruction (ownership transfer, array + * compaction) plus feeding the reconstructed decision through the same + * three-call reporting channel (sched_record_session_add_decision(), + * sim_print_decision(), xce_repl_check_decision()) a freshly-drained + * decision uses, and that an already-accounted job produces NO duplicate + * report through this mechanism (Requirement 3.3). * * sim_reconcile_unaccounted_jobs() is `static` and not yet called from * anywhere in production code (task 3 wires it into @@ -48,6 +48,7 @@ #include #include #include +#include #include @@ -329,7 +330,7 @@ test_reconcile_leaves_run_job_unaccounted_when_no_recorded_decision(void) * jobid at all -- the replay's own window never captured it. */ decoded = make_decoded_session(1000ULL, NULL); - sim_reconcile_unaccounted_jobs(&sim, decoded); + sim_reconcile_unaccounted_jobs(&sim, decoded, NULL); TEST_ASSERT_NULL_MESSAGE( job->accounts, @@ -360,14 +361,14 @@ static void test_reconcile_is_safe_to_recheck_on_a_later_round(void) job = add_job(&(sim.core), 7002ULL, JOB_STAT_RUN, NULL); decoded_round1 = make_decoded_session(1000ULL, NULL); - sim_reconcile_unaccounted_jobs(&sim, decoded_round1); + sim_reconcile_unaccounted_jobs(&sim, decoded_round1, NULL); TEST_ASSERT_NULL(job->accounts); /* Round 2: still no matching recorded decision -- calling again must * not error, crash, or otherwise behave differently just because this * is a repeat check of the same job. */ decoded_round2 = make_decoded_session(2000ULL, NULL); - sim_reconcile_unaccounted_jobs(&sim, decoded_round2); + sim_reconcile_unaccounted_jobs(&sim, decoded_round2, NULL); TEST_ASSERT_NULL_MESSAGE( job->accounts, @@ -383,16 +384,24 @@ static void test_reconcile_is_safe_to_recheck_on_a_later_round(void) /* earlier or freshly dispatched by the live path) is never re-touched -- */ /* the trigger condition (job->accounts == NULL) must gate correctly even */ /* when a matching recorded DISPATCH entry for the same jobid IS present */ -/* this round (task 2.2 does not implement match-handling yet, but the */ -/* trigger itself must still never fire for an already-accounted job). */ +/* this round. Extended (task 2.4) to also prove Requirement 3.3's "report */ +/* only the fresh one" guarantee at the decision-reporting layer, not just */ +/* at the job->accounts layer: neither the recording channel nor the */ +/* step/continue console output may see a duplicate report for this job. */ /* ---------------------------------------------------------------------- */ static void test_reconcile_skips_job_that_already_has_accounts(void) { - xce_sched_sim_driver_t sim; - xce_sched_job_t* job; - xce_job_account_t* accounts_before; - replay_session_t* decoded; + xce_sched_sim_driver_t sim; + xce_sched_job_t* job; + xce_job_account_t* accounts_before; + replay_session_t* decoded; + sched_record_session_ctx_t* record_ctx; + char tmp[] = "/tmp/xce_simreconcile_dedup_XXXXXX"; + int fd = mkstemp(tmp); + int saved_stdout; + off_t len; + char* captured; fixture_init(&sim); @@ -403,8 +412,36 @@ static void test_reconcile_skips_job_that_already_has_accounts(void) accounts_before = job->accounts; decoded = make_decoded_session(1000ULL, make_dispatch_entry(7003ULL)); + record_ctx = sched_record_session_ctx_new(1000ULL); + TEST_ASSERT_NOT_NULL(record_ctx); - sim_reconcile_unaccounted_jobs(&sim, decoded); + /* Same dup()/dup2() stdout-capture technique test_sim_verify.c's own + * capture_stdout_around()/run_sim_and_capture_stdout() establish, + * inlined here (this call takes more than the zero args that helper's + * function-pointer shape allows). */ + TEST_ASSERT_TRUE_MESSAGE(fd >= 0, "mkstemp for stdout capture must " + "succeed"); + fflush(stdout); + saved_stdout = dup(STDOUT_FILENO); + TEST_ASSERT_TRUE(saved_stdout >= 0); + TEST_ASSERT_TRUE(dup2(fd, STDOUT_FILENO) >= 0); + + sim_reconcile_unaccounted_jobs(&sim, decoded, record_ctx); + + fflush(stdout); + dup2(saved_stdout, STDOUT_FILENO); + close(saved_stdout); + + len = lseek(fd, 0, SEEK_CUR); + TEST_ASSERT_TRUE(len >= 0); + captured = (char*)calloc(1, (size_t)len + 1); + TEST_ASSERT_NOT_NULL(captured); + TEST_ASSERT_TRUE(lseek(fd, 0, SEEK_SET) == 0); + if (len > 0) { + ssize_t nread = read(fd, captured, (size_t)len); + TEST_ASSERT_TRUE(nread == len); + } + close(fd); TEST_ASSERT_EQUAL_PTR_MESSAGE( accounts_before, job->accounts, @@ -413,9 +450,20 @@ static void test_reconcile_skips_job_that_already_has_accounts(void) "double-application guard)"); TEST_ASSERT_EQUAL_INT_MESSAGE( 1, decoded->n_decisions, - "task 2.2 does not consume/compact recorded entries -- that is " - "task 2.3's job"); + "an already-accounted job's matching entry must not be consumed/" + "compacted either -- the trigger never fires for it at all"); + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, record_ctx->n, + "Requirement 3.3: an already-accounted job (fresh live decision or " + "earlier reconciliation) must produce NO recorded-decision report " + "from this mechanism -- reporting only the fresh one, never a " + "duplicate"); + TEST_ASSERT_NULL_MESSAGE( + strstr(captured, "[sim] DISPATCH"), + "Requirement 3.3: an already-accounted job must produce NO " + "step/continue console output from this mechanism either"); + free(captured); printf("test_reconcile_skips_job_that_already_has_accounts: PASS\n"); } @@ -438,7 +486,7 @@ static void test_reconcile_ignores_non_dispatch_entry_with_matching_jobid( decoded = make_decoded_session(1000ULL, make_pend_entry(7004ULL)); - sim_reconcile_unaccounted_jobs(&sim, decoded); + sim_reconcile_unaccounted_jobs(&sim, decoded, NULL); TEST_ASSERT_NULL_MESSAGE( job->accounts, @@ -472,7 +520,7 @@ static void test_reconcile_skips_non_run_state_job(void) * job before any lookup happens. */ decoded = make_decoded_session(1000ULL, make_dispatch_entry(7005ULL)); - sim_reconcile_unaccounted_jobs(&sim, decoded); + sim_reconcile_unaccounted_jobs(&sim, decoded, NULL); TEST_ASSERT_NULL_MESSAGE( job->accounts, @@ -507,7 +555,7 @@ test_reconcile_reconstructs_allocs_and_accounts_from_matching_dispatch(void) decoded = make_decoded_session(1000ULL, make_dispatch_entry_from_alloc(jalloc)); - sim_reconcile_unaccounted_jobs(&sim, decoded); + sim_reconcile_unaccounted_jobs(&sim, decoded, NULL); TEST_ASSERT_EQUAL_PTR_MESSAGE( jalloc, job->allocs, @@ -563,7 +611,7 @@ static void test_reconcile_compacts_via_shift_down_preserving_order(void) decoded->decisions[3] = make_pend_entry(9003ULL); /* C */ decoded->n_decisions = 4; - sim_reconcile_unaccounted_jobs(&sim, decoded); + sim_reconcile_unaccounted_jobs(&sim, decoded, NULL); TEST_ASSERT_NOT_NULL(job->accounts); TEST_ASSERT_EQUAL_INT_MESSAGE( @@ -616,7 +664,7 @@ static void test_reconcile_then_release_via_job_state_change_is_asan_clean(void) 1000ULL, make_dispatch_entry_from_alloc(make_test_job_alloc_with_host( 7012ULL, "worker1", "slots", 4.0f))); - sim_reconcile_unaccounted_jobs(&sim, decoded); + sim_reconcile_unaccounted_jobs(&sim, decoded, NULL); TEST_ASSERT_NOT_NULL(job->accounts); TEST_ASSERT_NOT_NULL(job->allocs); @@ -651,6 +699,115 @@ static void test_reconcile_then_release_via_job_state_change_is_asan_clean(void) "PASS\n"); } +/* ---------------------------------------------------------------------- */ +/* Test 10 (task 2.4, the task's own PRIMARY Observable): a matching */ +/* recorded DISPATCH entry, once reconstructed (task 2.3), is fed through */ +/* the exact same three-call decision-reporting channel a freshly-drained */ +/* decision uses -- sched_record_session_add_decision() (proven via a real */ +/* record_ctx's own accumulated decisions[]/n/n_dispatched fields, exactly */ +/* the same struct sched_sim_driver_run()'s drain loop populates) and */ +/* sim_print_decision() (proven via captured stdout -- the literal */ +/* mechanism by which a decision "shows... in step/continue output", per */ +/* this task's own Observable Completion text). xce_repl_check_decision() */ +/* is also called (not independently observable here without a live REPL */ +/* breakpoint fixture -- repl.c/repl_break.c/repl_info.c are unchanged and */ +/* out of this task's boundary per design.md) but must not crash, which */ +/* this test's mere completion already proves for both sim->interactive */ +/* on and off. Preserves the original decision's type (DISPATCH) and */ +/* resource-allocation detail (Requirement 3.2): the SAME job->allocs */ +/* pointer the reconciliation step already reconstructed is what reaches */ +/* both the recording channel and the console output. */ +/* ---------------------------------------------------------------------- */ + +static void +test_reconcile_reports_reconstructed_decision_through_existing_channel(void) +{ + xce_sched_sim_driver_t sim; + xce_sched_job_t* job; + xce_job_alloc_t* jalloc; + replay_session_t* decoded; + sched_record_session_ctx_t* record_ctx; + char tmp[] = "/tmp/xce_simreconcile_report_XXXXXX"; + int fd = mkstemp(tmp); + int saved_stdout; + off_t len; + char* captured; + + fixture_init(&sim); + + job = add_job(&(sim.core), 7020ULL, JOB_STAT_RUN, NULL); + TEST_ASSERT_NULL(job->accounts); + + jalloc = make_test_job_alloc_with_host(7020ULL, "worker9", "slots", 3.0f); + decoded + = make_decoded_session(1000ULL, make_dispatch_entry_from_alloc(jalloc)); + record_ctx = sched_record_session_ctx_new(1000ULL); + TEST_ASSERT_NOT_NULL(record_ctx); + + /* Exercise the REPL hook too (sim->interactive on) -- must remain a + * safe no-op with no repl_state allocated yet (repl.h's own "never + * load-bearing" precondition), proving xce_repl_check_decision() is + * actually reached and does not crash regardless of interactive mode. */ + sim.interactive = true; + + TEST_ASSERT_TRUE_MESSAGE(fd >= 0, "mkstemp for stdout capture must " + "succeed"); + fflush(stdout); + saved_stdout = dup(STDOUT_FILENO); + TEST_ASSERT_TRUE(saved_stdout >= 0); + TEST_ASSERT_TRUE(dup2(fd, STDOUT_FILENO) >= 0); + + sim_reconcile_unaccounted_jobs(&sim, decoded, record_ctx); + + fflush(stdout); + dup2(saved_stdout, STDOUT_FILENO); + close(saved_stdout); + + len = lseek(fd, 0, SEEK_CUR); + TEST_ASSERT_TRUE(len >= 0); + captured = (char*)calloc(1, (size_t)len + 1); + TEST_ASSERT_NOT_NULL(captured); + TEST_ASSERT_TRUE(lseek(fd, 0, SEEK_SET) == 0); + if (len > 0) { + ssize_t nread = read(fd, captured, (size_t)len); + TEST_ASSERT_TRUE(nread == len); + } + close(fd); + + /* The reconciliation itself (task 2.3) still happened. */ + TEST_ASSERT_EQUAL_PTR(jalloc, job->allocs); + TEST_ASSERT_NOT_NULL(job->accounts); + + /* sched_record_session_add_decision() reporting (Requirement 3.1). */ + TEST_ASSERT_EQUAL_INT_MESSAGE( + 1, record_ctx->n, + "the reconstructed DISPATCH decision must be reported through the " + "recording channel exactly once"); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, record_ctx->n_dispatched, + "must be counted as a DISPATCH"); + TEST_ASSERT_EQUAL_INT_MESSAGE( + ReplayDecision_body_dispatch, record_ctx->decisions[0]->body_which, + "must be recorded with the DISPATCH body shape"); + TEST_ASSERT_EQUAL_PTR_MESSAGE( + jalloc, record_ctx->decisions[0]->body.dispatch->alloc, + "Requirement 3.2: the recorded entry must carry the SAME " + "resource-allocation detail (identical pointer, borrowed -- same " + "convention recording_session.c's own header comment documents " + "for a freshly-drained decision) the reconciliation step " + "reconstructed, not a copy or a different value"); + + /* sim_print_decision() console output (Requirement 3.1 -- this IS the + * "shows... in step/continue output" mechanism). */ + TEST_ASSERT_NOT_NULL_MESSAGE( + strstr(captured, "[sim] DISPATCH jobid="), + "the reconstructed decision must be printed through the same " + "step/continue console output a freshly-computed decision uses"); + + free(captured); + printf("test_reconcile_reports_reconstructed_decision_through_existing_" + "channel: PASS\n"); +} + /* ---------------------------------------------------------------------- */ /* Test 6: NULL `sim`/`decoded_session` is a safe no-op (design.md's */ /* Service Interface preconditions -- "never load-bearing" convention). */ @@ -664,9 +821,9 @@ static void test_reconcile_null_args_are_a_safe_noop(void) fixture_init(&sim); decoded = make_decoded_session(1000ULL, NULL); - sim_reconcile_unaccounted_jobs(NULL, decoded); - sim_reconcile_unaccounted_jobs(&sim, NULL); - sim_reconcile_unaccounted_jobs(NULL, NULL); + sim_reconcile_unaccounted_jobs(NULL, decoded, NULL); + sim_reconcile_unaccounted_jobs(&sim, NULL, NULL); + sim_reconcile_unaccounted_jobs(NULL, NULL, NULL); /* Reaching this line without a crash IS the assertion. */ TEST_ASSERT_TRUE(1); @@ -687,6 +844,8 @@ int main(void) test_reconcile_reconstructs_allocs_and_accounts_from_matching_dispatch); RUN_TEST(test_reconcile_compacts_via_shift_down_preserving_order); RUN_TEST(test_reconcile_then_release_via_job_state_change_is_asan_clean); + RUN_TEST( + test_reconcile_reports_reconstructed_decision_through_existing_channel); RUN_TEST(test_reconcile_null_args_are_a_safe_noop); return UNITY_END(); }