From 171c88fcec6ebf1296e00f23dfecfb706b81b8b4 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Fri, 19 Jun 2026 16:10:44 +0000 Subject: [PATCH] feat(xce-test-e2e-primitives): joblist_query helper + $jobcount capture (task 5.1) --- .kiro/specs/xce-test-e2e-primitives/tasks.md | 3 +- src/controller/testsuite/test.c | 77 ++++++++++++++++---- 2 files changed, 65 insertions(+), 15 deletions(-) diff --git a/.kiro/specs/xce-test-e2e-primitives/tasks.md b/.kiro/specs/xce-test-e2e-primitives/tasks.md index d605fa17..6bccea73 100644 --- a/.kiro/specs/xce-test-e2e-primitives/tasks.md +++ b/.kiro/specs/xce-test-e2e-primitives/tasks.md @@ -41,7 +41,7 @@ - _Depends: 1.1, 3.1_ - [ ] 5. Job-count capture and assertions -- [ ] 5.1 `cmd_joblist` sets `$jobcount`; extract a shared count/contains helper +- [x] 5.1 `cmd_joblist` sets `$jobcount`; extract a shared count/contains helper - Refactor `cmd_joblist` to expose an internal helper that returns the current job count and tests membership of a JobId; set `g_last_jobcount` after a `joblist`. - Observable: after `joblist`, `echo $jobcount` prints the number of jobs returned; the helper is reusable by the assertions. - _Requirements: 3.4_ @@ -91,3 +91,4 @@ - 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) and `g_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_jobsub` inits local `jobid=0`, prints `jobsubmit ok` + captures `g_last_jobid` ONLY after BOTH gates: `rc>=0` AND `jobid!=0`; else typed error (`E_XCE_NOMASTER/REDIRECT/NOT_READY`/timeout switch via `xce_strerror_r`) + `return <0`. **`$jobid` capture uses `fmt_jobid(g_last_jobid, ..., jobid)` — the FULL jobid, byte-identical to how `cmd_joblist` formats `jobs[i]->jobid`** (NOT `array_jobid()`, which the display-only `ok:` line uses). This is REQUIRED so task 5.3 `expect-job $jobid` matches 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)` — single `xcecli_joblist` call + internal free; returns count (>=0) or `<0`; membership via `strcmp(fmt_jobid(buf,...,jobs[i]->jobid), find_jobid)` (SAME full-jobid form as 4.1's `$jobid`). `cmd_joblist` delegates `joblist_query(NULL,NULL,1)` and sets `g_last_jobcount=count` ONLY on success (no fake count on failure). 5.2/5.3 assertion commands call `joblist_query` directly. Live `$jobcount`-after-success needs a controller (soak 7.2). diff --git a/src/controller/testsuite/test.c b/src/controller/testsuite/test.c index 51a1dd26..064502c9 100644 --- a/src/controller/testsuite/test.c +++ b/src/controller/testsuite/test.c @@ -139,11 +139,13 @@ static int g_last_status = 0; * * g_last_jobid most recently acknowledged JobId as a string; empty ("") * means unset. Populated later by cmd_jobsub (task 4.1). - * g_last_jobcount most recent job count; -1 means unset. Populated later by - * cmd_joblist (task 5.1). + * g_last_jobcount most recent job count; -1 means unset. Populated by + * cmd_joblist on a successful `joblist` (task 5.1, Req 3.4); + * a failed joblist leaves it untouched so no fake count is + * advertised. * - * Until those tasks land, both are always unset — which is exactly what makes - * an unset `$jobid`/`$jobcount` reference fail the command (Req 3.6). */ + * Until a command sets one, it stays unset — which is exactly what makes an + * unset `$jobid`/`$jobcount` reference fail the command (Req 3.6). */ static char g_last_jobid[XCE_MAXNAME_LEN] = { 0 }; static int g_last_jobcount = -1; @@ -877,11 +879,30 @@ static char* job_state_str(int state) return "UNKNOWN"; } -int cmd_joblist(int argc, char* argv[]) +/* Shared, print-free joblist core for `cmd_joblist` and the `expect-jobs` / + * `expect-job` assertions (xce-test-e2e-primitives task 5.1, design.md:81, + * 116-117). + * + * Performs the single `xcecli_joblist` call (through the failover-aware client + * context) and returns the current job count (>= 0), or a negative errno-style + * value on failure. When `find_jobid != NULL`, `*contains` is set to 1 if a job + * whose `fmt_jobid(...)` string equals `find_jobid` is present, else 0; the + * `fmt_jobid` representation matches the one `cmd_joblist` prints and the one + * `cmd_jobsub` captures into `$jobid` (task 4.1), so `expect-job $jobid` lines + * up. When `print_table` is set, the human-readable table is printed (the + * `cmd_joblist` path). The job array is always freed internally. + * + * On error this returns the negative rc WITHOUT setting any auto-variable, so a + * failed joblist never advertises a fake `$jobcount` (Req 3.6). */ +static int joblist_query(const char* find_jobid, int* contains, int print_table) { int rc, i; xce_job_t** jobs; + if (contains != NULL) { + *contains = 0; + } + rc = xcecli_joblist(active_ctx->ctx, &jobs); if (rc < 0) { @@ -890,21 +911,30 @@ int cmd_joblist(int argc, char* argv[]) return rc; } - printf("%-26s%-10s%-10s%-8s%-16s%-16s%-16s%10s%30s\n", "JobId", "User", - "Queue", "State", "SubmitTime", "StartTime", "FinishTime", "NTasks", - "Command"); + if (print_table) { + printf("%-26s%-10s%-10s%-8s%-16s%-16s%-16s%10s%30s\n", "JobId", "User", + "Queue", "State", "SubmitTime", "StartTime", "FinishTime", + "NTasks", "Command"); + } for (i = 0; i < rc; i++) { char buf[128]; fmt_jobid(buf, sizeof(buf) - 1, jobs[i]->jobid); - printf("%-26s%-10s%-10s%-8s%-16ld%-16ld%-16ld%4d,%-5d%30s\n", buf, - jobs[i]->submit->user, jobs[i]->submit->queue, - job_state_str(jobs[i]->state), jobs[i]->queue_time, - jobs[i]->start_time, jobs[i]->finish_time, - jobs[i]->submit->mintasks, jobs[i]->submit->maxtasks, - jobs[i]->submit->start_info->cmdline); + if ((find_jobid != NULL) && (contains != NULL) + && (strcmp(buf, find_jobid) == 0)) { + *contains = 1; + } + + if (print_table) { + printf("%-26s%-10s%-10s%-8s%-16ld%-16ld%-16ld%4d,%-5d%30s\n", buf, + jobs[i]->submit->user, jobs[i]->submit->queue, + job_state_str(jobs[i]->state), jobs[i]->queue_time, + jobs[i]->start_time, jobs[i]->finish_time, + jobs[i]->submit->mintasks, jobs[i]->submit->maxtasks, + jobs[i]->submit->start_info->cmdline); + } } for (i = 0; i < rc; i++) { @@ -913,6 +943,25 @@ int cmd_joblist(int argc, char* argv[]) free(jobs); + return rc; +} + +int cmd_joblist(int argc, char* argv[]) +{ + int count; + + count = joblist_query(NULL, NULL, 1); + + if (count < 0) { + return count; + } + + /* Capture $jobcount (Req 3.4): a successful joblist publishes the returned + * job count so a later `echo $jobcount` / `expect-jobs` substitutes it. On + * failure we returned above without touching g_last_jobcount, so a failed + * joblist never advertises a fake count (Req 3.6). */ + g_last_jobcount = count; + return 0; }