feat(scheduler-sim-repl-verbosity): task 4.2 - retire redundant per-decision echo
Gates sim_print_decision() so the bare per-decision line no longer prints under continue mode, making the Round Rollup's summary the sole source of decision output (Requirement 2.3, closing bug-1610). step mode and non-interactive/headless runs are completely unaffected (Requirements 5.1, 5.4). Implemented as a file-local inline wrapper in sim_loop.c rather than repl.h, since the gated function is static to that file and repl.h can't see repl_state's internals without leaking them to every other repl.h consumer. Fixes 3 pre-existing tests (predating this spec, from scheduler-core) that pinned the now-intentionally-changed bare-line behavior under continue mode - each fixed differently based on its actual purpose rather than uniformly: two kept continue mode with their anchor switched to the Round Rollup's summary (since they specifically test continue-mode breakpoint interruption, which step mode would trivialize), one switched to step mode (since its actual invariant - print-before-breakpoint-check call order - is mode-independent and continue mode no longer has any observable anchor for it). Review caught a second, unguarded sim_print_decision() call site in sim_reconcile_unaccounted_jobs() (added by an earlier, separately- merged spec) that design.md's boundary never accounted for - per the review, this is actually the dominant real-world path for DISPATCH decisions in replay mode, so leaving it unguarded would have defeated Requirement 2.3's "sole source of decision output" claim for the common case. Gated it through the same wrapper; added 2 new tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAWMhfRJFnmmnG8h97yUDd
This commit is contained in:
parent
3d08a43a35
commit
d9109b7c4b
7 changed files with 386 additions and 39 deletions
|
|
@ -110,7 +110,7 @@ flowchart TD
|
|||
- `src/scheduler/src/repl_rollup.c` (new, post-review — Correction above) — the Round Rollup's own fingerprint-build/pend-jobid-query/emit/flush/round-boundary-process logic (Requirements 2, 3), split out of `repl.c` to stay under the 800-line cap; shares state with `repl.c` via `repl_internal.h`, matching `repl_break.c`/`repl_info.c`/`repl_params.c`'s established split; added to `scheduler_srcs` in `src/scheduler/meson.build` immediately after `repl.c`.
|
||||
- `src/scheduler/src/repl_info.c` — new resource-value classification/scaling helpers (byte-denominated list, `swp`'s megabyte special case, whole-number formatting) and a table/grouping rewrite of `repl_info_worker()`'s and `repl_info_resource()`'s resource-listing loops, both reading `rs->units_mode` (Requirement 6); the assigned-job listing in `repl_info_worker()` adopts the jobid-range-compression helper (Requirement 4.5).
|
||||
- `src/scheduler/src/repl_params.c` — `repl_cmd_set()` gains a `units raw|human|both` branch alongside the existing `speed`/`output`/`verify` branches, storing the selection on the new `rs->units_mode` field (Requirement 6.2, 6.3).
|
||||
- `src/scheduler/src/sim_loop.c` — the existing call site invoking `sim_print_decision()` gains a `continue`-mode gate, mirroring `repl.h`'s inline-wrapper convention (Requirements 2, 5).
|
||||
- `src/scheduler/src/sim_loop.c` — the existing call site invoking `sim_print_decision()` gains a `continue`-mode gate, mirroring `repl.h`'s inline-wrapper convention (Requirements 2, 5). **Correction (post-review, task 4.2 remediation)**: this bullet's original "the existing call site" (singular) undercounted the file's actual production call sites — `sim_loop.c` has TWO direct calls to `sim_print_decision()`, `sched_sim_driver_run()`'s main decision-drain loop (this bullet's original target) AND `sim_reconcile_unaccounted_jobs()`'s reconstructed-decision report (added by the sibling scheduler-sim-decision-parity/scheduler-sim-eager-reconciliation specs, unrelated in authorship but real production code that reports DISPATCH decisions through this exact channel). Since that reconciliation path is a primary source of DISPATCH decisions surfacing in replay mode, leaving it unguarded reproduced bug-1610's own duplication there too. Both call sites now route through `sim_print_decision_gate()` — see the `sim_print_decision` Gate component's own Correction below for the updated Boundary Commitment.
|
||||
- `src/scheduler/include/repl.h` — no functional change expected; existing inline wrappers already cover the gating points this feature needs.
|
||||
|
||||
### New/Modified Test Files
|
||||
|
|
@ -263,7 +263,9 @@ int repl_fmt_jobid_range(char* buf, size_t bufsz,
|
|||
|
||||
**Responsibilities & Constraints**
|
||||
- Skip the direct print only when `sim->interactive && sim->repl_state->mode == XCE_REPL_MODE_CONTINUE`; `step` mode and non-interactive (headless) runs are unaffected (Requirement 5.1, 5.4).
|
||||
- Implemented as an inline-wrapper-style gate at the existing call site in `sched_sim_driver_run()`, consistent with `repl.h`'s established convention, rather than a bare `if` block added to the loop body.
|
||||
- Implemented as an inline-wrapper-style gate, consistent with `repl.h`'s established convention, rather than a bare `if` block added to each call site's body.
|
||||
|
||||
**Correction (post-review, task 4.2 remediation)**: this component's original Boundary Commitment named "the existing call site in `sched_sim_driver_run()`" (singular). Review found `sim_loop.c` has a SECOND, direct, unguarded call to `sim_print_decision()` inside `sim_reconcile_unaccounted_jobs()` (added by the sibling scheduler-sim-decision-parity/scheduler-sim-eager-reconciliation specs after this design.md was originally written) — and that reconciliation path is actually a PRIMARY mechanism by which DISPATCH decisions surface in replay mode at all (most live-drain-loop activity is PEND). A round where reconciliation fires was therefore still showing both the bare per-decision line AND the Round Rollup's summary counting the same dispatch, reproducing bug-1610's own duplication via this second, previously-unaccounted-for call site. This component's gate (same function, same condition, no new symbol) now guards BOTH call sites: `sched_sim_driver_run()`'s main decision-drain loop (this component's original target) and `sim_reconcile_unaccounted_jobs()`'s reconstructed-decision report. Requirement 2.3/this feature's own Observable ("the round summary is the sole source of decision output") is only actually satisfied once both are covered — see `test_sim_reconcile_unaccounted.c`'s two new tests (`test_reconcile_continue_mode_with_repl_state_suppresses_bare_decision_line` / `test_reconcile_step_mode_with_repl_state_still_prints_bare_decision_line`) for the second call site's own contract coverage, parallel to `test_sim_print_decision_gate.c`'s existing coverage of the first.
|
||||
|
||||
**Contracts**: Service [ ] / API [ ] / Event [ ] / Batch [ ] / State [x]
|
||||
|
||||
|
|
|
|||
|
|
@ -52,7 +52,7 @@
|
|||
- _Boundary: sim_print_decision gate_
|
||||
- _Depends: 2.2_
|
||||
|
||||
- [ ] 4.2 (P) Suppress the old per-decision echo during continuous execution so the round summary is the sole source of decision output
|
||||
- [x] 4.2 (P) Suppress the old per-decision echo during continuous execution so the round summary is the sole source of decision output
|
||||
- Leave single-stepping and non-interactive output untouched
|
||||
- Observable: all tests from 4.1 pass; a full continuous-execution run over a replay shows only round-summary lines, with no duplicate bare decision lines
|
||||
- _Requirements: 2.3, 5.1, 5.4_
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
|
|
@ -9,6 +9,11 @@
|
|||
#include "plugin.h"
|
||||
#include "preason.h"
|
||||
#include "repl.h"
|
||||
#include "repl_internal.h" /* scheduler-sim-repl-verbosity spec, task 4.2 --
|
||||
* see this file's own header comment ("Boundary
|
||||
* note") for why this one narrow exception to
|
||||
* repl.h's usual opaque-`repl_state`-pointer
|
||||
* convention is scoped to this file alone. */
|
||||
#include "sim.h"
|
||||
#include "utils.h"
|
||||
#include "xcealloc.h" /* task 2.3: make_job_account() -- Recorded-Decision
|
||||
|
|
@ -29,11 +34,14 @@
|
|||
* (sim_record_write_event()), and per-round ReplaySession accumulation
|
||||
* (recording_session.c's scheduler_t-free sched_record_session_ctx_new()/
|
||||
* _add_decision()/_finalize(), externed directly). sim_print_decision()
|
||||
* remains this file's own unconditional Req 5 AC 19 default; it,
|
||||
* --sim-output recording, and --sim-verify comparison are independently
|
||||
* gated per drained decision. Task 11.4 owns comparing each drained
|
||||
* decision against the recorded ReplaySession.decisions when --sim-verify
|
||||
* is set -- see the ReplaySession branch below and sim_verify.c.
|
||||
* was originally this file's own unconditional Req 5 AC 19 default; task
|
||||
* 4.2 (scheduler-sim-repl-verbosity spec) narrows that default -- see
|
||||
* "Boundary note" below and sim_print_decision_gate()'s own doc comment.
|
||||
* --sim-output recording and --sim-verify comparison remain independently
|
||||
* gated per drained decision, unaffected by task 4.2. Task 11.4 owns
|
||||
* comparing each drained decision against the recorded
|
||||
* ReplaySession.decisions when --sim-verify is set -- see the ReplaySession
|
||||
* branch below and sim_verify.c.
|
||||
*
|
||||
* Task 12.2 (Req 5 AC 22 "Breakpoints") adds three bare hook calls below --
|
||||
* xce_repl_check_event()/xce_repl_check_begin_round()/xce_repl_check_decision()
|
||||
|
|
@ -45,6 +53,38 @@
|
|||
* logic live in repl.h/repl_break.c, this file having zero line-budget
|
||||
* headroom for either (see repl_internal.h's own "Why a split at all").
|
||||
*
|
||||
* Boundary note (task 4.2, scheduler-sim-repl-verbosity spec, design.md's
|
||||
* "sim_print_decision Gate" component, Req 2.3/5.1): that component's own
|
||||
* spec is to skip the direct per-decision print only when `sim->interactive
|
||||
* && sim->repl_state->mode == XCE_REPL_MODE_CONTINUE`, "implemented as an
|
||||
* inline-wrapper-style gate ... consistent with repl.h's established
|
||||
* convention". Every existing repl.h wrapper (xce_repl_check_event() and
|
||||
* siblings) only ever tests the PUBLIC `sim->interactive` field inline and
|
||||
* delegates all repl-internal-state logic (breakpoint matching, tallying,
|
||||
* ...) to an `extern` function implemented in repl.c/repl_break.c, which
|
||||
* alone -- via repl_internal.h -- can see `xce_sched_repl_state_t`'s full
|
||||
* definition (sim.h keeps it an opaque forward-declared pointer precisely
|
||||
* so sim_loop.c/repl.h never need that definition). This gate cannot follow
|
||||
* that exact shape: the function it gates, sim_print_decision() (this
|
||||
* file), is `static` -- private to this translation unit -- so repl.h
|
||||
* cannot wrap a conditional call to it from outside, and moving the check
|
||||
* itself into repl.h would require repl.h to include repl_internal.h,
|
||||
* leaking the full repl_state struct into every OTHER repl.h consumer too
|
||||
* (a strictly bigger boundary break than this file alone taking on a
|
||||
* narrow, explicit, single-purpose include of it). This file therefore
|
||||
* includes repl_internal.h directly (see the #include block above) and
|
||||
* implements the mode check as its own file-local, static inline wrapper
|
||||
* (sim_print_decision_gate(), just below sim_print_decision()) -- same
|
||||
* "inline wrapper hides the condition, call site stays a single bare call
|
||||
* expression" STYLE repl.h's own wrappers use, just co-located with the
|
||||
* `static` function it wraps rather than declared in the shared header.
|
||||
* No new extern symbol crosses out of this file for this purpose, so
|
||||
* test_sim_isolation.c's nm(1) undefined-symbol layer (sched_sim_isolation_lib,
|
||||
* built from sim.c+sim_loop.c alone) is unaffected; its source-text-scan
|
||||
* layer is unaffected too, since repl_internal.h's own only #include is
|
||||
* repl.h, itself already on this file's include list -- no new controller-
|
||||
* facing symbol enters this file's reachable set.
|
||||
*
|
||||
* Picker-driving thread (sim_picker_thread_fn, below): stands in for BOTH
|
||||
* the normal-mode picker AND allocator threads (scheduler.c), which the
|
||||
* simulation driver cannot use directly (no xce_scheduler_t*, Req 5 AC 17)
|
||||
|
|
@ -413,6 +453,50 @@ static void sim_print_decision(const xce_decision_t* d)
|
|||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* sim_print_decision_gate (task 4.2, scheduler-sim-repl-verbosity spec,
|
||||
* design.md's "sim_print_decision Gate" component, Req 2.3/5.1) -- an
|
||||
* inline-wrapper-style gate on sim_print_decision(); see this file's own
|
||||
* header comment ("Boundary note") for why it lives here, file-local,
|
||||
* rather than as a repl.h wrapper. Skips the direct per-decision print
|
||||
* ONLY when `sim->interactive && sim->repl_state->mode ==
|
||||
* XCE_REPL_MODE_CONTINUE` -- the Round Rollup (repl_rollup.c, task 2.2)
|
||||
* already reports the round's decisions in that mode, so the bare
|
||||
* "[sim] DISPATCH/PEND/PREEMPT jobid=..." line would otherwise duplicate
|
||||
* it every round (bug-1610). `step` mode and non-interactive (headless,
|
||||
* `sim->interactive == false`) runs are unaffected -- sim_print_decision()
|
||||
* still fires exactly as before in both (Req 5.1, 5.4).
|
||||
*
|
||||
* **Correction (post-review, task 4.2 remediation):** this gate has TWO
|
||||
* call sites, not one -- sched_sim_driver_run()'s main decision-drain
|
||||
* loop below (the task's original target), AND
|
||||
* sim_reconcile_unaccounted_jobs()'s own reconstructed-decision report
|
||||
* (added later by the sibling scheduler-sim-decision-parity/scheduler-
|
||||
* sim-eager-reconciliation specs, unrelated in authorship but real
|
||||
* production code that reports DISPATCH decisions through this exact
|
||||
* same channel -- see that function's own doc comment and its call site
|
||||
* below). Both were missed by task 4.2's original single-call-site scope;
|
||||
* this remediation wires the second one in too, closing bug-1610's
|
||||
* duplication gap completely rather than only at the drain loop.
|
||||
*
|
||||
* A NULL `sim` or a not-yet-allocated `sim->repl_state` (repl_state is
|
||||
* lazily allocated by xce_repl_start(), repl.c, and is guaranteed non-NULL
|
||||
* whenever `sim->interactive` is true and the REPL has actually started --
|
||||
* see sim.h's own repl_state doc comment) is treated as "print", the same
|
||||
* "never load-bearing, degrade to unattended" default this codebase's
|
||||
* other REPL hooks use for an unexpected NULL.
|
||||
*/
|
||||
static inline void sim_print_decision_gate(const xce_sched_sim_driver_t* sim,
|
||||
const xce_decision_t* d)
|
||||
{
|
||||
if ((sim != NULL) && sim->interactive && (sim->repl_state != NULL)
|
||||
&& (sim->repl_state->mode == XCE_REPL_MODE_CONTINUE)) {
|
||||
return;
|
||||
}
|
||||
|
||||
sim_print_decision(d);
|
||||
}
|
||||
|
||||
/*
|
||||
* sim_picker_thread_fn - the simulation driver's picker-driving thread
|
||||
* (Req 5 AC 18's pseudocode: `core_go()` signals it, the driver's own
|
||||
|
|
@ -547,8 +631,14 @@ static void* sim_picker_thread_fn(void* arg)
|
|||
* 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
|
||||
* active), sim_print_decision_gate() (task 4.2 remediation -- the
|
||||
* step/continue console output, gated exactly like the drain loop's own
|
||||
* call site: suppressed only under `sim->interactive && sim->repl_state->
|
||||
* mode == XCE_REPL_MODE_CONTINUE`, since this reconciliation path is
|
||||
* itself a primary source of DISPATCH decisions surfacing in replay mode
|
||||
* and was therefore ALSO duplicating the Round Rollup's summary,
|
||||
* bug-1610, before this fix -- see sim_print_decision_gate()'s own doc
|
||||
* comment), 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
|
||||
|
|
@ -685,7 +775,15 @@ static void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim,
|
|||
sched_record_session_add_decision(
|
||||
record_ctx, xce_core_now(&(sim->core)), d);
|
||||
}
|
||||
sim_print_decision(d);
|
||||
/* task 4.2 remediation: this call site was originally
|
||||
* an unguarded sim_print_decision(d), missed by task
|
||||
* 4.2's original scope (which only wired the gate into
|
||||
* sched_sim_driver_run()'s drain loop below). Routed
|
||||
* through the same sim_print_decision_gate(sim, d) --
|
||||
* no new symbol, same suppression condition -- see that
|
||||
* function's own doc comment for why this second call
|
||||
* site matters (bug-1610). */
|
||||
sim_print_decision_gate(sim, d);
|
||||
xce_repl_check_decision(sim, d);
|
||||
}
|
||||
}
|
||||
|
|
@ -996,7 +1094,7 @@ int sched_sim_driver_run(xce_sched_sim_driver_t* sim,
|
|||
sched_record_session_add_decision(
|
||||
record_ctx, xce_core_now(&(sim->core)), d);
|
||||
}
|
||||
sim_print_decision(d);
|
||||
sim_print_decision_gate(sim, d);
|
||||
xce_repl_check_decision(sim, d);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1768,12 +1768,12 @@ static void test_event_breakpoint_pauses_repl_at_matching_event(void)
|
|||
char aux_dir[PATH_MAX];
|
||||
char script_path[PATH_MAX];
|
||||
char capture_path[PATH_MAX];
|
||||
char pend_marker[80];
|
||||
char round_marker[96];
|
||||
char jobid_buf[64];
|
||||
FILE* f;
|
||||
char* captured;
|
||||
char* bp_pos;
|
||||
char* pend_pos;
|
||||
char* round_pos;
|
||||
|
||||
f = write_header_and_snapshot(replay_path, sizeof(replay_path),
|
||||
"_evbp");
|
||||
|
|
@ -1798,14 +1798,24 @@ static void test_event_breakpoint_pauses_repl_at_matching_event(void)
|
|||
|
||||
captured = run_group_d_and_capture(replay_path, script_path, capture_path);
|
||||
|
||||
/* bug-1591: sim_print_decision() (sim_loop.c) now renders jobids via
|
||||
* fmt_jobid(), not raw %llu -- this marker must match that same
|
||||
* human-readable form. */
|
||||
/* scheduler-sim-repl-verbosity task 4.2 (Req 2.3, bug-1610): under
|
||||
* `continue` mode sim_print_decision()'s bare per-decision line no
|
||||
* longer prints -- the Round Rollup (repl_rollup.c) reports the
|
||||
* session's decision instead, deferred until the collapsed range
|
||||
* ends. This fixture has exactly one round, so that range is only
|
||||
* flushed (and printed) by xce_repl_on_replay_complete()'s
|
||||
* repl_round_collapse_flush() call, triggered by the script's
|
||||
* trailing `quit`. Look for that round-rollup summary's
|
||||
* "jobids=<...>" field instead of the old bare "PEND jobid=<...>"
|
||||
* line as evidence the session's decision was eventually processed
|
||||
* -- bug-1591's fmt_jobid()-vs-raw-%llu fix applies equally to
|
||||
* repl_fmt_jobid_range()'s single-jobid rendering (repl.c), which is
|
||||
* byte-identical to fmt_jobid()'s own non-array format. */
|
||||
fmt_jobid(jobid_buf, sizeof(jobid_buf), 4242ULL);
|
||||
snprintf(pend_marker, sizeof(pend_marker), "PEND jobid=%s", jobid_buf);
|
||||
snprintf(round_marker, sizeof(round_marker), "jobids=%s", jobid_buf);
|
||||
|
||||
bp_pos = strstr(captured, "hit breakpoint 1 @ event #4: JOB_STATE_CHANGE");
|
||||
pend_pos = strstr(captured, pend_marker);
|
||||
bp_pos = strstr(captured, "hit breakpoint 1 @ event #4: JOB_STATE_CHANGE");
|
||||
round_pos = strstr(captured, round_marker);
|
||||
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||
bp_pos, "the breakpoint must fire exactly at event #4 (jobid=222), "
|
||||
|
|
@ -1813,11 +1823,12 @@ static void test_event_breakpoint_pauses_repl_at_matching_event(void)
|
|||
TEST_ASSERT_NULL_MESSAGE(
|
||||
strstr(captured, "@ event #3"),
|
||||
"the breakpoint must NOT have fired on event #3 (jobid=111)");
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(pend_pos,
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(round_pos,
|
||||
"the session's own PEND decision must "
|
||||
"still have run after the resumed "
|
||||
"still have run (reported via the round "
|
||||
"rollup summary) after the resumed "
|
||||
"breakpoint pause");
|
||||
TEST_ASSERT_TRUE_MESSAGE(bp_pos < pend_pos,
|
||||
TEST_ASSERT_TRUE_MESSAGE(bp_pos < round_pos,
|
||||
"the event breakpoint (mid-batch, before "
|
||||
"push_events) must be reported BEFORE the "
|
||||
"session's own decision output");
|
||||
|
|
@ -1854,10 +1865,24 @@ static void test_decision_breakpoint_pauses_after_decision_output(void)
|
|||
make_tmp_dir(aux_dir, sizeof(aux_dir), "_decbpaux");
|
||||
snprintf(script_path, sizeof(script_path), "%s/script.txt", aux_dir);
|
||||
snprintf(capture_path, sizeof(capture_path), "%s/capture.out", aux_dir);
|
||||
/* task 12.5 (bug-1594): trailing `quit` -- see the event-breakpoint
|
||||
* test above for why. */
|
||||
write_text_file(script_path,
|
||||
"break DECISION type=PEND\ncontinue\ncontinue\nquit\n");
|
||||
/* scheduler-sim-repl-verbosity task 4.2 (Req 2.3, bug-1610): `continue`
|
||||
* mode now suppresses sim_print_decision()'s bare per-decision line
|
||||
* entirely (the Round Rollup reports it instead, deferred until the
|
||||
* collapsed range ends), so this fixture's single round would leave no
|
||||
* "PEND jobid=..." text to anchor the print-before-breakpoint-check
|
||||
* ordering this test exists to prove. That ordering
|
||||
* (sim_print_decision() then xce_repl_on_decision()'s breakpoint
|
||||
* check, sim_loop.c) is a mode-independent call-sequence invariant --
|
||||
* xce_repl_check_decision() (repl.h) gates only on `sim->interactive`,
|
||||
* never on REPL mode -- so `step` mode observes it exactly as well as
|
||||
* `continue` did, and `step` still prints the bare per-decision line
|
||||
* unconditionally (Requirement 5.1, unaffected by task 4.2). `step 1`
|
||||
* pauses after session 1 completes, but the decision breakpoint below
|
||||
* fires and nests its own repl_command_loop() first, so the trailing
|
||||
* `quit` is read from within THAT nested loop -- same "quit ends the
|
||||
* child deterministically" requirement as the `continue` scripts above,
|
||||
* just reached via a different pause mechanism. */
|
||||
write_text_file(script_path, "break DECISION type=PEND\nstep 1\nquit\n");
|
||||
|
||||
captured = run_group_d_and_capture(replay_path, script_path, capture_path);
|
||||
|
||||
|
|
|
|||
|
|
@ -526,14 +526,14 @@ static void test_pre_replay_pause_blocks_before_any_event_is_processed(void)
|
|||
char aux_dir[PATH_MAX];
|
||||
char script_path[PATH_MAX];
|
||||
char capture_path[PATH_MAX];
|
||||
char pend_marker[80];
|
||||
char round_marker[96];
|
||||
char jobid_buf[64];
|
||||
FILE* f;
|
||||
pid_t pid;
|
||||
int status;
|
||||
char* captured;
|
||||
char* ready_pos;
|
||||
char* pend_pos;
|
||||
char* round_pos;
|
||||
|
||||
f = write_header_and_snapshot(replay_path, sizeof(replay_path),
|
||||
"_prepause");
|
||||
|
|
@ -574,23 +574,34 @@ static void test_pre_replay_pause_blocks_before_any_event_is_processed(void)
|
|||
|
||||
captured = read_whole_file(capture_path);
|
||||
|
||||
/* bug-1591: sim_print_decision() (sim_loop.c) now renders jobids via
|
||||
* fmt_jobid(), not raw %llu -- this marker must match that same
|
||||
* human-readable form. */
|
||||
/* scheduler-sim-repl-verbosity task 4.2 (Req 2.3, bug-1610): under
|
||||
* `continue` mode sim_print_decision()'s bare per-decision line no
|
||||
* longer prints -- the Round Rollup (repl_rollup.c) reports the
|
||||
* session's decision instead, deferred until the collapsed range
|
||||
* ends. This fixture has exactly one round, so that range is only
|
||||
* flushed (and printed) by xce_repl_on_replay_complete()'s
|
||||
* repl_round_collapse_flush() call, triggered here by the script's
|
||||
* trailing `quit`. Look for that round-rollup summary's
|
||||
* "jobids=<...>" field instead of the old bare "PEND jobid=<...>"
|
||||
* line as evidence the fixture's one session produced a live
|
||||
* decision -- bug-1591's fmt_jobid()-vs-raw-%llu fix applies equally
|
||||
* to repl_fmt_jobid_range()'s single-jobid rendering (repl.c), which
|
||||
* is byte-identical to fmt_jobid()'s own non-array format. */
|
||||
fmt_jobid(jobid_buf, sizeof(jobid_buf), 4242ULL);
|
||||
snprintf(pend_marker, sizeof(pend_marker), "PEND jobid=%s", jobid_buf);
|
||||
snprintf(round_marker, sizeof(round_marker), "jobids=%s", jobid_buf);
|
||||
|
||||
ready_pos = strstr(captured, "[sim] ready. type 'help' for commands.");
|
||||
pend_pos = strstr(captured, pend_marker);
|
||||
round_pos = strstr(captured, round_marker);
|
||||
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(ready_pos,
|
||||
"pre-replay pause banner must be printed");
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(pend_pos,
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(round_pos,
|
||||
"the fixture's one session must have "
|
||||
"produced a live PEND decision");
|
||||
TEST_ASSERT_TRUE_MESSAGE(ready_pos < pend_pos,
|
||||
"produced a live PEND decision, reported "
|
||||
"via the round rollup summary");
|
||||
TEST_ASSERT_TRUE_MESSAGE(ready_pos < round_pos,
|
||||
"the pre-replay pause banner must appear "
|
||||
"BEFORE any decision output -- proves no "
|
||||
"BEFORE any round-rollup output -- proves no "
|
||||
"event was processed before the pause");
|
||||
|
||||
free(captured);
|
||||
|
|
|
|||
|
|
@ -22,6 +22,24 @@
|
|||
* decision uses, and that an already-accounted job produces NO duplicate
|
||||
* report through this mechanism (Requirement 3.3).
|
||||
*
|
||||
* **Correction (post-review, scheduler-sim-repl-verbosity task 4.2
|
||||
* remediation):** the reporting channel's console-print leg is now
|
||||
* sim_print_decision_gate(sim, d), not a bare sim_print_decision(d) --
|
||||
* review found this file's own reconciliation call site had been missed by
|
||||
* task 4.2's original single-call-site scope (bug-1610). The two tests at
|
||||
* the bottom of this file (test_reconcile_continue_mode_with_repl_state_
|
||||
* suppresses_bare_decision_line / test_reconcile_step_mode_with_repl_
|
||||
* state_still_prints_bare_decision_line) pin that gate's contract at THIS
|
||||
* call site specifically, using the same repl_state_ensure()-based
|
||||
* direct-call fixture technique test_repl_round_rollup.c's own fixtures
|
||||
* establish. test_reconcile_reports_reconstructed_decision_through_
|
||||
* existing_channel above (task 2.4's original test) deliberately leaves
|
||||
* sim->repl_state NULL and continues to assert the bare line IS present --
|
||||
* empirically confirmed unaffected by this remediation, since a NULL
|
||||
* repl_state falls through the gate's own NULL-safety default ("never
|
||||
* load-bearing, degrade to print") exactly like every other REPL hook in
|
||||
* this codebase.
|
||||
*
|
||||
* sim_reconcile_unaccounted_jobs() is `static` and not yet called from
|
||||
* anywhere in production code (task 3 wires it into
|
||||
* sched_sim_driver_run()'s round-boundary sequence) -- called directly here
|
||||
|
|
@ -1377,6 +1395,195 @@ test_reconcile_multiple_jobs_captured_independently_across_different_rounds(
|
|||
"different_rounds: PASS\n");
|
||||
}
|
||||
|
||||
/* ---------------------------------------------------------------------- */
|
||||
/* Test 16 (scheduler-sim-repl-verbosity task 4.2 remediation, post-review: */
|
||||
/* review found this file's own second production call site to */
|
||||
/* sim_print_decision() -- inside sim_reconcile_unaccounted_jobs() itself */
|
||||
/* -- was never gated by sim_print_decision_gate() (sim_loop.c), the ONE */
|
||||
/* other call site task 4.2 originally wired. Since this reconciliation */
|
||||
/* path is a PRIMARY source of DISPATCH decisions surfacing in replay mode */
|
||||
/* (most live-drain-loop activity is PEND), a round where it fires used to */
|
||||
/* show BOTH this bare "[sim] DISPATCH jobid=..." line AND the Round */
|
||||
/* Rollup's summary counting the same dispatch -- the exact bug-1610 */
|
||||
/* duplication this whole task exists to eliminate, just via this second */
|
||||
/* call site. This test proves the fix: with sim->interactive == true AND */
|
||||
/* a properly-allocated sim->repl_state in XCE_REPL_MODE_CONTINUE, the bare */
|
||||
/* line from THIS mechanism must no longer appear -- the Round Rollup's own */
|
||||
/* summary becomes the sole source of decision output here too, matching */
|
||||
/* design.md's Requirement 2.3 ("the round summary is the sole source of */
|
||||
/* decision output") for real this time. */
|
||||
/* ---------------------------------------------------------------------- */
|
||||
|
||||
static void
|
||||
test_reconcile_continue_mode_with_repl_state_suppresses_bare_decision_line(
|
||||
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_gate_continue_XXXXXX";
|
||||
int fd = mkstemp(tmp);
|
||||
int saved_stdout;
|
||||
off_t len;
|
||||
char* captured;
|
||||
|
||||
fixture_init(&sim);
|
||||
|
||||
job = add_job(&(sim.core), 7060ULL, JOB_STAT_RUN, NULL);
|
||||
TEST_ASSERT_NULL(job->accounts);
|
||||
|
||||
jalloc = make_test_job_alloc_with_host(7060ULL, "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);
|
||||
|
||||
/* The gate-relevant fixture shape (unlike test_reconcile_reports_
|
||||
* reconstructed_decision_through_existing_channel above, which
|
||||
* deliberately leaves repl_state NULL to prove the OTHER half of the
|
||||
* gate's NULL-safety contract): a genuinely allocated repl_state in
|
||||
* XCE_REPL_MODE_CONTINUE, the same repl_state_ensure() technique
|
||||
* test_repl_round_rollup.c's own fixtures use for a direct-call (no
|
||||
* real REPL command loop) test. */
|
||||
sim.interactive = true;
|
||||
sim.repl_state = repl_state_ensure(&sim);
|
||||
TEST_ASSERT_NOT_NULL(sim.repl_state);
|
||||
sim.repl_state->mode = XCE_REPL_MODE_CONTINUE;
|
||||
|
||||
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 -- this gate only
|
||||
* ever affects the console print, nothing else in the reporting chain. */
|
||||
TEST_ASSERT_EQUAL_PTR(jalloc, job->allocs);
|
||||
TEST_ASSERT_NOT_NULL(job->accounts);
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(
|
||||
1, record_ctx->n,
|
||||
"the recording channel (sched_record_session_add_decision()) must "
|
||||
"still report the reconstructed decision -- only the console print "
|
||||
"is gated, not the recording");
|
||||
|
||||
/* THE ASSERTION THIS TEST EXISTS TO PROVE: the bare per-decision line
|
||||
* must NOT appear via this call site while sim->interactive &&
|
||||
* sim->repl_state->mode == XCE_REPL_MODE_CONTINUE (bug-1610, this
|
||||
* remediation's own scope). */
|
||||
TEST_ASSERT_NULL_MESSAGE(
|
||||
strstr(captured, "[sim] DISPATCH jobid="),
|
||||
"under continue mode with a genuinely allocated repl_state, "
|
||||
"sim_reconcile_unaccounted_jobs()'s own reconstructed-decision "
|
||||
"report must NOT print the bare per-decision line -- the Round "
|
||||
"Rollup's own summary is the sole source of decision output here "
|
||||
"too, closing bug-1610's duplication gap at this second call site");
|
||||
|
||||
free(captured);
|
||||
free(sim.repl_state);
|
||||
printf(
|
||||
"test_reconcile_continue_mode_with_repl_state_suppresses_bare_"
|
||||
"decision_line: PASS\n");
|
||||
}
|
||||
|
||||
/* ---------------------------------------------------------------------- */
|
||||
/* Test 17 (regression-safety half of Test 16): under `step` mode with a */
|
||||
/* genuinely allocated repl_state, the bare per-decision line from THIS */
|
||||
/* same call site must still appear exactly as before -- only `continue` */
|
||||
/* mode is gated (Requirement 5.1's "step mode ... unaffected" guarantee, */
|
||||
/* now proven for this second call site too, not just the drain loop's). */
|
||||
/* ---------------------------------------------------------------------- */
|
||||
|
||||
static void
|
||||
test_reconcile_step_mode_with_repl_state_still_prints_bare_decision_line(
|
||||
void)
|
||||
{
|
||||
xce_sched_sim_driver_t sim;
|
||||
xce_sched_job_t* job;
|
||||
xce_job_alloc_t* jalloc;
|
||||
replay_session_t* decoded;
|
||||
char tmp[] = "/tmp/xce_simreconcile_gate_step_XXXXXX";
|
||||
int fd = mkstemp(tmp);
|
||||
int saved_stdout;
|
||||
off_t len;
|
||||
char* captured;
|
||||
|
||||
fixture_init(&sim);
|
||||
|
||||
job = add_job(&(sim.core), 7061ULL, JOB_STAT_RUN, NULL);
|
||||
|
||||
jalloc = make_test_job_alloc_with_host(7061ULL, "worker9", "slots", 3.0f);
|
||||
decoded
|
||||
= make_decoded_session(1000ULL, make_dispatch_entry_from_alloc(jalloc));
|
||||
|
||||
sim.interactive = true;
|
||||
sim.repl_state = repl_state_ensure(&sim);
|
||||
TEST_ASSERT_NOT_NULL(sim.repl_state);
|
||||
/* XCE_REPL_MODE_STEP == 0, already repl_state_ensure()'s calloc()
|
||||
* default -- set explicitly regardless, so this test's intent reads
|
||||
* clearly and does not silently depend on the enum's numeric value. */
|
||||
sim.repl_state->mode = XCE_REPL_MODE_STEP;
|
||||
|
||||
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, NULL);
|
||||
|
||||
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(jalloc, job->allocs);
|
||||
TEST_ASSERT_NOT_NULL(job->accounts);
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(
|
||||
strstr(captured, "[sim] DISPATCH jobid="),
|
||||
"while single-stepping (repl_state allocated, mode == "
|
||||
"XCE_REPL_MODE_STEP), sim_reconcile_unaccounted_jobs()'s own "
|
||||
"reconstructed-decision report must still print the bare "
|
||||
"per-decision line exactly as before -- only continue mode is "
|
||||
"gated");
|
||||
|
||||
free(captured);
|
||||
free(sim.repl_state);
|
||||
printf(
|
||||
"test_reconcile_step_mode_with_repl_state_still_prints_bare_"
|
||||
"decision_line: PASS\n");
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
UNITY_BEGIN();
|
||||
|
|
@ -1400,5 +1607,9 @@ int main(void)
|
|||
RUN_TEST(test_reconcile_no_match_leaves_pend_job_uncaptured_and_retries);
|
||||
RUN_TEST(
|
||||
test_reconcile_multiple_jobs_captured_independently_across_different_rounds);
|
||||
RUN_TEST(
|
||||
test_reconcile_continue_mode_with_repl_state_suppresses_bare_decision_line);
|
||||
RUN_TEST(
|
||||
test_reconcile_step_mode_with_repl_state_still_prints_bare_decision_line);
|
||||
return UNITY_END();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue