feat(xce-test-cli-ux): migrate all flag-accepting commands to flagspec

Migrates 17 commands (cmd_hello, cmd_jobinfo, cmd_jobrun, cmd_jobsub,
cmd_resval, cmd_jobkill, cmd_jobckpt, cmd_qctrl, cmd_hostctrl,
cmd_runtasks, cmd_setpend, cmd_jobstart, cmd_jobfinish, cmd_jobfail,
cmd_resupdate, cmd_walcompare, join_worker_parse_attrs) from raw
getopt()/hand-rolled parsing to the shared flagspec_t + getopt_long
mechanism, so every one now uniformly rejects an unrecognized or
malformed flag with a usage summary and, where a close match exists,
a suggestion. cmd_walcompare's hand-rolled parser needed a leading '+'
in its optstring to preserve its pre-migration flags-only-before-
positionals behavior. Also fixes bug-1704: cmd_qctrl's uninitialized
action variable, now sentineled and rejected when unset, mirroring
cmd_hostctrl's existing pattern -- this also makes the destructive-
command gate's -i/-c detection (task 3.3) safe to rely on. Every
existing short flag's letter, value format, and default is unchanged
for a valid invocation. 1357/1357 across the full controller suite
(+4), 0 regressions.

Task 3.1 of .kiro/specs/xce-test-cli-ux/tasks.md.
Requirements: 2.1, 2.2, 2.3, 2.4, 6.1, 6.2, 6.3, 6.4, 10.1, 10.2, 10.3

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
This commit is contained in:
Rongsong Shen 2026-09-09 17:27:48 +00:00
parent 878a42fb48
commit f152c59e75
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
6 changed files with 883 additions and 92 deletions

View file

@ -98,7 +98,7 @@
- _Boundary: Bug-1557 Fix_
- [ ] 3. Integration: migrate existing commands onto the new foundations
- [ ] 3.1 Migrate every flag-accepting command's parsing to the shared validation library
- [x] 3.1 Migrate every flag-accepting command's parsing to the shared validation library
- Convert every command currently parsing flags via the older mechanisms (including the one command using a fully hand-rolled long-flag parser) to the shared descriptor-table-driven parsing built in 1.1/1.2, so every one uniformly rejects an unrecognized or malformed flag and gains a long-form equivalent for each concept
- While migrating the queue-control command, initialize its action selection to an explicit unset sentinel and reject the invocation if none of its mutually-exclusive action flags was given, matching its sibling host-control command's existing pattern
- Observable completion: every migrated command rejects a deliberately invalid flag with a usage summary and, where applicable, a suggestion, and the queue-control command rejects an invocation given none of its action flags instead of transmitting an indeterminate value
@ -178,6 +178,7 @@
## Implementation Notes
- `src/tools/xce-build.sh controller asan xce.build --incremental` writes its build dir at `src/xce.build/` (relative to the script's own location), not a repo-root `xce.build/` — meson logs are at `src/xce.build/meson-logs/testlog.txt`. (task 1.1)
- New Unity test files need two wiring points, not just meson.build's source list: also register an `extern void test_<module>(void);` + `TEST(test_<module>);` in `src/controller/testsuite/controller_test.c`, mirroring the existing `test_rcfile` registration, or the suite silently never runs. (task 1.1)
- All 17 flag-accepting commands migrated to `flagspec_t`+`getopt_long` (review verified essentially every command line-by-line via git diff context, not just a sample — zero regressions to any existing short flag's letter/value-format/default). `cmd_walcompare`'s migration needs a leading `+` in its optstring (`getopt_long`'s "stop at first non-option") to preserve its pre-migration "flags only before positionals" behavior — without it, argv permuting would change which tokens are treated as flags vs. member paths. Only 7 of the 17 migrated commands got dedicated black-box tests (`qctrl`, `jobinfo`, `jobkill`, `hostctrl`, `setpend`, `resupdate`, `walcompare`); the other 10 (`hello`, `jobrun`, `jobsub`, `resval`, `jobckpt`, `runtasks`, `jobstart`, `jobfinish`, `jobfail`, `join_worker_parse_attrs`) have no valid-flag regression test at all beyond this review's manual line-by-line diff check — worth a follow-up smoke test per command if this area gets touched again. (task 3.1)
- `cmd_join_worker`'s sibling fields `maxswap`/`maxtmp` have the exact same stale-MB-scale bug as bug-1557's `maxmem` (1024/2048 bytes instead of KB/MB-appropriate values) but were deliberately left unfixed here: `USAGE_JOIN_WORKER` currently documents these exact stale values accurately, so fixing only the code would create a new code/docs mismatch outside this task's mandate. Worth a coordinated follow-up bug fixing both fields' defaults and their help text together. A new `test_join_worker_sibling_defaults_unchanged` test locks in today's values so that follow-up isn't confused with this task's maxmem-specific assertions. (task 2.6)
- When a fix touches a pure function stuck inside `test.c` (no networking/REPL/global-state dependency), extract it into its own module with a standalone meson `test()` target rather than settling for a throwaway/manual verification — this codebase already has two precedents for exactly this (`resval_typeresolve.c`, `misc.c`'s runusage helpers; now also `joblist_display.c` for `worker_status_str`/`job_state_str`/`format_job_time`). `test.c` itself is never linked into `controller_unit_test`, so this extract-to-standalone-module pattern is the only way to get permanent automated coverage for such functions — a scratch/deleted verification harness is not sufficient and will be sent back on review. `cmd_reslist`/`joblist_query`/`cmd_getconf`'s table-printing bodies remain genuinely untestable without a live cluster (fetch+print are inlined together, confirmed on review) and stay uncovered by permanent tests for now. (task 2.5)
- `cmd_source` translates `rcfile_load`'s raw positive "aborted" return to `-1` rather than forwarding it verbatim, because `preprocess_and_run` only treats a *negative* return as failure (`if (rc < 0)`) — forwarding the raw positive value would let an enclosing `source` line silently read a transitively-aborted nested source as success. No test yet exercises this specific nested-abort-propagation case (all 3 required scenarios pass either way) — worth a dedicated test in Validation phase (4.3 or nearby). (task 2.4)

View file

@ -19290,7 +19290,7 @@
"error_message": "cmd_qctrl declares `int action;` with no initializer and only assigns it inside the -a/-i/-o/-c getopt cases; there is no post-loop check that one of them was given before `queuectrl.action = action` is sent over the wire. Running `qctrl -q normal` with no action flag (syntactically valid per USAGE_QCTRL, which lists all four as optional) transmits indeterminate stack memory as the queue action.",
"file": "src/controller/testsuite/test.c:2663 (declaration), :2668-2688 (getopt loop), :2695 (queuectrl.action = action)",
"root_cause": "Missing initializer + missing post-loop validation, unlike the very next sibling command cmd_hostctrl which initializes action=HOST_DO_NONE and explicitly rejects that sentinel.",
"fix": "NOT YET FIXED. Recommended: initialize action to a sentinel (e.g. -1) and reject with a clear error if unset after the getopt loop, mirroring cmd_hostctrl's existing pattern.",
"fix": "FIXED (2026-09-09, xce-test-cli-ux task 3.1, flagspec migration). cmd_qctrl now initializes action = QUEUE_DO_NONE (xce/types.h pre-existing sentinel, same enum family as HOST_DO_NONE) before the getopt_long() loop, and rejects the invocation post-loop if action is still QUEUE_DO_NONE after parsing -- mirroring cmd_hostctrl's exact HOST_DO_NONE pattern. Landed as part of migrating cmd_qctrl (and 16 other flag-accepting commands) from getopt() to the shared flagspec_t + getopt_long() mechanism (flagspec.c/.h, tasks 1.1/1.2). Regression-locked by test_qctrl_missing_action_flag_rejected and test_qctrl_flag_forms_short_and_long_both_accepted in src/controller/testsuite/test_test_cli.c.",
"tags": [
"controller",
"testsuite",
@ -19299,11 +19299,11 @@
"uninitialized-variable",
"undefined-behavior",
"cli-ux-audit",
"unfixed"
"fixed"
],
"related_bugs": [],
"occurrences": 1,
"last_seen": "2026-08-26T09:32:18Z"
"last_seen": "2026-09-09T00:00:00Z"
},
{
"id": "bug-1705",

View file

@ -435,6 +435,8 @@ test('joblist display test', joblist_display_unit_test,
join_worker_args_unit_test_src = [
'testsuite/test_join_worker_args.c',
'testsuite/join_worker_args.c',
'testsuite/flagspec.c', # xce-test-cli-ux task 3.1: join_worker_parse_attrs
# migrated onto flagspec_build_opts/flagspec_reject
]
join_worker_args_unit_test = executable('join_worker_args_unit_test',

View file

@ -9,16 +9,47 @@
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h> /* getopt, optarg, optind */
#include <unistd.h> /* getopt_long, optarg, optind */
#include "flagspec.h" /* shared flagspec_t descriptor-table flag parsing +
* uniform rejection (xce-test-cli-ux tasks 1.1/1.2),
* migrated onto by task 3.1 */
/* Every flag's existing short letter, accepted value, and default are
* unchanged by this migration (Requirement 6.3) -- only a long-form
* equivalent and uniform rejection of an unrecognized/malformed flag are
* added. Long names reuse the same concept spellings used elsewhere in
* xce_test for the identical real-world concepts (e.g. "host"/"jobid" in
* test.c's own migrated commands); the ones here (cpufactor, maxcpus,
* maxmem, maxswap, maxtmp, ndisks) are unique to `join` itself. */
static const flagspec_t join_flags[] = {
{ 'f', "cpufactor", "cpufactor", required_argument, 0, "1.0" },
{ 'c', "maxcpus", "maxcpus", required_argument, 0, "4" },
{ 'm', "maxmem", "maxmem", required_argument, 0,
"64 * 1024 * 1024 (64 MB in bytes)" },
{ 's', "maxswap", "maxswap", required_argument, 0, "1024" },
{ 't', "maxtmp", "maxtmp", required_argument, 0, "2048" },
{ 'd', "ndisks", "ndisks", required_argument, 0, "1" },
};
#define JOIN_NFLAGS (sizeof(join_flags) / sizeof(join_flags[0]))
int join_worker_parse_attrs(int argc, char* argv[], hostattrs_t* attrs)
{
int opt;
int opt;
struct option long_opts[JOIN_NFLAGS + 1];
char optstring[16];
if (attrs == NULL) {
return -EINVAL;
}
if (flagspec_build_opts(join_flags, JOIN_NFLAGS, long_opts, optstring,
sizeof(optstring))
< 0) {
fprintf(stderr, "xce_test: join: internal error building flag table\n");
return -EINVAL;
}
/* bug-1557: maxmem is bytes-canonical (see resources.yaml, hardware_
* monitor.c's total_memory) -- this default used to read plain "64"
* back when maxmem was MB-scale (64 MB); an earlier session converted
@ -38,8 +69,10 @@ int join_worker_parse_attrs(int argc, char* argv[], hostattrs_t* attrs)
attrs->hosttype = "LINUX64";
optind = 1;
opterr = 0;
while ((opt = getopt(argc, argv, "f:c:m:s:t:d:")) != -1) {
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case 'f':
attrs->cpufactor = atof(optarg);
@ -59,10 +92,11 @@ int join_worker_parse_attrs(int argc, char* argv[], hostattrs_t* attrs)
case 'd':
attrs->ndisks = atoi(optarg);
break;
default:
printf(" join [-f cpufactor] [-c maxcpus] [-m maxmem] [-s maxswap] "
"[-t maxtmp] [-d ndisks]\n");
return -EINVAL;
case '?':
return flagspec_reject(
"join", join_flags, JOIN_NFLAGS, argv[optind - 1],
"join [-f <cpufactor>] [-c <maxcpus>] [-m <maxmem>] "
"[-s <maxswap>] [-t <maxtmp>] [-d <ndisks>]");
}
}

View file

@ -25,6 +25,11 @@
#include "controller.capnp.h"
#include "ctrlstatus_fmt.h" /* ctrlstatus_format_time_ms (task 6.3) */
#include "delay.h"
#include "flagspec.h" /* shared flagspec_t descriptor-table flag parsing,
* rejection (with typo suggestion), and help
* rendering (xce-test-cli-ux tasks 1.1/1.2); every
* flag-accepting command below migrates onto this
* (task 3.1) */
#include "idgen.h"
#include "preason.h"
#include "walcompare.h" /* cross-member committed-prefix divergence (cmd_walcompare) */
@ -1311,39 +1316,77 @@ int cmd_waldump(int argc, char* argv[])
* committed-vs-uncommitted classification for each divergence (task 1.1). Exit
* 0 = identical, nonzero = divergence or load error — so it doubles as a
* scripted assertion. */
/* Both flags are long-only (no short letter ever existed for either --
* cmd_walcompare was the one command using a fully hand-rolled long-flag
* parser, per xce-test-cli-ux task 3.1's explicit scope). Their synthetic
* getopt_long() `val`s are FLAGSPEC_LONG_ONLY_BASE + <index in this table>
* (flagspec.h) -- the enum below names those indices so the switch below
* doesn't rely on bare magic numbers. */
enum { WALCOMPARE_OPT_REPORT = 0, WALCOMPARE_OPT_COMMITTED = 1 };
static const flagspec_t walcompare_flags[] = {
{ 0, "report", "report", no_argument, 0, NULL },
{ 0, "committed", "committed", required_argument, 0,
"derive the shared boundary from members' CRC-intact tips" },
};
#define WALCOMPARE_NFLAGS \
(sizeof(walcompare_flags) / sizeof(walcompare_flags[0]))
int cmd_walcompare(int argc, char* argv[])
{
uint64_t committed = 0; /* 0 = derive the shared boundary */
int report = 0; /* --report => full per-member machine table */
int ai = 1;
uint64_t committed = 0; /* 0 = derive the shared boundary */
int report = 0; /* --report => full per-member machine table */
int opt;
struct option long_opts[WALCOMPARE_NFLAGS + 1];
char optstring[8];
char posix_optstring[sizeof(optstring) + 1];
/* Optional flags (any order): --report, --committed N. */
for (;;) {
if (argc > ai && strcmp(argv[ai], "--report") == 0) {
optind = 1;
opterr = 0;
if (flagspec_build_opts(walcompare_flags, WALCOMPARE_NFLAGS, long_opts,
optstring, sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: walcompare: internal error building flag table\n");
return -EINVAL;
}
/* Leading '+' (GNU getopt extension): stop at the first non-option
* argument instead of permuting argv -- matches the pre-migration
* hand-rolled parser's exact behavior (flags recognized only before the
* positional member paths; both flags here are long-only, so the built
* optstring is empty and this is simply "+"). */
posix_optstring[0] = '+';
strcpy(posix_optstring + 1, optstring);
while ((opt = getopt_long(argc, argv, posix_optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case FLAGSPEC_LONG_ONLY_BASE + WALCOMPARE_OPT_REPORT:
report = 1;
ai += 1;
continue;
}
if (argc > ai && strcmp(argv[ai], "--committed") == 0) {
if (argc <= ai + 1) {
fprintf(stderr, "walcompare: --committed needs an LSN\n");
return -1;
}
break;
case FLAGSPEC_LONG_ONLY_BASE + WALCOMPARE_OPT_COMMITTED: {
char* end = NULL;
unsigned long v = strtoul(argv[ai + 1], &end, 10);
if (end == argv[ai + 1] || *end != '\0') {
unsigned long v = strtoul(optarg, &end, 10);
if (end == optarg || *end != '\0') {
fprintf(stderr, "walcompare: bad --committed LSN '%s'\n",
argv[ai + 1]);
optarg);
return -1;
}
committed = (uint64_t)v;
ai += 2;
continue;
break;
}
case '?':
return flagspec_reject(
"walcompare", walcompare_flags, WALCOMPARE_NFLAGS,
argv[optind - 1],
"walcompare [--report] [--committed <N>] <memberA> "
"<memberB> [more...]");
}
break;
}
int n = argc - ai;
int n = argc - optind;
if (n < 2) {
fprintf(stderr,
"usage: walcompare [--report] [--committed N] <memberA> "
@ -1357,7 +1400,7 @@ int cmd_walcompare(int argc, char* argv[])
walcompare_member_t members[16];
for (int i = 0; i < n; i++) {
members[i].path = argv[ai + i];
members[i].path = argv[optind + i];
members[i].committed_lsn = committed; /* 0 => derive shared boundary */
}
@ -1661,16 +1704,33 @@ int cmd_quit(int argc, char* argv[])
return 0;
}
static const flagspec_t hello_flags[] = {
{ 'c', "count", "count", required_argument, 0, "1" },
{ 'm', "message", "message", required_argument, 0,
"\"Greeting from xce client\"" },
};
#define HELLO_NFLAGS (sizeof(hello_flags) / sizeof(hello_flags[0]))
int cmd_hello(int argc, char* argv[])
{
int rc = 0;
int opt;
char* msg = "Greeting from xce client";
int count = 1;
int rc = 0;
int opt;
char* msg = "Greeting from xce client";
int count = 1;
struct option long_opts[HELLO_NFLAGS + 1];
char optstring[32];
optind = 1;
opterr = 0;
while ((opt = getopt(argc, argv, "c:m:")) != -1) {
if (flagspec_build_opts(hello_flags, HELLO_NFLAGS, long_opts, optstring,
sizeof(optstring))
< 0) {
fprintf(stderr, "xce_test: hello: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL)) != -1) {
switch (opt) {
case 'c':
count = atoi(optarg);
@ -1678,13 +1738,10 @@ int cmd_hello(int argc, char* argv[])
case 'm':
msg = optarg;
break;
default:
/* -w was never implemented (getopt string above is "c:m:" only)
* -- this hint previously advertised a nonexistent -w wait_usec
* flag; corrected to match the actual accepted options. */
fprintf(stderr, " hello [-c count] [-m message]\n");
return -1;
break;
case '?':
return flagspec_reject("hello", hello_flags, HELLO_NFLAGS,
argv[optind - 1],
"hello [-c <count>] [-m <message>]");
}
}
@ -2335,13 +2392,30 @@ static void print_jobinfo(xce_job_t* job)
}
}
static const flagspec_t jobinfo_flags[] = {
{ 'j', "jobid", "jobid", required_argument, 1, NULL },
};
#define JOBINFO_NFLAGS (sizeof(jobinfo_flags) / sizeof(jobinfo_flags[0]))
int cmd_jobinfo(int argc, char* argv[])
{
int opt;
int opt;
struct option long_opts[JOBINFO_NFLAGS + 1];
char optstring[16];
optind = 1;
opterr = 0;
while ((opt = getopt(argc, argv, "j:")) != -1) {
if (flagspec_build_opts(jobinfo_flags, JOBINFO_NFLAGS, long_opts,
optstring, sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: jobinfo: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
uint64_t jobid;
xce_job_t* job;
int rc = 0;
@ -2362,6 +2436,10 @@ int cmd_jobinfo(int argc, char* argv[])
}
break;
case '?':
return flagspec_reject("jobinfo", jobinfo_flags, JOBINFO_NFLAGS,
argv[optind - 1],
"jobinfo -j <jobid> [-j <jobid> ...]");
}
}
return 0;
@ -2409,6 +2487,12 @@ static int fill_job_alloc(xce_job_alloc_t* jalloc, int nhosts,
return 0;
}
static const flagspec_t jobrun_flags[] = {
{ 'm', "host", "host", required_argument, 0, "1 task; repeatable up to 5" },
{ 'j', "jobid", "jobid", required_argument, 1, NULL },
};
#define JOBRUN_NFLAGS (sizeof(jobrun_flags) / sizeof(jobrun_flags[0]))
int cmd_jobrun(int argc, char* argv[])
{
int rc;
@ -2418,13 +2502,25 @@ int cmd_jobrun(int argc, char* argv[])
xce_job_alloc_t jalloc;
xce_job_dispatch_t jdispatch
= { .njobs = 1, .allocs = (xce_job_alloc_t*[]) { &jalloc } };
int nhosts = 0;
char* host;
int count;
int nhosts = 0;
char* host;
int count;
struct option long_opts[JOBRUN_NFLAGS + 1];
char optstring[16];
optind = 1;
opterr = 0;
while ((opt = getopt(argc, argv, "m:j:")) != -1) {
if (flagspec_build_opts(jobrun_flags, JOBRUN_NFLAGS, long_opts, optstring,
sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: jobrun: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case 'm':
if (nhosts >= MAX_TASK_HOSTS) {
@ -2460,6 +2556,10 @@ int cmd_jobrun(int argc, char* argv[])
return -1;
}
break;
case '?':
return flagspec_reject(
"jobrun", jobrun_flags, JOBRUN_NFLAGS, argv[optind - 1],
"jobrun -j <jobid> [-m <host>[:<ntasks>]] [-m ...]");
}
}
@ -2510,6 +2610,18 @@ static int build_job_exdata(xce_job_submit_t* submit)
return 0;
}
static const flagspec_t jobsub_flags[] = {
{ 'u', "user", "user", required_argument, 1, NULL },
{ 'q', "queue", "queue", required_argument, 1, NULL },
{ 'R', "resreq", "resreq", required_argument, 0,
"\"type == any\"" },
{ 'c', "command-line", "command-line", required_argument, 1, NULL },
{ 'n', "ntasks", "ntasks", required_argument, 0, "1" },
{ 'x', "exdata", "exdata", required_argument, 0, NULL },
{ 'a', "array-size", "arraysize", required_argument, 0, "1" },
};
#define JOBSUB_NFLAGS (sizeof(jobsub_flags) / sizeof(jobsub_flags[0]))
int cmd_jobsub(int argc, char* argv[])
{
int rc = 0;
@ -2524,8 +2636,11 @@ int cmd_jobsub(int argc, char* argv[])
uint32_t min = 1, max = 1;
uint32_t arsize = 1;
char jobidStr[128];
struct option long_opts[JOBSUB_NFLAGS + 1];
char optstring[32];
optind = 1;
opterr = 0;
memset(&submit, 0x0, sizeof(submit));
submit.start_info
@ -2535,7 +2650,16 @@ int cmd_jobsub(int argc, char* argv[])
return -ENOMEM;
}
while ((opt = getopt(argc, argv, "u:q:R:c:x:n:a:")) != -1) {
if (flagspec_build_opts(jobsub_flags, JOBSUB_NFLAGS, long_opts, optstring,
sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: jobsub: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case 'u':
submit.user = strdup(optarg);
@ -2578,10 +2702,11 @@ int cmd_jobsub(int argc, char* argv[])
arsize = 1;
}
break;
default:
printf(
"jobsub -u <user> -q <queue> -R <resreq> -c <command line>\n");
rc = -EINVAL;
case '?':
rc = flagspec_reject(
"jobsub", jobsub_flags, JOBSUB_NFLAGS, argv[optind - 1],
"jobsub -u <user> -q <queue> -c <cmdline> [-R <resreq>] "
"[-n <min>[,<max>]] [-x <exdata>] [-a <arraysize>]");
goto cleanup;
}
}
@ -2790,6 +2915,13 @@ static void format_bytes_human(double bytes, char* buf, size_t bufsize)
snprintf(buf, bufsize, "%.2f %s", val, UNITS[i]);
}
static const flagspec_t resval_flags[] = {
{ 'n', "resname", "resname", required_argument, 1, NULL },
{ 'm', "host", "host", required_argument, 0,
"required unless the resource is cluster-wide" },
};
#define RESVAL_NFLAGS (sizeof(resval_flags) / sizeof(resval_flags[0]))
int cmd_resval(int argc, char* argv[])
{
char* host = NULL;
@ -2797,10 +2929,22 @@ int cmd_resval(int argc, char* argv[])
xce_resval_t** vals = NULL;
int opt;
int rc;
struct option long_opts[RESVAL_NFLAGS + 1];
char optstring[16];
optind = 1;
opterr = 0;
while ((opt = getopt(argc, argv, "n:m:")) != -1) {
if (flagspec_build_opts(resval_flags, RESVAL_NFLAGS, long_opts, optstring,
sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: resval: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case 'n':
res = strdup(optarg);
@ -2808,11 +2952,13 @@ int cmd_resval(int argc, char* argv[])
case 'm':
host = strdup(optarg);
break;
default:
fprintf(stderr, "resval -n resname [-m host]\n");
case '?':
rc = flagspec_reject("resval", resval_flags, RESVAL_NFLAGS,
argv[optind - 1],
"resval -n <resname> [-m <host>]");
FREEUP(host);
FREEUP(res);
return -1;
return rc;
}
}
@ -2863,6 +3009,12 @@ int cmd_resval(int argc, char* argv[])
return 0;
}
static const flagspec_t jobkill_flags[] = {
{ 's', "signal", "signal", required_argument, 0, "SIGTERM" },
{ 'j', "jobid", "jobid", required_argument, 1, NULL },
};
#define JOBKILL_NFLAGS (sizeof(jobkill_flags) / sizeof(jobkill_flags[0]))
int cmd_jobkill(int argc, char* argv[])
{
int signo;
@ -2872,11 +3024,23 @@ int cmd_jobkill(int argc, char* argv[])
xce_jobctrl_t jobctrl;
xce_signal_t siginfo;
char jobidStr[64];
struct option long_opts[JOBKILL_NFLAGS + 1];
char optstring[16];
optind = 1;
opterr = 0;
signo = SIGTERM;
while ((opt = getopt(argc, argv, "s:j:")) != -1) {
if (flagspec_build_opts(jobkill_flags, JOBKILL_NFLAGS, long_opts,
optstring, sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: jobkill: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case 's':
signo = atoi(optarg);
@ -2884,8 +3048,10 @@ int cmd_jobkill(int argc, char* argv[])
case 'j':
parse_jobid(&jobid, optarg);
break;
default:
break;
case '?':
return flagspec_reject("jobkill", jobkill_flags, JOBKILL_NFLAGS,
argv[optind - 1],
"jobkill -j <jobid> [-s <signo>]");
}
}
@ -2912,6 +3078,16 @@ int cmd_jobkill(int argc, char* argv[])
return 0;
}
static const flagspec_t jobckpt_flags[] = {
{ 'k', "kill", "kill", no_argument, 0,
NULL },
{ 'j', "jobid", "jobid", required_argument, 1,
NULL },
{ 'm', "checkpoint-method", "checkpoint-method", required_argument, 0,
"\"default\"" },
};
#define JOBCKPT_NFLAGS (sizeof(jobckpt_flags) / sizeof(jobckpt_flags[0]))
int cmd_jobckpt(int argc, char* argv[])
{
uint64_t jobid = 0;
@ -2920,11 +3096,23 @@ int cmd_jobckpt(int argc, char* argv[])
xce_jobctrl_t jobctrl;
xce_ckpt_options_t ckptoption;
char jobidStr[64];
struct option long_opts[JOBCKPT_NFLAGS + 1];
char optstring[16];
optind = 1;
opterr = 0;
ckptoption.kill = 0;
while ((opt = getopt(argc, argv, "kj:m:")) != -1) {
if (flagspec_build_opts(jobckpt_flags, JOBCKPT_NFLAGS, long_opts,
optstring, sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: jobckpt: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case 'k':
ckptoption.kill = 1;
@ -2935,8 +3123,10 @@ int cmd_jobckpt(int argc, char* argv[])
case 'm':
ckptoption.method = optarg;
break;
default:
break;
case '?':
return flagspec_reject("jobckpt", jobckpt_flags, JOBCKPT_NFLAGS,
argv[optind - 1],
"jobckpt -j <jobid> [-k] [-m <method>]");
}
}
@ -2964,17 +3154,44 @@ int cmd_jobckpt(int argc, char* argv[])
return 0;
}
static const flagspec_t qctrl_flags[] = {
{ 'a', "activate", "activate", no_argument, 0, NULL },
{ 'i', "deactivate", "deactivate", no_argument, 0, NULL },
{ 'o', "open", "open", no_argument, 0, NULL },
{ 'c', "close", "close", no_argument, 0, NULL },
{ 'q', "queue", "queue", required_argument, 1, NULL },
};
#define QCTRL_NFLAGS (sizeof(qctrl_flags) / sizeof(qctrl_flags[0]))
int cmd_qctrl(int argc, char* argv[])
{
int opt;
int rc;
xce_queuectrl_t queuectrl;
int action;
/* bug-1704 fix: initialize to the QUEUE_DO_NONE sentinel (xce/types.h)
* and reject post-loop if it is still unset -- mirrors cmd_hostctrl's
* existing HOST_DO_NONE sentinel-and-reject pattern immediately below.
* Before this fix, `action` had no initializer and no post-loop check,
* so an invocation giving -q but none of -a/-i/-o/-c transmitted
* indeterminate stack memory as the queue action. */
int action = QUEUE_DO_NONE;
char* queue = NULL;
struct option long_opts[QCTRL_NFLAGS + 1];
char optstring[16];
optind = 1;
opterr = 0;
while ((opt = getopt(argc, argv, "aiociq:")) != -1) {
if (flagspec_build_opts(qctrl_flags, QCTRL_NFLAGS, long_opts, optstring,
sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: qctrl: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case 'a':
action = QUEUE_ACTIVE;
@ -2991,8 +3208,11 @@ int cmd_qctrl(int argc, char* argv[])
case 'q':
queue = strdup(optarg);
break;
default:
break;
case '?':
FREEUP(queue);
return flagspec_reject("qctrl", qctrl_flags, QCTRL_NFLAGS,
argv[optind - 1],
"qctrl -q <queue> [-a] [-i] [-o] [-c]");
}
}
@ -3001,6 +3221,16 @@ int cmd_qctrl(int argc, char* argv[])
return -1;
}
/* bug-1704 fix (continued): reject rather than send an indeterminate
* action when none of -a/-i/-o/-c (nor their long forms) was given. */
if (action == QUEUE_DO_NONE) {
fprintf(stderr,
"you need to specify one of -a/--activate, -i/--deactivate, "
"-o/--open, or -c/--close\n");
free(queue);
return -1;
}
queuectrl.action = action;
queuectrl.queue = queue;
@ -3024,17 +3254,36 @@ int cmd_qctrl(int argc, char* argv[])
* but are rejected -EINVAL server-side today, so no flag is offered for them
* here). Used by the config-upgrade chaotic soak (task 8.6) to set a host's
* admin status before/around a config upgrade and assert it survives. */
static const flagspec_t hostctrl_flags[] = {
{ 'o', "open", "open", no_argument, 0, NULL },
{ 'c', "close", "close", no_argument, 0, NULL },
{ 'h', "host", "host", required_argument, 1, NULL },
};
#define HOSTCTRL_NFLAGS (sizeof(hostctrl_flags) / sizeof(hostctrl_flags[0]))
int cmd_hostctrl(int argc, char* argv[])
{
int opt;
int rc;
xce_hostctrl_t hostctrl;
int action = HOST_DO_NONE;
int action = HOST_DO_NONE;
char* hostname = NULL;
struct option long_opts[HOSTCTRL_NFLAGS + 1];
char optstring[16];
optind = 1;
opterr = 0;
while ((opt = getopt(argc, argv, "och:")) != -1) {
if (flagspec_build_opts(hostctrl_flags, HOSTCTRL_NFLAGS, long_opts,
optstring, sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: hostctrl: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case 'o':
action = HOST_OPEN;
@ -3045,8 +3294,11 @@ int cmd_hostctrl(int argc, char* argv[])
case 'h':
hostname = strdup(optarg);
break;
default:
break;
case '?':
FREEUP(hostname);
return flagspec_reject("hostctrl", hostctrl_flags,
HOSTCTRL_NFLAGS, argv[optind - 1],
"hostctrl -h <hostname> (-o|-c)");
}
}
@ -3150,6 +3402,19 @@ int cmd_upgradeconfig(int argc, char* argv[])
return 0;
}
static const flagspec_t runtasks_flags[] = {
{ 'm', "host", "host", required_argument, 0,
"1 task; repeatable up to 5" },
{ 'u', "user", "user", required_argument, 0, NULL },
{ 'c', "command-line", "command-line", required_argument, 0, NULL },
{ 'x', "exdata", "exdata", required_argument, 0, NULL },
{ 'd', "dir", "dir", required_argument, 0, NULL },
{ 'j', "jobid", "jobid", required_argument, 0, NULL },
{ 't', "launch-type", "launch-type", required_argument, 0,
"\"mpi\" if >1 total tasks, else \"sequential\"" },
};
#define RUNTASKS_NFLAGS (sizeof(runtasks_flags) / sizeof(runtasks_flags[0]))
int cmd_runtasks(int argc, char* argv[])
{
xce_taskhost_t hosts[MAX_TASK_HOSTS];
@ -3167,10 +3432,22 @@ int cmd_runtasks(int argc, char* argv[])
char* launch_type = NULL;
int i;
uint64_t jobid = 0;
struct option long_opts[RUNTASKS_NFLAGS + 1];
char optstring[32];
optind = 1;
opterr = 0;
while ((opt = getopt(argc, argv, "m:u:c:x:d:j:t:")) != -1) {
if (flagspec_build_opts(runtasks_flags, RUNTASKS_NFLAGS, long_opts,
optstring, sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: taskrun: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case 'm':
if (nhosts >= MAX_TASK_HOSTS) {
@ -3229,8 +3506,12 @@ int cmd_runtasks(int argc, char* argv[])
case 't':
launch_type = strdup(optarg);
break;
default:
break;
case '?':
return flagspec_reject(
"taskrun", runtasks_flags, RUNTASKS_NFLAGS,
argv[optind - 1],
"taskrun [-j <jobid>] [-m <host>[:<n>]]... [-u <user>] "
"[-c <cmdline>] [-x <exdata>] [-d <dir>] [-t <launch_type>]");
}
}
@ -3311,6 +3592,12 @@ static int add_pendreason(xce_job_preason_t* preason, char* s)
return add_job_preason(preason, code, phost, pres);
}
static const flagspec_t setpend_flags[] = {
{ 'j', "jobid", "jobid", required_argument, 1, NULL },
{ 'p', "reason", "reason", required_argument, 1, NULL },
};
#define SETPEND_NFLAGS (sizeof(setpend_flags) / sizeof(setpend_flags[0]))
int cmd_setpend(int argc, char* argv[])
{
int opt;
@ -3318,11 +3605,23 @@ int cmd_setpend(int argc, char* argv[])
xce_pendreason_t preason = { .reasons = &jreasons };
xce_preason_set_t reasons
= { .nreasons = 1, .reasons = (xce_pendreason_t*[]) { &preason } };
int rc = 0;
int rc = 0;
struct option long_opts[SETPEND_NFLAGS + 1];
char optstring[16];
optind = 1;
opterr = 0;
while ((opt = getopt(argc, argv, "j:p:")) != -1) {
if (flagspec_build_opts(setpend_flags, SETPEND_NFLAGS, long_opts,
optstring, sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: setpend: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
int rc = 0;
switch (opt) {
case 'j':
@ -3335,6 +3634,10 @@ int cmd_setpend(int argc, char* argv[])
return rc;
}
break;
case '?':
return flagspec_reject(
"setpend", setpend_flags, SETPEND_NFLAGS, argv[optind - 1],
"setpend -j <jobid> -p <reason> [-p <reason> ...]");
}
}
@ -3405,6 +3708,13 @@ int cmd_join_worker(int argc, char* argv[])
* apply_committed_entry()) that a real worker's report would, without
* requiring a real scheduler dispatch (on_job_start only requires the job to
* already exist in jobmgr, i.e. have been submitted). */
static const flagspec_t jobstart_flags[] = {
{ 'j', "jobid", "jobid", required_argument, 1, NULL },
{ 'p', "pid", "pid", required_argument, 0,
"this xce_test process's own pid" },
};
#define JOBSTART_NFLAGS (sizeof(jobstart_flags) / sizeof(jobstart_flags[0]))
int cmd_jobstart(int argc, char* argv[])
{
int rc;
@ -3413,11 +3723,23 @@ int cmd_jobstart(int argc, char* argv[])
unsigned int pid = 0;
unsigned int pids[1];
job_pidinfo_t info;
struct option long_opts[JOBSTART_NFLAGS + 1];
char optstring[16];
optind = 1;
opterr = 0;
memset(&info, 0x0, sizeof(info));
while ((opt = getopt(argc, argv, "j:p:")) != -1) {
if (flagspec_build_opts(jobstart_flags, JOBSTART_NFLAGS, long_opts,
optstring, sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: jstart: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case 'j':
parse_jobid(&jobid, optarg);
@ -3425,9 +3747,10 @@ int cmd_jobstart(int argc, char* argv[])
case 'p':
pid = (unsigned int)atoi(optarg);
break;
default:
printf(" jstart -j jobid [-p pid]\n");
return -EINVAL;
case '?':
return flagspec_reject("jstart", jobstart_flags, JOBSTART_NFLAGS,
argv[optind - 1],
"jstart -j <jobid> [-p <pid>]");
}
}
@ -3456,6 +3779,12 @@ int cmd_jobstart(int argc, char* argv[])
return 0;
}
static const flagspec_t jobfinish_flags[] = {
{ 'j', "jobid", "jobid", required_argument, 1, NULL },
{ 'c', "exit-code", "exit-code", required_argument, 0, "0" },
};
#define JOBFINISH_NFLAGS (sizeof(jobfinish_flags) / sizeof(jobfinish_flags[0]))
int cmd_jobfinish(int argc, char* argv[])
{
int rc;
@ -3463,6 +3792,8 @@ int cmd_jobfinish(int argc, char* argv[])
uint64_t jobid = 0;
int code = 0;
xce_jobfinish_t finish;
struct option long_opts[JOBFINISH_NFLAGS + 1];
char optstring[16];
/* raft-apply-decoupling task 7.2: reset optind so repeated jfinish
* invocations within one xce_test process (the fact-stream soak scenario
@ -3474,8 +3805,18 @@ int cmd_jobfinish(int argc, char* argv[])
* cmd_ handlers in this file already follow (e.g. cmd_join_worker just
* above, cmd_jobsub, cmd_resupdate). */
optind = 1;
opterr = 0;
while ((opt = getopt(argc, argv, "j:c:")) != -1) {
if (flagspec_build_opts(jobfinish_flags, JOBFINISH_NFLAGS, long_opts,
optstring, sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: jfinish: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case 'j':
parse_jobid(&jobid, optarg);
@ -3483,9 +3824,10 @@ int cmd_jobfinish(int argc, char* argv[])
case 'c':
code = atoi(optarg);
break;
default:
printf(" jfinish -j jobid [-c exitcode]\n");
return -EINVAL;
case '?':
return flagspec_reject("jfinish", jobfinish_flags,
JOBFINISH_NFLAGS, argv[optind - 1],
"jfinish -j <jobid> [-c <exitcode>]");
}
}
@ -3516,6 +3858,14 @@ int cmd_jobfinish(int argc, char* argv[])
* -> apply_job_fail_transition() at apply time. Reuses the existing
* job_failinfo_t shape already exercised internally by this file's own
* notify_job_fail() helper (used by run_job() on a local exec failure). */
static const flagspec_t jobfail_flags[] = {
{ 'j', "jobid", "jobid", required_argument, 1, NULL },
{ 'c', "failure-code", "failure-code", required_argument, 0, "0" },
{ 's', "stage", "stage", required_argument, 0,
"JOB_EXEC_STAGE_POST_EXEC" },
};
#define JOBFAIL_NFLAGS (sizeof(jobfail_flags) / sizeof(jobfail_flags[0]))
int cmd_jobfail(int argc, char* argv[])
{
int rc;
@ -3524,11 +3874,23 @@ int cmd_jobfail(int argc, char* argv[])
int code = 0;
int stage = JOB_EXEC_STAGE_POST_EXEC;
job_failinfo_t failinfo;
struct option long_opts[JOBFAIL_NFLAGS + 1];
char optstring[16];
optind = 1;
opterr = 0;
memset(&failinfo, 0x0, sizeof(failinfo));
while ((opt = getopt(argc, argv, "j:c:s:")) != -1) {
if (flagspec_build_opts(jobfail_flags, JOBFAIL_NFLAGS, long_opts,
optstring, sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: jfail: internal error building flag table\n");
return -EINVAL;
}
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case 'j':
parse_jobid(&jobid, optarg);
@ -3539,9 +3901,10 @@ int cmd_jobfail(int argc, char* argv[])
case 's':
stage = atoi(optarg);
break;
default:
printf(" jfail -j jobid [-c code] [-s stage]\n");
return -EINVAL;
case '?':
return flagspec_reject(
"jfail", jobfail_flags, JOBFAIL_NFLAGS, argv[optind - 1],
"jfail -j <jobid> [-c <code>] [-s <stage>]");
}
}
@ -3758,6 +4121,14 @@ static void resupdate_report_one(char* name, char* host, xce_val_t* sent)
}
}
static const flagspec_t resupdate_flags[] = {
{ 'n', "resname", "resname", required_argument, 1, NULL },
{ 'v', "value", "value", required_argument, 1, NULL },
{ 't', "value-type", "value-type", required_argument, 0,
"numeric unless overridden" },
};
#define RESUPDATE_NFLAGS (sizeof(resupdate_flags) / sizeof(resupdate_flags[0]))
int cmd_resupdate(int argc, char* argv[])
{
#define MAX_N_RES (16)
@ -3770,15 +4141,28 @@ int cmd_resupdate(int argc, char* argv[])
int type_override = RESVAL_TYPE_UNKNOWN;
int opt;
int rc;
struct option long_opts[RESUPDATE_NFLAGS + 1];
char optstring[16];
optind = 1;
opterr = 0;
nres = 0;
if (flagspec_build_opts(resupdate_flags, RESUPDATE_NFLAGS, long_opts,
optstring, sizeof(optstring))
< 0) {
fprintf(stderr,
"xce_test: resupdate: internal error building flag table\n");
return -EINVAL;
}
/* -t is deferred: it applies to the pair most recently opened by -n
* (design.md's documented order is -n/-v/[-t]), so a pending pair's
* set_resval() call is deferred until the NEXT -n or end-of-args --
* not fired immediately on -v as before -- so a trailing -t still has
* a chance to be seen for that same pair. */
while ((opt = getopt(argc, argv, "n:v:t:")) != -1) {
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
!= -1) {
switch (opt) {
case 'n':
if (resname != NULL) {
@ -3827,6 +4211,12 @@ int cmd_resupdate(int argc, char* argv[])
printf("ignore -t `%s' as no res name found\n", optarg);
}
break;
case '?':
return flagspec_reject(
"resupdate", resupdate_flags, RESUPDATE_NFLAGS,
argv[optind - 1],
"resupdate -n <resname> -v <value> [-t <numeric|string>] "
"[-n <resname> -v <value> [-t <numeric|string>] ...]");
}
}

View file

@ -1074,6 +1074,366 @@ static void test_source_nested_noncyclic_executes_every_line(void)
clitest_fixture_cleanup_dir();
}
/* ------------------------------------------------------------------ */
/* Flag Parsing Layer migration (task 3.1, Requirements 2.1-2.4, 6.1-6.4, */
/* 10.1-10.3) */
/* ------------------------------------------------------------------ */
/*
* test_qctrl_missing_action_flag_rejected -- bug-1704's mandatory
* regression test, named explicitly by task 3.1's Observable Completion
* line: `qctrl` given a queue name but NONE of its mutually-exclusive
* action flags (-a/-i/-o/-c, nor their new long forms --activate/
* --deactivate/--open/--close) must be REJECTED, not silently transmit an
* indeterminate (pre-fix: uninitialized) `action` value over the wire.
* The sentinel-and-reject check (mirroring cmd_hostctrl's pre-existing
* HOST_DO_NONE pattern) runs entirely inside cmd_qctrl before any
* xcecli_queuectrl() network call, so no live controller is needed for
* this assertion to be meaningful. */
static void test_qctrl_missing_action_flag_rejected(void)
{
char xce_test_path[PATH_MAX];
char* const argv[] = { "xce_test", "--norc", NULL };
clitest_session_t sess;
clitest_buf_t out;
clitest_buf_t err;
int exit_code = -999;
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, resolve_xce_test_path(xce_test_path, sizeof(xce_test_path)),
"must be able to resolve the sibling xce_test binary's path");
clitest_buf_init(&out);
clitest_buf_init(&err);
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, clitest_spawn(&sess, xce_test_path, argv),
"clitest_spawn must launch xce_test behind a real pty");
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err, "xce>", false, 5000),
"expected the initial 'xce>' interactive prompt on stdout");
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "client"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err, "xce/client>", false, 5000),
"expected to enter the client context");
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "qctrl -q normal"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err,
"you need to specify one of", true, 5000),
"bug-1704: qctrl given -q but none of -a/-i/-o/-c (nor their long "
"forms) must be rejected with an explicit stderr error, never send "
"an indeterminate action");
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "quit"));
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, clitest_wait_for_exit(&sess, &out, &err, 5000, &exit_code),
"xce_test did not exit within the harness's safety timeout after "
"'quit'");
TEST_ASSERT_EQUAL_INT_MESSAGE(
1, exit_code,
"the rejected qctrl invocation must mark the session failed");
clitest_kill_and_reap(&sess);
clitest_close(&sess);
clitest_buf_free(&out);
clitest_buf_free(&err);
}
/*
* test_qctrl_flag_forms_short_and_long_both_accepted -- Requirement 6.3
* companion to the bug-1704 test above: proves the migration did not
* regress a VALID invocation for either qctrl's pre-existing short action
* flag (-a) or its newly added long form (--activate). Both must parse
* past the sentinel-and-reject check and reach the actual
* xcecli_queuectrl() network call (which then fails fast, since no live
* controller is running in this unit-test environment -- that failure
* itself is not under test here, only that parsing succeeded and the
* command was NOT rejected as an unrecognized flag or a missing action).
* A distinct `echo` marker after each qctrl invocation gives a reliable
* command-boundary signal on stdout (REPL commands execute strictly in
* order), so the stderr accumulated by the time each marker appears can be
* inspected without racing the (synchronous, blocking) qctrl call itself.
*/
static void test_qctrl_flag_forms_short_and_long_both_accepted(void)
{
char xce_test_path[PATH_MAX];
char* const argv[] = { "xce_test", "--norc", NULL };
clitest_session_t sess;
clitest_buf_t out;
clitest_buf_t err;
int exit_code = -999;
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, resolve_xce_test_path(xce_test_path, sizeof(xce_test_path)),
"must be able to resolve the sibling xce_test binary's path");
clitest_buf_init(&out);
clitest_buf_init(&err);
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, clitest_spawn(&sess, xce_test_path, argv),
"clitest_spawn must launch xce_test behind a real pty");
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err, "xce>", false, 5000),
"expected the initial 'xce>' interactive prompt on stdout");
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "client"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err, "xce/client>", false, 5000),
"expected to enter the client context");
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "qctrl -q normal -a"));
TEST_ASSERT_EQUAL_INT(
0, clitest_send_line(&sess, "echo marker_qctrl_short_form_done"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err,
"marker_qctrl_short_form_done", false, 5000),
"the short-form '-a' invocation must run to completion");
TEST_ASSERT_EQUAL_INT(
0, clitest_send_line(&sess, "qctrl -q normal --activate"));
TEST_ASSERT_EQUAL_INT(
0, clitest_send_line(&sess, "echo marker_qctrl_long_form_done"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err,
"marker_qctrl_long_form_done", false, 5000),
"the long-form '--activate' invocation must run to completion");
TEST_ASSERT_NULL_MESSAGE(
strstr(err.data, "unrecognized flag"),
"neither the short form '-a' nor its long form '--activate' may be "
"rejected as an unrecognized flag (Requirement 6.3: adding a long "
"form must not change the short flag's existing behavior)");
TEST_ASSERT_NULL_MESSAGE(
strstr(err.data, "you need to specify one of"),
"both '-a' and '--activate' must be recognized as setting the "
"action -- bug-1704's sentinel check must not fire for either");
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "quit"));
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, clitest_wait_for_exit(&sess, &out, &err, 5000, &exit_code),
"xce_test did not exit within the harness's safety timeout after "
"'quit'");
clitest_kill_and_reap(&sess);
clitest_close(&sess);
clitest_buf_free(&out);
clitest_buf_free(&err);
}
/*
* test_migrated_commands_sample_reject_unrecognized_flag -- representative
* sample (task 3.1's flagspec migration touched ~17 commands; the shared
* flagspec_reject()/flagspec_build_opts() mechanism itself is already
* unit-tested in isolation by tasks 1.1/1.2's test_flagspec.c, so
* exhaustively black-box-testing every migrated command here would be
* disproportionate). This sample specifically covers commands whose
* PRE-MIGRATION behavior on an unrecognized flag was to silently ignore it
* and keep running (jobinfo and jobkill: no default:/case '?' arm at all,
* or a `default: break;` -- research.md's documented finding) -- exactly
* the defect class Requirement 2.1/2.3 exists to close -- plus hostctrl
* (cmd_qctrl's sibling, named explicitly in this task's brief) and
* setpend/resupdate (which had NO default arm whatsoever). Every assertion
* fires before any network call inside each cmd_* handler, so no live
* controller is needed. jobinfo's case additionally proves Requirement
* 10.1's typo suggestion end-to-end ('-z' closely matches '-j'/'--jobid').
*/
static void test_migrated_commands_sample_reject_unrecognized_flag(void)
{
char xce_test_path[PATH_MAX];
char* const argv[] = { "xce_test", "--norc", NULL };
clitest_session_t sess;
clitest_buf_t out;
clitest_buf_t err;
int exit_code = -999;
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, resolve_xce_test_path(xce_test_path, sizeof(xce_test_path)),
"must be able to resolve the sibling xce_test binary's path");
clitest_buf_init(&out);
clitest_buf_init(&err);
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, clitest_spawn(&sess, xce_test_path, argv),
"clitest_spawn must launch xce_test behind a real pty");
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err, "xce>", false, 5000),
"expected the initial 'xce>' interactive prompt on stdout");
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "client"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err, "xce/client>", false, 5000),
"expected to enter the client context");
/* jobinfo: previously had no default:/case '?' arm at all -- an
* unrecognized flag silently fell through and the loop simply moved
* on. Also proves Requirement 10.1's typo suggestion: '-z' is within
* this table's edit-distance threshold of '-j'. */
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "jobinfo -z"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(
&sess, &out, &err,
"jobinfo: unrecognized flag '-z' (did you mean '-j/--jobid'?)",
true, 5000),
"jobinfo must reject an unrecognized flag with a usage summary and "
"a typo suggestion, not silently ignore it (Requirements 2.1, "
"10.1)");
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err, "usage: jobinfo", true,
5000),
"the rejection must include jobinfo's usage summary (Requirement "
"2.4)");
/* jobkill: previously `default: break;` -- an unrecognized flag given
* ALONGSIDE a valid -j was silently ignored and the command still
* executed. Sending it after a valid -j proves the loop now stops and
* rejects at the first bad flag, before ever reaching the network
* call or jobid validation. */
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "jobkill -j 1 -z"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err,
"jobkill: unrecognized flag '-z'", true, 5000),
"jobkill must reject an unrecognized flag instead of silently "
"ignoring it and proceeding (Requirement 2.1)");
/* hostctrl: cmd_qctrl's sibling, named explicitly in this task's own
* brief as already having the correct sentinel pattern this migration
* must preserve; also previously `default: break;`. */
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "hostctrl --bogus"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err,
"hostctrl: unrecognized flag '--bogus'", true,
5000),
"hostctrl must reject an unrecognized flag (Requirement 2.1)");
/* setpend lives in the SCHEDULER context (scheduler_commands[]), not
* client -- leave client and switch contexts before exercising it.
* Previously had NO default:/case '?' arm whatsoever. */
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, ".."));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err, "xce>", false, 5000),
"expected to return to the top-level context after '..'");
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "scheduler"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err, "xce/scheduler>", false,
5000),
"expected to enter the scheduler context");
TEST_ASSERT_EQUAL_INT(0,
clitest_send_line(&sess, "setpend -j 1 --bogus"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err,
"setpend: unrecognized flag '--bogus'", true,
5000),
"setpend must reject an unrecognized flag (Requirement 2.1)");
/* resupdate lives in the WORKER context (worker_commands[]), not
* client or scheduler -- switch again. Previously had NO
* default:/case '?' arm whatsoever. */
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, ".."));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err, "xce>", false, 5000),
"expected to return to the top-level context after '..'");
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "worker"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err, "xce/worker>", false, 5000),
"expected to enter the worker context");
TEST_ASSERT_EQUAL_INT(
0, clitest_send_line(&sess, "resupdate -n foo -v 1 --bogus"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err,
"resupdate: unrecognized flag '--bogus'",
true, 5000),
"resupdate must reject an unrecognized flag (Requirement 2.1)");
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "quit"));
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, clitest_wait_for_exit(&sess, &out, &err, 5000, &exit_code),
"xce_test did not exit within the harness's safety timeout after "
"'quit'");
TEST_ASSERT_EQUAL_INT_MESSAGE(
1, exit_code,
"every rejected invocation above must mark the session failed");
clitest_kill_and_reap(&sess);
clitest_close(&sess);
clitest_buf_free(&out);
clitest_buf_free(&err);
}
/*
* test_walcompare_unrecognized_flag_rejected_with_usage -- covers the ONE
* command that used a fully hand-rolled long-flag-only parser
* (strcmp(argv[ai], "--report")-style) rather than getopt(), per this
* task's explicit scope. Also proves this migration fixed a genuine
* silent-acceptance bug: the pre-migration hand-rolled loop treated ANY
* unrecognized token (including a mistyped "--" flag) as the start of the
* positional member-path list rather than rejecting it, so `walcompare
* --bogus a b` used to run `walcompare` against members ["--bogus", "a",
* "b"] instead of failing loudly. walcompare is a top-level, network-free
* offline tool, so this test needs no context switch and no live
* controller. */
static void test_walcompare_unrecognized_flag_rejected_with_usage(void)
{
char xce_test_path[PATH_MAX];
char* const argv[] = { "xce_test", "--norc", NULL };
clitest_session_t sess;
clitest_buf_t out;
clitest_buf_t err;
int exit_code = -999;
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, resolve_xce_test_path(xce_test_path, sizeof(xce_test_path)),
"must be able to resolve the sibling xce_test binary's path");
clitest_buf_init(&out);
clitest_buf_init(&err);
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, clitest_spawn(&sess, xce_test_path, argv),
"clitest_spawn must launch xce_test behind a real pty");
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err, "xce>", false, 5000),
"expected the initial 'xce>' interactive prompt on stdout");
TEST_ASSERT_EQUAL_INT(
0, clitest_send_line(&sess, "walcompare --bogus /tmp/a /tmp/b"));
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err,
"walcompare: unrecognized flag '--bogus'",
true, 5000),
"walcompare must reject an unrecognized long flag instead of "
"silently treating it as a positional member path (Requirement "
"2.1)");
TEST_ASSERT_TRUE_MESSAGE(
clitest_wait_for_text(&sess, &out, &err, "usage: walcompare", true,
5000),
"the rejection must include walcompare's usage summary "
"(Requirement 2.4)");
TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "quit"));
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, clitest_wait_for_exit(&sess, &out, &err, 5000, &exit_code),
"xce_test did not exit within the harness's safety timeout after "
"'quit'");
TEST_ASSERT_EQUAL_INT_MESSAGE(
1, exit_code,
"the rejected walcompare invocation must mark the session failed");
clitest_kill_and_reap(&sess);
clitest_close(&sess);
clitest_buf_free(&out);
clitest_buf_free(&err);
}
void test_test_cli(void)
{
UnitySetTestFile(__FILE__);
@ -1082,4 +1442,8 @@ void test_test_cli(void)
RUN_TEST(test_source_mutual_cycle_rejected_with_error);
RUN_TEST(test_source_missing_file_rejected_with_error);
RUN_TEST(test_source_nested_noncyclic_executes_every_line);
RUN_TEST(test_qctrl_missing_action_flag_rejected);
RUN_TEST(test_qctrl_flag_forms_short_and_long_both_accepted);
RUN_TEST(test_migrated_commands_sample_reject_unrecognized_flag);
RUN_TEST(test_walcompare_unrecognized_flag_rejected_with_usage);
}