From d1849c94d8cb87189d28e785222292ea1db60c3c Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sun, 6 Sep 2026 07:49:38 +0000 Subject: [PATCH] test(resreq-maxtasks-wiring): retire multislot test's bug-1770 workaround test_parjob_e2e_multislot.c's standing "bug-1770" assertion now expects the correct, real primary->minTasks/maxTasks (1/JOB_MAXTASKS) instead of the buggy always-1 value it used to document. The PATCH_MAXTASKS_FOR_BUG_1770 block that manually overwrote the already-ingested resreq's task-count fields is removed entirely -- the full multi-host allocation scenario (2-host commit, per-host slots) now runs end to end through unmodified real ingest. Header comment updated to record that bug-1770 is fixed by this feature rather than describing a live gap. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01XYdtrEWWZfKRRGED7raEYg --- .kiro/specs/resreq-maxtasks-wiring/tasks.md | 2 +- .../testsuite/test_parjob_e2e_multislot.c | 107 +++++++----------- 2 files changed, 44 insertions(+), 65 deletions(-) diff --git a/.kiro/specs/resreq-maxtasks-wiring/tasks.md b/.kiro/specs/resreq-maxtasks-wiring/tasks.md index ba1ba753..4df5e39a 100644 --- a/.kiro/specs/resreq-maxtasks-wiring/tasks.md +++ b/.kiro/specs/resreq-maxtasks-wiring/tasks.md @@ -49,7 +49,7 @@ - _Boundary: test_maxtasks_wiring_ingest.c, test_maxtasks_wiring_sim_driver_parity.c (see Implementation Notes)_ - _Depends: 2.1, 2.2_ -- [ ] 3.3 (P) Retire the multi-host allocator characterization test's manual workaround +- [x] 3.3 (P) Retire the multi-host allocator characterization test's manual workaround - Replace the existing multi-host allocator test's manual task-count patch with real ingest of an equivalent multi-task job, exercising the same scenario end to end through the now-fixed wiring - Observable: the updated test passes while driving its scenario entirely through unmodified real ingest, with the workaround removed - _Requirements: 2.1, 2.2_ diff --git a/src/scheduler/testsuite/test_parjob_e2e_multislot.c b/src/scheduler/testsuite/test_parjob_e2e_multislot.c index 5c875af4..8e084f43 100644 --- a/src/scheduler/testsuite/test_parjob_e2e_multislot.c +++ b/src/scheduler/testsuite/test_parjob_e2e_multislot.c @@ -74,54 +74,41 @@ * cache-clear wiring (`_Depends: 5.1, 5.2, 4.3` on this task) as part of the * same real call sequence production makes every round. * - * ── bug-1770 (discovered by this file, NOT fixed here) ────────────────── + * ── bug-1770 (discovered by this file; FIXED by resreq-maxtasks-wiring) ── * Confirmed by direct code read (resreq.c's parse_resreq_primary(), * core_events.c's core_build_job()/core_build_primary_schedobj(), * schedobj.c's make_sched_job()) and by a genuine RED-phase run of this - * very file: the REAL job-ingest path can NEVER produce a primary resreq - * with maxTasks > 1. parse_resreq_primary() unconditionally hardcodes + * very file: the REAL job-ingest path could NEVER produce a primary resreq + * with maxTasks > 1. parse_resreq_primary() unconditionally hardcoded * `primary->minTasks = 1; primary->maxTasks = 1;` regardless of the * resreq string's own content, and neither core_build_job() nor - * make_sched_job() ever copies the job's own submitted task count + * make_sched_job() ever copied the job's own submitted task count * (job_submit_data_t.mintasks/maxtasks, correctly propagated onto the * TOP-LEVEL xce_sched_job_t.mintasks/maxtasks by both functions) onto the * NESTED xce_primary_resreq_t.minTasks/maxTasks that sched_make_primary_ * alloc_()'s maxTasks>1 dispatch check (scheduler.c:290) and parjob_ - * allocator.c's my_alloc() actually read. Net effect: parjob_allocator is - * UNREACHABLE from any real production job submission today, regardless of - * what task count a real job requests -- every job's primary resreq that - * parses successfully has maxTasks pinned to 1 by parse_resreq_primary(), - * so sched_make_primary_alloc_() always routes it to seqjob_allocator - * instead (scheduler.c:290-294). This is a genuine, real, pre-existing gap - * this end-to-end test is what surfaced it -- every prior unit test in this - * feature (test_parjob_combinatorial.c etc.) hand-constructs its - * xce_primary_resreq_t fixture directly, setting maxTasks by plain struct - * assignment, so none of them exercise the real ingest path closely enough - * to notice maxTasks never actually arrives that way. + * allocator.c's my_alloc() actually read. Net effect: parjob_allocator was + * UNREACHABLE from any real production job submission, regardless of what + * task count a real job requested -- every job's primary resreq that + * parsed successfully had maxTasks pinned to 1 by parse_resreq_primary(), + * so sched_make_primary_alloc_() always routed it to seqjob_allocator + * instead (scheduler.c:290-294). This end-to-end test is what surfaced the + * gap -- every prior unit test in this feature (test_parjob_combinatorial.c + * etc.) hand-constructs its xce_primary_resreq_t fixture directly, setting + * maxTasks by plain struct assignment, so none of them exercised the real + * ingest path closely enough to notice maxTasks never actually arrived + * that way. Logged as bug-1770 (.wolf/buglog.json). * - * Deliberately NOT fixed as part of this task: wiring job_submit_data_t's - * task count into xce_primary_resreq_t requires a real design decision - * (parse_resreq_primary()'s signature would need the job's task count - * threaded in from BOTH of its two call sites, plus a decision about how a - * COMPOUND resreq's several nested primaries should each receive a single - * job-level task count) well outside this spec's Boundary Context (the - * iterator/allocator rework, not resreq-parsing/job-submission plumbing) -- - * exactly the same class of pre-existing, out-of-scope gap tasks 5.1/5.2 - * already found and deferred in this same feature (bug-1766, bug-1768, - * bug-1769). Logged as bug-1770 (.wolf/buglog.json) for dedicated - * follow-up. - * - * This file's fixture compensates with ONE minimal, explicitly-labeled - * patch of the real, already-ingested xce_primary_resreq_t's task-count - * fields (see PATCH_MAXTASKS_FOR_BUG_1770 below) immediately after the real - * JOBS_PUSH ingest completes and before the round is driven -- everything - * else (cache lookup, parjob dispatch, plan iteration, host claiming, - * multi-host commit, decision production) runs through the unmodified real - * dispatch path. Removing that patch reproduces the RED-phase failure this - * file's own task-5.3 implementation captured (see the implementer's status - * report for the exact command/output): the job would route to seqjob_ - * allocator instead of parjob_allocator, and decision->u.alloc->nhosts - * would be 1, not 2. + * Update: fixed for real by the resreq-maxtasks-wiring spec (see + * .kiro/specs/resreq-maxtasks-wiring/), which threads + * job_submit_data_t.mintasks/maxtasks through apply_job_task_count() + * (resreq.c) into the standalone primary's minTasks/maxTasks at both real + * ingest call sites (core_events.c's core_build_job() and + * snapshot_load.c's restore path). This file's fixture no longer needs any + * compensating patch: the real, already-ingested xce_primary_resreq_t's + * task-count fields now arrive correctly from unmodified real JOBS_PUSH + * ingest alone, and the standing assertion below has been flipped to + * expect that correct value instead of documenting the old gap. * * ── Fixture: forcing a genuine multi-host split ───────────────────────── * Two real workers, each with exactly 2.0 free "slots" (WORKER_SLOTS), @@ -487,35 +474,27 @@ test_multihost_parallel_job_allocation_spans_two_hosts_via_real_dispatch(void) * carries the real submitted task count through ingest... */ TEST_ASSERT_EQUAL_UINT32(JOB_MAXTASKS, sjob->maxtasks); - /* ...but the NESTED xce_primary_resreq_t.maxTasks the allocator - * dispatch actually reads does not -- bug-1770 (see file header). This - * assertion is the direct, standing proof of that gap: it will keep - * passing (documenting the bug) until bug-1770 is fixed for real. */ + /* ...and the NESTED xce_primary_resreq_t.minTasks/maxTasks the + * allocator dispatch actually reads now correctly carries the job's + * real submitted task count too -- resreq-maxtasks-wiring's fix + * (apply_job_task_count(), called from core_build_job() before + * core_build_schedobj()/core_build_primary_schedobj()). This is the + * direct, standing proof that bug-1770 is fixed for real: no patch of + * any kind is applied to `primary` below -- these are the exact, + * unmodified values real JOBS_PUSH ingest produced. The job's fixture + * (make_push_job() above) submits mintasks=1, maxtasks=JOB_MAXTASKS. */ primary = sjob->schedobj->u.primary->resreq; TEST_ASSERT_NOT_NULL(primary); TEST_ASSERT_EQUAL_INT_MESSAGE( - 1, primary->maxTasks, - "bug-1770: the real ingest path currently hardcodes primary->" - "maxTasks=1 regardless of the job's own submitted maxtasks -- if " - "this assertion ever fails because maxTasks now correctly arrives " - "as JOB_MAXTASKS, bug-1770 has been fixed for real and the " - "PATCH_MAXTASKS_FOR_BUG_1770 workaround below should be deleted"); - - /* PATCH_MAXTASKS_FOR_BUG_1770: compensate for the confirmed, - * out-of-boundary gap above by patching the REAL, already-ingested - * primary resreq's task-count fields directly (see file header for - * the full rationale for not fixing this in production code as part - * of this task). Everything downstream of this point -- cache lookup, - * parjob dispatch, plan iteration, host claiming, multi-host commit, - * decision production -- runs through the unmodified real dispatch - * path. resreq_fp is kept consistent too (same fields - * core_build_primary_schedobj() itself derives from primary->min/ - * maxTasks at ingest time), though this single-dispatch test never - * exercises a second cache lookup that would depend on it. */ - primary->minTasks = 1; - primary->maxTasks = JOB_MAXTASKS; - sjob->schedobj->u.primary->resreq_fp.min_tasks = 1; - sjob->schedobj->u.primary->resreq_fp.max_tasks = JOB_MAXTASKS; + 1, primary->minTasks, + "resreq-maxtasks-wiring: primary->minTasks must arrive as the " + "job's real submitted mintasks (1) straight from real ingest, " + "with no test-side patch"); + TEST_ASSERT_EQUAL_INT_MESSAGE( + JOB_MAXTASKS, primary->maxTasks, + "resreq-maxtasks-wiring: primary->maxTasks must arrive as the " + "job's real submitted maxtasks (JOB_MAXTASKS) straight from real " + "ingest, with no test-side patch -- this is the fix for bug-1770"); /* ── Real per-session begin (task 5.2's cache-clear wiring) ──────── */ rc = xce_core_begin_session(&(sched->core));