test(scheduler-dispatch-commit): terminal dispatch failure e2e coverage (task 5.3)
Adds test_handle_jobdispatch_terminal_no_notify_no_leak to test_jobdispatch_no_double_notify.c, closing a gap neither existing test fully covered: test_dispatch_fail_terminal.c (task 3.2) bypasses handle_jobdispatch() entirely via dispatch_a_job_test(), so it can't exercise the double-notify race at all; test_jobdispatch_no_double_notify.c (task 4.1) only proves an aggregate 3-job-batch notify count, not a direct per-job zero-notify proof. The new test drives a single terminal-classified job through the real handle_jobdispatch() entry point and asserts, in one run: JOB_STAT_EXIT with allocs left NULL, a direct (non-aggregate) zero-count read from the notify-capture spy proving no jobDispatchFail was produced for this job, and (via task 3.2's canary-reference technique, armed before the call under test) no reference over-release. Test-only; job.c/handler.c are untouched. RED phase verified via an actual revert-rebuild-rerun of handle_jobdispatch() to its pre-task-4.1 shape. Independently reviewed and approved -- the reviewer notably declined to trust a relayed build report and re-ran verification first-hand before approving. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cxa1PmqAB5PyXfJaNvj8TN
This commit is contained in:
parent
16d90319db
commit
bea956997a
2 changed files with 127 additions and 1 deletions
|
|
@ -83,7 +83,7 @@
|
|||
- _Requirements: 3.1, 3.2, 4.1, 4.3, 4.4, 4.5_
|
||||
- _Depends: 2.2, 3.3_
|
||||
|
||||
- [ ] 5.3 End-to-end test: terminal dispatch failure resolves without duplicate reporting or leaked references
|
||||
- [x] 5.3 End-to-end test: terminal dispatch failure resolves without duplicate reporting or leaked references
|
||||
- Simulate a locally-invalid dispatch decision and confirm the job resolves to permanently failed, that no retry-eligible report is also produced for the same failure, and that the job record's reference count returns to its pre-attempt value
|
||||
- Observable completion: the assertions above all pass in the same test run
|
||||
- _Requirements: 4.2, 4.5_
|
||||
|
|
@ -118,3 +118,4 @@
|
|||
- Task 5.1: no controller-side test was added for this task — Requirements 2.1/2.3 (controller transitions to RUN independent of confirmation) are already covered by task 3.1's `test_dc_success_commits_alloc_state_and_resets_counter`. No live scheduler+controller wire-protocol integration harness exists anywhere in this codebase; building one was judged disproportionate to what this task verifies, and the reviewer agreed.
|
||||
- Task 5.2: **bug-1631 (logged, NOT fixed, deliberately deferred)** — `dispatch_a_job_()`'s four retryable-failure branches (job.c, tasks 3.1/3.3) never revert `job.state`/`job.allocs` when a retryable failure occurs while the job's controller-side state is CURRENTLY RUN (they were only ever designed for a job whose every dispatch attempt starts from PEND, which is all that's reachable today). A kiro-debug investigation (HIGH confidence), independently re-verified even more exhaustively by this task's own reviewer (full trace of every `pendJobs`-reinsertion and every controller-side `job.state`-mutation call site), confirmed this scenario is genuinely UNREACHABLE via any real caller in the current, fully-implemented system: the scheduler's picker permanently removes a job from its PEND candidate pool the instant it dispatches it (task 2.1), and nothing already-implemented ever re-adds a successfully-dispatched (RUN) job back to that pool. The only mechanism design.md itself anticipates for that is `scheduler-preemption`, confirmed still unimplemented (`requirements-generated`, no approvals). **Revisit bug-1631 and add the state/allocs reversal to `dispatch_a_job_()`'s four retryable branches only once `scheduler-preemption` (or an equivalent mechanism) actually creates a real RUN→retryable-redispatch path** — not before, since there's no reachable caller to regression-test against today. `test_rpc_failure_bound_resets_after_intervening_success` (`test_dispatch_fail_retry.c`) proves Requirement 4.3's actual, literal ask (the counter genuinely resets on an intervening success) without asserting the unreachable state/allocs behavior.
|
||||
- Task 5.2: design.md's "vacuously by construction" reasoning (`Controller Dispatch Commit` and `Controller Dispatch-Failure Classification` sections — "on any of the six classified failure paths there is nothing to release") silently assumes every `dispatch_a_job_()` invocation starts from a job that's never had anything committed (i.e., a first-ever dispatch attempt) — true today (see bug-1631 above) but not stated as a scoped assumption in the doc text itself. Worth correcting design.md's wording if it's ever revisited for the `scheduler-preemption` follow-up.
|
||||
- Task 5.3: `test_dispatch_fail_terminal.c` (task 3.2) and `test_jobdispatch_no_double_notify.c` (task 4.1) each covered PART of this task but not the full combination in one run — 3.2's test bypasses `handle_jobdispatch()` entirely (via `dispatch_a_job_test()`), so it can't exercise the double-notify race at all; 4.1's test only proves an *aggregate* batch notify count, not a *direct* per-job zero-notify proof. `test_handle_jobdispatch_terminal_no_notify_no_leak` (extending 4.1's file, the one with the right entry point) closes this by combining EXIT resolution + a direct single-job zero-notify assertion + the canary-reference no-leak proof, all through the real `handle_jobdispatch()` entry point in one test. Pattern worth remembering: a batch-aggregate test and a bypassed-entry-point test can each look like they cover a scenario while neither actually does, in combination, what a later task's literal wording asks for ("all pass IN THE SAME TEST RUN").
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue