10 KiB
Implementation Plan
TDD is mandatory (project rule): add the failing test/scenario first, then the change (red → green → refactor). Almost all work is in the single test client TU
src/controller/testsuite/test.c, so tasks are sequential by design; only the soak conversion touches a different file. Behavioral tests are driven throughxce_testitself (piped scripts), gating on its exit code.
-
1. Foundation: verdict, loop wrapper, ergonomics
-
1.1 Session failure tracking + process exit code + abort/continue + comment/blank handling
- Add file-scope state (
g_session_failed,g_continue_on_error,g_last_status) and apreprocess_and_run(line)wrapper used by the main read loop: ignore blank and#-comment lines; run normal lines viaexecute_line; updateg_last_status/g_session_failedfrom the result; in default mode stop on first failure, undercontinue-on-errorkeep going.mainreturnsg_session_failed ? 1 : 0. Extendcmd_setwith acontinue-on-errortoggle. - Observable: a piped script with a bogus/failing command makes
xce_testexit non-zero; an all-valid script exits 0; withset continue-on-errora failing line does not stop later lines but the final exit is still non-zero;#/blank lines are ignored. - Requirements: 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 6.1, 6.2
- Boundary: VerdictLoopWrapper
- Add file-scope state (
-
1.2
echocommand- Add
cmd_echo(table-registered) that prints its argument message and returns success. - Observable:
echo helloemitshelloand the command succeeds (exit unaffected). - Requirements: 6.3
- Boundary: VerdictLoopWrapper
- Depends: 1.1
- Add
-
2. Shell-escape
-
2.1
!<cmd>runs a shell command and feeds$?- In
preprocess_and_run, when the first non-whitespace char is!, run the remainder viasystem(); setg_last_status = WEXITSTATUS(rc)(non-zero onsystemfailure); a non-zero status is a command failure (subject to 1.1's abort/continue + exit code). - Observable:
!truesucceeds and sets$?=0;!falsefails the session and sets$?≠0; a script can!kill .../!bash start.sh ...to drive node lifecycle. - Requirements: 2.1, 2.2, 2.3, 2.4
- Boundary: ShellEscape
- Depends: 1.1
- In
-
3. Auto-variable substitution
-
3.1 Substitute the fixed auto-vars
$?,$jobid,$jobcount- Add
substitute_autovars(line)(run before tokenizing inpreprocess_and_run): replace whole-token$?→g_last_status,$jobid→g_last_jobid,$jobcount→g_last_jobcount. A reference to an unset$jobid/$jobcountfails the command (no garbage substitution). No other variable syntax is recognized. - Observable: after a command,
echo $?prints the prior status; referencing$jobidbefore any successfuljobsubfails that command rather than substituting an empty/garbage value. - Requirements: 3.1, 3.3, 3.5, 3.6
- Boundary: AutoVars
- Depends: 1.1
- Add
-
4. Correct job-submission reporting (bug-786) +
$jobidcapture -
4.1
cmd_jobsubreports success only on a clean ack with a valid JobId- Gate the
jobsubmit ok: JobId: <id>report on a successful reply status AND a well-formed JobId; on success setg_last_jobid(feeds$jobid, 3.2). Otherwise print a typed error (redirect / no-master / not-ready / timeout) and return<0. Never print "ok" with an uninitialized/garbage id (63-15--1-...). - Observable: a submit that does not get a clean ack returns failure and does NOT set
$jobid;jobsubnever prints success with a malformed id; a successful submit sets$jobidto the acked JobId. - Requirements: 7.1, 7.2, 7.3, 3.2
- Boundary: JobsubFix
- Depends: 1.1, 3.1
- Gate the
-
5. Job-count capture and assertions
-
5.1
cmd_joblistsets$jobcount; extract a shared count/contains helper- Refactor
cmd_joblistto expose an internal helper that returns the current job count and tests membership of a JobId; setg_last_jobcountafter ajoblist. - Observable: after
joblist,echo $jobcountprints the number of jobs returned; the helper is reusable by the assertions. - Requirements: 3.4
- Boundary: JoblistCount
- Depends: 1.1, 3.1
- Refactor
-
5.2
expect-jobs Nassertion- Add
cmd_expect_jobs: pass when the current job count equalsN; otherwise print expected vs actual and return<0. - Observable:
expect-jobs <correct>succeeds;expect-jobs <wrong>fails the session and prints expected/actual. - Requirements: 5.1, 5.2
- Boundary: Assertions
- Depends: 5.1
- Add
-
5.3
expect-job <jobid>assertion- Add
cmd_expect_job: pass when the JobId is present in the job list; otherwise print the missing id and return<0. Works with$jobidsubstitution. - Observable:
expect-job $jobidpasses for a submitted job;expect-job <absent>fails the session and prints the missing id. - Requirements: 5.3, 5.4
- Boundary: Assertions
- Depends: 5.1, 3.1, 4.1
- Add
-
6. Wait for elected primary
-
6.1
wait-primary [timeout_ms]- Add
cmd_wait_primary: loop a cheap, failover-aware client op (the existingjoblist/probe path viaactive_ctx) until it succeeds or the timeout elapses (monotonic clock); default timeout when the arg is omitted; return 0 on success,<0on timeout. - Observable: against a healthy cluster
wait-primaryreturns immediately; after a primary kill it blocks until a new primary serves, then succeeds; with no primary within the timeout it fails. - Requirements: 4.1, 4.2, 4.3, 4.4
- Boundary: WaitPrimary
- Depends: 1.1
- Add
-
7. Reference E2E driver
-
7.1 Convert
test-commit-durability.shto use the primitives- Replace the prose-grep + sanitizer-filter logic with piped
xce_testscripts usingwait-primary,jobsub/$jobid,expect-jobs/expect-job, and!-shell-escape for the primary kill/restart; gate each phase on thexce_testexit code. Preserve the rolling-kill + zero-loss coverage; add a!-driven cold-restart-all +expect-jobsstep. - Observable: the converted soak runs end-to-end and reports pass/fail via
xce_testexit codes (no joblist prose parsing); rolling-kill zero-loss coverage is retained; the cold-restart step asserts all committed jobs recover. - Requirements: 8.1, 8.2, 8.3
- Boundary: ReferenceDriver
- Depends: 1.1, 2.1, 4.1, 5.2, 5.3
- Replace the prose-grep + sanitizer-filter logic with piped
-
7.2 Run the converted soak on a live
/xce-deploy-hacluster- Execute the converted soak against a real 3-node cluster (real fsync-able fs) and confirm pass.
- Observable: soak passes all rounds with the exit-code verdict; manual environment prerequisite (live cluster), same status as other live-E2E tasks.
- Requirements: 8.2, 8.3
- Boundary: ReferenceDriver
- Depends: 7.1
Implementation Notes
- All of tasks 1–6 modify the single TU
src/controller/testsuite/test.c; sequential by design (no(P)). Task 7 touchessrc/tools/test-commit-durability.shbut depends on the primitives, so it is also ordered last. - Exit-code verdict is only trustworthy with a clean process exit; run the soak with
ASAN_OPTIONS=detect_leaks=0(existing convention). Client leak cleanup is out of scope (separate item). - Build dir is
src/xce.build(notsrc/build). Fast incremental build:cd src && nix develop --command bash -c "meson compile -C xce.build xce_test". Binary:src/xce.build/controller/xce_test. Behavioral tests = piped scripts gating on$?withASAN_OPTIONS=detect_leaks=0; no controller needed (an unknown command is a convenient failing command).xce_testis NOT a meson-registeredtest(), someson test --suite controllerdoes not exercise it. - Pre-existing unrelated failure:
meson test --suite controllerreportstest_demotion_replay_yields_committed_onlyFAIL (incontroller_unit_test/test_commit_barrier_reply.c) — from the in-flightha-commit-durabilityfeature, in a binary that does NOT compiletest.c. Not caused by this spec; ignore it for verdicts here. - 1.1:
preprocess_and_run()is the single chokepoint where the leading-!shell-escape (2.1) and$-autovar substitution (3.1) plug in. File-scopeg_session_failed/g_continue_on_error/g_last_statusadded;mainreturnsg_session_failed ? 1 : 0;set continue-on-erroris a process-global argc==2 special-case incmd_set(not a per-contextsettings[]entry). - Feature Flag Protocol: the flag must be fully REMOVED at the end (no pinned
#define X (1)guarding a dead#else). Reviewer rejects leftover flag scaffolding as dead code. (3.1 hit this — re-mediated.) - 3.1: introduced
g_last_jobid[XCE_MAXNAME_LEN]={0}(empty = unset) andg_last_jobcount=-1(-1 = unset) file-scope state, POPULATED later by 4.1 (jobsub) / 5.1 (joblist).substitute_autovars(line,out,sz)does whole-token replace of$?/$jobid/$jobcount; unset$jobid/$jobcount→ command fails (autovar: $X referenced before set), no garbage.!shell-escape lines are NOT substituted (raw to shell — the shell owns its$). Added#include "config.h"for XCE_MAXNAME_LEN. - 4.1 (bug-786 fixed):
cmd_jobsubinits localjobid=0, printsjobsubmit ok+ capturesg_last_jobidONLY after BOTH gates:rc>=0ANDjobid!=0; else typed error (E_XCE_NOMASTER/REDIRECT/NOT_READY/timeout switch viaxce_strerror_r) +return <0.$jobidcapture usesfmt_jobid(g_last_jobid, ..., jobid)— the FULL jobid, byte-identical to howcmd_joblistformatsjobs[i]->jobid(NOTarray_jobid(), which the display-onlyok:line uses). This is REQUIRED so task 5.3expect-job $jobidmatches the joblist column for array jobs. Also moved the error check AFTER the FREEUP block → fixed a pre-existing error-path leak. Live success-path ($jobid==joblist entry) verified by code reading; runtime-verified via soak (7.2). - 5.1: extracted DRY print-free helper
static int joblist_query(const char* find_jobid, int* contains, int print_table)— singlexcecli_joblistcall + internal free; returns count (>=0) or<0; membership viastrcmp(fmt_jobid(buf,...,jobs[i]->jobid), find_jobid)(SAME full-jobid form as 4.1's$jobid).cmd_joblistdelegatesjoblist_query(NULL,NULL,1)and setsg_last_jobcount=countONLY on success (no fake count on failure). 5.2/5.3 assertion commands calljoblist_querydirectly. Live$jobcount-after-success needs a controller (soak 7.2).