From 6110d12e5a60e2f0d71dedc273dd6c8bcb9bfbf4 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Mon, 28 Sep 2026 10:27:46 +0000 Subject: [PATCH] feat(libxce-alloc-preason-equality): white-box equivalence tests use public job_alloc_eq/job_preason_eq (task 4.1) Co-Authored-By: Claude Opus 5.5 (1M context) --- .../libxce-alloc-preason-equality/tasks.md | 2 +- .../testsuite/test_repl_equivalence.c | 85 ++++++++++++++--- .../testsuite/test_repl_limit_table.c | 81 +++++++++++++--- .../testsuite/test_sim_equivalence.c | 95 +++++++++++++++---- 4 files changed, 217 insertions(+), 46 deletions(-) diff --git a/.kiro/specs/libxce-alloc-preason-equality/tasks.md b/.kiro/specs/libxce-alloc-preason-equality/tasks.md index ce0da2a1..fe857d00 100644 --- a/.kiro/specs/libxce-alloc-preason-equality/tasks.md +++ b/.kiro/specs/libxce-alloc-preason-equality/tasks.md @@ -45,7 +45,7 @@ Test-first throughout (red → green → refactor). Every sub-task leaves the bu - _Boundary: scheduler allocation chain_ - [ ] 4. Integration: move the consumers to the public API -- [ ] 4.1 Move the three white-box equivalence tests to the public comparators +- [x] 4.1 Move the three white-box equivalence tests to the public comparators - Switch the three test files that call the simulator's private comparators to the public allocation and full-match pending-reason comparisons, printing the description in their failure messages and freeing it. - Keep their white-box inclusion of the simulator's verification source, which they still need for the decision-level functions. - At each full-match pending-reason call, add a short comment on why full match is valid there (no unsatisfiable-allocation cache on either side). diff --git a/src/scheduler/testsuite/test_repl_equivalence.c b/src/scheduler/testsuite/test_repl_equivalence.c index cbe452fb..8ffcc71d 100644 --- a/src/scheduler/testsuite/test_repl_equivalence.c +++ b/src/scheduler/testsuite/test_repl_equivalence.c @@ -50,9 +50,12 @@ * every other shape -- robust regardless of exactly how many ReplayEvent * pass-through messages land between sessions on either side. * - * Comparator: reuses sim_verify.c's own leaf-level structural comparators - * (sim_verify_job_alloc_eq()/sim_verify_job_preason_eq(), both `static` but - * visible in this TU via the white-box #include below), the same technique + * Comparator: uses the public libxce leaf-level structural comparators + * (job_alloc_eq() from xcealloc.h, compares `from`; full-match + * job_preason_eq() from preason.h -- valid because neither side runs with + * the unsatisfiable-allocation cache), printing their first-difference + * description on failure. The white-box #include of ../src/sim_verify.c + * below stays for the decision-level functions sim_loop.c calls. Same technique * test_sim_equivalence.c's own replay_decision_content_eq() already * establishes -- duplicated here (this file family's own established * per-file-duplication convention; no shared test-utility header exists @@ -119,7 +122,9 @@ #include "controller.capnp.h" #include "kvmgr.h" +#include "preason.h" #include "scheduler.h" +#include "xcealloc.h" #include "xstring.h" /* Pull in the real functions under test into this TU (white-box #include, @@ -712,37 +717,79 @@ static void free_all_sessions(replay_session_t** sessions, int count) * replay_decision_content_eq - structural equality between two DECODED * ReplayDecision entries (batch's vs the REPL's own, at the same session * and position). Same technique test_sim_equivalence.c's own function of - * the same name establishes (delegating to sim_verify.c's leaf comparators, + * the same name establishes (delegating to the public libxce comparators + * job_alloc_eq()/job_preason_eq(), with an optional first-difference + * description in *why that the caller prints and free()s; * duplicated here per this file family's own no-shared-header convention -- * see this file's header comment). */ static int replay_decision_content_eq(const replay_decision_t* a, - const replay_decision_t* b) + const replay_decision_t* b, + char** why) { + char buf[128]; + + if (why != NULL) { + *why = NULL; + } if ((a == NULL) || (b == NULL)) { + if (why != NULL) { + *why = strdup("decision: NULL argument"); + } return 0; } if (a->body_which != b->body_which) { + if (why != NULL) { + snprintf(buf, sizeof(buf), "body_which: %d != %d", + (int)a->body_which, (int)b->body_which); + *why = strdup(buf); + } return 0; } switch (a->body_which) { case ReplayDecision_body_dispatch: if ((a->body.dispatch == NULL) || (b->body.dispatch == NULL)) { + if (why != NULL) { + *why = strdup("dispatch: NULL body"); + } return 0; } - return sim_verify_job_alloc_eq(a->body.dispatch->alloc, - b->body.dispatch->alloc); + /* Public libxce comparator: compares `from` too (both sides stamp it + * from the deciding session's start), excludes only instid. */ + return job_alloc_eq(a->body.dispatch->alloc, + b->body.dispatch->alloc, why); case ReplayDecision_body_pend: if ((a->body.pend == NULL) || (b->body.pend == NULL)) { + if (why != NULL) { + *why = strdup("pend: NULL body"); + } return 0; } - return (a->body.pend->jobid == b->body.pend->jobid) - && sim_verify_job_preason_eq(a->body.pend->reason, - b->body.pend->reason); + if (a->body.pend->jobid != b->body.pend->jobid) { + if (why != NULL) { + snprintf(buf, sizeof(buf), "pend.jobid: %llu != %llu", + (unsigned long long)a->body.pend->jobid, + (unsigned long long)b->body.pend->jobid); + *why = strdup(buf); + } + return 0; + } + /* Full match (main reason + host- and resource-based detail lists) + * is valid here, unlike --sim-verify's main-reason-only PEND check + * (bug-1845): this test registers no unsatisfiable_lookup_cb, so + * neither side has the unsatisfiable-allocation cache and both build + * the detail lists with the same full per-host computation. */ + return job_preason_eq(a->body.pend->reason, + b->body.pend->reason, why); default: + if (why != NULL) { + snprintf(buf, sizeof(buf), "body_which: unsupported %d", + (int)a->body_which); + *why = strdup(buf); + } return 0; } } @@ -947,11 +994,19 @@ test_repl_step_continue_to_completion_matches_batch_decision_sequence(void) TEST_ASSERT_EQUAL_INT((int)ReplayDecision_body_pend, (int)rd->body_which); - TEST_ASSERT_TRUE_MESSAGE( - replay_decision_content_eq(bd, rd), - "REPL step/continue-to-completion must produce a " - "byte-identical decision to batch mode at this session " - "and position"); + { + char* why = NULL; + char msg[512]; + int eq = replay_decision_content_eq(bd, rd, &why); + + snprintf(msg, sizeof(msg), + "REPL step/continue-to-completion must produce a " + "byte-identical decision to batch mode at this " + "session and position (session %d, decision %d): %s", + i, j, (why != NULL) ? why : "(no description)"); + free(why); + TEST_ASSERT_TRUE_MESSAGE(eq, msg); + } } } diff --git a/src/scheduler/testsuite/test_repl_limit_table.c b/src/scheduler/testsuite/test_repl_limit_table.c index a3e95438..bb7d38ef 100644 --- a/src/scheduler/testsuite/test_repl_limit_table.c +++ b/src/scheduler/testsuite/test_repl_limit_table.c @@ -63,8 +63,10 @@ #include "controller.capnp.h" #include "group.h" #include "kvmgr.h" +#include "preason.h" #include "scheduler.h" #include "xce/types.h" +#include "xcealloc.h" #include "xcelimit.h" #include "xstring.h" @@ -2644,33 +2646,81 @@ static void det_free_all_sessions(replay_session_t** sessions, int count) free(sessions); } +/* + * det_replay_decision_content_eq - structural equality between two decoded + * ReplayDecision entries (baseline vs 'info limit' run), delegating the + * payload to the public libxce comparators job_alloc_eq() (xcealloc.h) and + * the full-match job_preason_eq() (preason.h). On mismatch, *why (when + * `why` is non-NULL) is a malloc'd first-difference description that the + * caller prints and free()s. + */ static int det_replay_decision_content_eq(const replay_decision_t* a, - const replay_decision_t* b) + const replay_decision_t* b, + char** why) { + char buf[128]; + + if (why != NULL) { + *why = NULL; + } if ((a == NULL) || (b == NULL)) { + if (why != NULL) { + *why = strdup("decision: NULL argument"); + } return 0; } if (a->body_which != b->body_which) { + if (why != NULL) { + snprintf(buf, sizeof(buf), "body_which: %d != %d", + (int)a->body_which, (int)b->body_which); + *why = strdup(buf); + } return 0; } switch (a->body_which) { case ReplayDecision_body_dispatch: if ((a->body.dispatch == NULL) || (b->body.dispatch == NULL)) { + if (why != NULL) { + *why = strdup("dispatch: NULL body"); + } return 0; } - return sim_verify_job_alloc_eq(a->body.dispatch->alloc, - b->body.dispatch->alloc); + /* Public libxce comparator: compares `from` too (both sides stamp it + * from the deciding session's start), excludes only instid. */ + return job_alloc_eq(a->body.dispatch->alloc, + b->body.dispatch->alloc, why); case ReplayDecision_body_pend: if ((a->body.pend == NULL) || (b->body.pend == NULL)) { + if (why != NULL) { + *why = strdup("pend: NULL body"); + } return 0; } - return (a->body.pend->jobid == b->body.pend->jobid) - && sim_verify_job_preason_eq(a->body.pend->reason, - b->body.pend->reason); + if (a->body.pend->jobid != b->body.pend->jobid) { + if (why != NULL) { + snprintf(buf, sizeof(buf), "pend.jobid: %llu != %llu", + (unsigned long long)a->body.pend->jobid, + (unsigned long long)b->body.pend->jobid); + *why = strdup(buf); + } + return 0; + } + /* Full match (main reason + host- and resource-based detail lists) + * is valid here, unlike --sim-verify's main-reason-only PEND check + * (bug-1845): this test registers no unsatisfiable_lookup_cb, so + * neither side has the unsatisfiable-allocation cache and both build + * the detail lists with the same full per-host computation. */ + return job_preason_eq(a->body.pend->reason, + b->body.pend->reason, why); default: + if (why != NULL) { + snprintf(buf, sizeof(buf), "body_which: unsupported %d", + (int)a->body_which); + *why = strdup(buf); + } return 0; } } @@ -2875,11 +2925,20 @@ test_repl_step_continue_with_info_limit_interleaved_matches_baseline_decision_se TEST_ASSERT_EQUAL_INT((int)ReplayDecision_body_pend, (int)id->body_which); - TEST_ASSERT_TRUE_MESSAGE( - det_replay_decision_content_eq(bd, id), - "Requirement 5.2: 'info limit' issued mid-session must " - "produce a byte-identical decision to the baseline at this " - "session and position"); + { + char* why = NULL; + char msg[512]; + int eq = det_replay_decision_content_eq(bd, id, &why); + + snprintf(msg, sizeof(msg), + "Requirement 5.2: 'info limit' issued mid-session " + "must produce a byte-identical decision to the " + "baseline at this session and position (session " + "%d, decision %d): %s", + i, j, (why != NULL) ? why : "(no description)"); + free(why); + TEST_ASSERT_TRUE_MESSAGE(eq, msg); + } } } diff --git a/src/scheduler/testsuite/test_sim_equivalence.c b/src/scheduler/testsuite/test_sim_equivalence.c index bd363afe..bcdfb0fa 100644 --- a/src/scheduler/testsuite/test_sim_equivalence.c +++ b/src/scheduler/testsuite/test_sim_equivalence.c @@ -114,17 +114,22 @@ * pass even if event translation or picker-state-rebuild silently dropped * every field the ordering logic itself does not read. * - * Comparator design: reuses sim_verify.c's own leaf-level structural - * comparators, sim_verify_job_alloc_eq()/sim_verify_job_preason_eq() (both - * `static`, but visible in this TU via the white-box #include of - * ../src/sim_verify.c below -- ordinary C translation-unit visibility, not a - * new sharing mechanism). sim_decision_eq() itself (also in sim_verify.c) + * Comparator design: uses the public libxce leaf-level structural + * comparators, job_alloc_eq() (xcealloc.h; compares `from`, excludes only + * instid) and the full-match job_preason_eq() (preason.h; main reason plus + * host- and resource-based detail lists), each reporting a first-difference + * description that the failure message prints. Full-match pending reasons + * are valid here because neither side runs with the scheduler's + * unsatisfiable-allocation cache (no unsatisfiable_lookup_cb is registered), + * so both build the detail lists with the same full per-host computation. + * The white-box #include of ../src/sim_verify.c below stays: sim_loop.c + * calls its decision-level functions. sim_decision_eq() itself (in sim_verify.c) * is NOT reused directly: it compares a replay_decision_t against a LIVE * xce_decision_t (recorded-vs-live), a different pair of top-level shapes * than this file needs (recorded-vs-recorded, replay_decision_t vs * replay_decision_t, R vs S) -- but the SAME xce_job_alloc_t/ - * xce_job_preason_t leaf types and their SAME field-by-field comparators - * apply either way, so this file's own small + * xce_job_preason_t leaf types and the SAME public field-by-field + * comparators apply either way, so this file's own small * replay_decision_content_eq() below delegates to those two leaf * comparators instead of re-deriving the field walk from scratch. * @@ -171,6 +176,7 @@ #include "kvmgr.h" #include "preason.h" #include "scheduler.h" +#include "xcealloc.h" #include "xstring.h" /* Pull in the real functions under test into this TU (white-box #include, @@ -868,41 +874,81 @@ static replay_session_t* decode_session_after_skip(const char* path, int n_skip) * (a different comparison pair than sim_verify.c's own sim_decision_eq(), * which compares a recorded replay_decision_t against a LIVE xce_decision_t * -- see this file's own header comment for why a small new comparator is - * written here rather than reusing sim_decision_eq() itself, while still - * reusing its two LEAF comparators, sim_verify_job_alloc_eq()/ - * sim_verify_job_preason_eq() -- both `static` in sim_verify.c but visible - * in this TU via this file's white-box #include of that file above). + * written here rather than reusing sim_decision_eq() itself), delegating the + * payload to the public libxce comparators job_alloc_eq()/job_preason_eq(). + * On mismatch, *why (when `why` is non-NULL) is a malloc'd description of + * the first difference, which the caller prints and free()s. * AC 7.1 excludes ReplayHeader/SessionSummary from the equivalence claim -- * this function, and every call site below, touches only `body_which` and * the DISPATCH/PEND payload, never `ts` or any summary field. */ static int replay_decision_content_eq(const replay_decision_t* r, - const replay_decision_t* s) + const replay_decision_t* s, + char** why) { + char buf[128]; + + if (why != NULL) { + *why = NULL; + } if ((r == NULL) || (s == NULL)) { + if (why != NULL) { + *why = strdup("decision: NULL argument"); + } return 0; } if (r->body_which != s->body_which) { + if (why != NULL) { + snprintf(buf, sizeof(buf), "body_which: %d != %d", + (int)r->body_which, (int)s->body_which); + *why = strdup(buf); + } return 0; } switch (r->body_which) { case ReplayDecision_body_dispatch: if ((r->body.dispatch == NULL) || (s->body.dispatch == NULL)) { + if (why != NULL) { + *why = strdup("dispatch: NULL body"); + } return 0; } - return sim_verify_job_alloc_eq(r->body.dispatch->alloc, - s->body.dispatch->alloc); + /* Public libxce comparator: compares `from` too (both sides stamp it + * from the deciding session's start), excludes only instid. */ + return job_alloc_eq(r->body.dispatch->alloc, + s->body.dispatch->alloc, why); case ReplayDecision_body_pend: if ((r->body.pend == NULL) || (s->body.pend == NULL)) { + if (why != NULL) { + *why = strdup("pend: NULL body"); + } return 0; } - return (r->body.pend->jobid == s->body.pend->jobid) - && sim_verify_job_preason_eq(r->body.pend->reason, - s->body.pend->reason); + if (r->body.pend->jobid != s->body.pend->jobid) { + if (why != NULL) { + snprintf(buf, sizeof(buf), "pend.jobid: %llu != %llu", + (unsigned long long)r->body.pend->jobid, + (unsigned long long)s->body.pend->jobid); + *why = strdup(buf); + } + return 0; + } + /* Full match (main reason + host- and resource-based detail lists) + * is valid here, unlike --sim-verify's main-reason-only PEND check + * (bug-1845): this test registers no unsatisfiable_lookup_cb, so + * neither side has the unsatisfiable-allocation cache and both build + * the detail lists with the same full per-host computation. */ + return job_preason_eq(r->body.pend->reason, + s->body.pend->reason, why); default: + if (why != NULL) { + snprintf(buf, sizeof(buf), "body_which: unsupported %d", + (int)r->body_which); + *why = strdup(buf); + } return 0; } } @@ -1004,8 +1050,19 @@ test_normal_mode_recording_and_simulation_replay_produce_identical_decisions( TEST_ASSERT_EQUAL_INT((int)ReplayDecision_body_pend, (int)r_session->decisions[i]->body_which); - TEST_ASSERT_TRUE(replay_decision_content_eq(r_session->decisions[i], - s_session->decisions[i])); + { + char* why = NULL; + char msg[512]; + int eq = replay_decision_content_eq(r_session->decisions[i], + s_session->decisions[i], + &why); + + snprintf(msg, sizeof(msg), + "decision %d: recorded (R) vs simulated (S) differ: %s", + i, (why != NULL) ? why : "(no description)"); + free(why); + TEST_ASSERT_TRUE_MESSAGE(eq, msg); + } } free_ReplaySession_ptr(&r_session);