diff --git a/.kiro/specs/job-array-one-based-index/tasks.md b/.kiro/specs/job-array-one-based-index/tasks.md index 2a47c4ee..37eade56 100644 --- a/.kiro/specs/job-array-one-based-index/tasks.md +++ b/.kiro/specs/job-array-one-based-index/tasks.md @@ -85,7 +85,7 @@ - Done when the autovariable unit test passes for `$jobid[3]`, `$jobid[3]x`, unset `$jobid`, `$?`, `$jobcount` set and unset, and `!`/`#` lines, and the existing harness tests still pass - _Requirements: 4.10, 4.11_ -- [ ] 4.3 Parse single-job harness arguments strictly +- [x] 4.3 Parse single-job harness arguments strictly - Add one argument helper that trims spaces/tabs, applies the libxce selection parser, refuses element ranges (including reversed ones) with the single-element message, and reports whether an element number was given - Add a not-found reporter that adds the `[k]` hint for identities given without an element number - Use both in every single-job command (including `jobinfo` per `-j`) and make `expect-job` parse its argument the same way; the job-list lookup behind it compares job numbers instead of formatted text @@ -242,3 +242,4 @@ - 1.3: plain `idgen()` still has no sequence-overflow guard; after `idgen_array` takes block 31 the context sequence is 2^20, so a plain `idgen()` in the same second would spill into the timestamp (pre-existing class). Add an `-EOVERFLOW` check to plain `idgen()` in 9.1. - 2.2: two extra XCE_TESTING seams exist (`jobsubmit_test_release_hook`, `jobsubmit_inflight_try_add_for_test`); the controller test fixture has no WAL, so async commit-failure runs synchronously inside handle_jobsubmit — reuse it in 2.3. Stale comment at `src/controller/testsuite/test_commit_barrier_reply.c:1458` (claims release via jobsubmit_async_ctx_release) — fix in 2.3. - 4.1: after `jobsub -a N`, `$jobid` is the array identity, so `expect-job $jobid` currently reports not found; 4.3 must make it behave per 4.7/4.9 (not-found hint naming `[k]`, or exact-ID match). Nit for 4.3: the `refused` early return in `cmd_jobsub` sits between the bug-786 ack comment and the code it describes — move it above the comment. +- 4.3: the controller acks `setpend` (`do_set_preason`, job.c ~3869-3880) and `jobrun` (`handle_jobdispatch`, job.c ~2246-2258) with status 0 before doing the work, so those two harness commands can never report not-found; 4.7/5.13 for them depend on a controller change outside this spec's boundary. `jobinfo`'s not-found is `rc == 0` with no row (`do_reply_joblist`), not -ENOENT. diff --git a/src/harness/cmdutil.c b/src/harness/cmdutil.c index 5f186e6a..ed480c1b 100644 --- a/src/harness/cmdutil.c +++ b/src/harness/cmdutil.c @@ -12,7 +12,7 @@ #include "cmdutil.h" #include "config.h" /* MAX_ARRAY_IDX */ -#include "utils.h" /* xce_strerror_r */ +#include "utils.h" /* xce_strerror_r, parse_jobsel */ #include "xce/errnox.h" /* strtol over [s, s + len): the whole span must be digits (optional @@ -83,6 +83,76 @@ int cmdutil_parse_array_size(const char* text, uint32_t* n) return 0; } +int cmdutil_job_arg(const char* arg, uint64_t* jobid, int* had_index, + char* errbuf, size_t errlen) +{ + jobsel_t sel; + jobsel_err_t err; + const char* shown = (arg != NULL) ? arg : ""; + + if ((jobid == NULL) || (had_index == NULL)) { + if ((errbuf != NULL) && (errlen > 0)) { + snprintf(errbuf, errlen, "internal error: no result buffer"); + } + return -EINVAL; + } + + err = parse_jobsel(arg, &sel); + + /* 4.8: a range -- reversed ones too, since 5.5 applies only to the + * command-line client and REST -- is refused as a whole; the harness + * acts on exact IDs only */ + if ((err == JOBSEL_EREVERSED) + || ((err == JOBSEL_OK) && (sel.form == JOBSEL_RANGE))) { + if ((errbuf != NULL) && (errlen > 0)) { + snprintf(errbuf, errlen, + "'%s' selects a range of elements; this command takes " + "a single element [k]", + shown); + } + return -EINVAL; + } + + if (err != JOBSEL_OK) { + if ((errbuf != NULL) && (errlen > 0)) { + snprintf(errbuf, errlen, "invalid jobid '%s': %s", shown, + jobsel_strerror(err)); + } + return -EINVAL; + } + + if (sel.form == JOBSEL_SINGLE) { + *jobid = jobsel_element_id(&sel, sel.lo); + *had_index = 1; + } else { + *jobid = sel.key; + *had_index = 0; + } + return 0; +} + +void cmdutil_job_not_found(FILE* out, const char* arg, int had_index) +{ + fprintf(out, "job %s not found\n", (arg != NULL) ? arg : ""); + if (!had_index) { + fprintf(out, " (array elements must be named as [k])\n"); + } +} + +int cmdutil_job_lookup_report(FILE* out, const char* cmd, int rc, + const void* job, const char* arg, int had_index) +{ + if ((rc < 0) && (rc != -ENOENT)) { + return rc; + } + if ((rc >= 0) && (job != NULL)) { + return 0; + } + fprintf(out, "%s: ", (cmd != NULL) ? cmd : ""); + cmdutil_job_not_found(out, arg, had_index); + return -ENOENT; +} + int cmdutil_split_host_tasks(const char* arg, char* host, size_t hostsz, int* ntasks) { diff --git a/src/harness/include/cmdutil.h b/src/harness/include/cmdutil.h index 617608bc..c4a21406 100644 --- a/src/harness/include/cmdutil.h +++ b/src/harness/include/cmdutil.h @@ -7,7 +7,8 @@ #define CMDUTIL_H #include /* size_t */ -#include /* uint32_t */ +#include /* uint32_t, uint64_t */ +#include /* FILE */ /* Parse all of `s` as a base-10 integer in [lo, hi] into *out. * Returns 0, -EINVAL (empty, NULL or not entirely numeric) or -ERANGE @@ -31,6 +32,31 @@ int cmdutil_parse_ntasks(const char* arg, uint32_t* min, uint32_t* max); * 32767, MAX_ARRAY_IDX); *n is left unchanged on error. */ int cmdutil_parse_array_size(const char* text, uint32_t* n); +/* One job argument of a harness command that acts on one job + * (job-array-one-based-index Req 4.7-4.9, 5.7, 5.8, 5.13-5.15, 8.2): + * trims spaces/tabs and parses it with parse_jobsel(). A range + * `[a-b]` (reversed ones included) is refused with the 4.8 + * single-element message; any other rejection names the argument and the + * libxce reason. On success returns 0 with *jobid the exact ID (the element + * ID for `[k]`, `[0]` included) and *had_index set when `[k]` was given; on + * rejection returns -EINVAL, writes the message into errbuf (when non-NULL) + * and leaves *jobid and *had_index unchanged. */ +int cmdutil_job_arg(const char* arg, uint64_t* jobid, int* had_index, + char* errbuf, size_t errlen); + +/* Prints "job not found" and, when !had_index, the 4.7 hint that + * array elements must be named as [k], to `out`. */ +void cmdutil_job_not_found(FILE* out, const char* arg, int had_index); + +/* Judge one single-job lookup (job-array-one-based-index Req 4.7, 5.13): + * the controller answers a lookup of an unknown job with rc 0 and no row, + * so `job == NULL` with rc 0 is "not found", not success. Returns 0 when a + * row came back (prints nothing); -ENOENT when rc is 0 with no row or rc is + * -ENOENT, after printing ": " + cmdutil_job_not_found() to `out`; + * otherwise rc (< 0, an RPC error) unprinted, for the caller to report. */ +int cmdutil_job_lookup_report(FILE* out, const char* cmd, int rc, + const void* job, const char* arg, int had_index); + /* jobsub's default exdata: when *exdata is NULL, set it to a newly * allocated {"sub_env_vars":{"PATH":}} JSON string; an exdata the * caller already set (-x) is left alone. Returns 0 or -ENOMEM. */ diff --git a/src/harness/test.c b/src/harness/test.c index 9c6d112a..dbfb6385 100644 --- a/src/harness/test.c +++ b/src/harness/test.c @@ -2457,8 +2457,40 @@ static int joblist_thunk(void* ctxp) return call->rc; } -static int joblist_query(const char* find_jobid, int* contains, int print_table, - int quiet, int json_mode) +/* job-array-one-based-index Req 4.7, 4.8, 5.13-5.15: parse one job argument + * of a single-job command with cmdutil_job_arg(); on rejection print + * ": " and return -EINVAL, so the command stops before any RPC + * and never sends job 0 for a rejected argument. */ +static int job_arg_or_report(const char* cmd, const char* arg, uint64_t* jobid, + int* had_index) +{ + char ebuf[256]; + + if (cmdutil_job_arg(arg, jobid, had_index, ebuf, sizeof(ebuf)) < 0) { + fprintf(stderr, "%s: %s\n", cmd, ebuf); + errno = EINVAL; + return -EINVAL; + } + return 0; +} + +/* RPC failure of a single-job command: the controller's not-found reply + * (-ENOENT) goes through cmdutil_job_not_found() (4.7 hint when `arg` had + * no [k]); anything else through cmdutil_rpc_error(). */ +static void job_rpc_error(const char* cmd, const char* what, int rc, + const char* arg, int had_index) +{ + if (rc == -ENOENT) { + fprintf(stderr, "%s: ", cmd); + cmdutil_job_not_found(stderr, arg, had_index); + errno = ENOENT; + return; + } + cmdutil_rpc_error(what, rc); +} + +static int joblist_query(const uint64_t* find_jobid, int* contains, + int print_table, int quiet, int json_mode) { int rc, i; xce_job_t** jobs; @@ -2520,8 +2552,10 @@ static int joblist_query(const char* find_jobid, int* contains, int print_table, fmt_jobid(buf, sizeof(buf) - 1, jobs[i]->jobid); + /* Req 4.9: membership compares job numbers, so any written form + * of the identity that parse_jobsel() accepts matches */ if ((find_jobid != NULL) && (contains != NULL) - && (strcmp(buf, find_jobid) == 0)) { + && (jobs[i]->jobid == *find_jobid)) { *contains = 1; } @@ -2684,37 +2718,47 @@ int cmd_expect_jobs(int argc, char* argv[]) return 0; } -/* `expect-job ` assertion (xce-test-e2e-primitives task 5.3, Req 5.3/5.4, - * design.md cmd_expect_jobs / cmd_expect_job). +/* `expect-job ` assertion (xce-test-e2e-primitives task 5.3, + * Req 5.3/5.4, design.md cmd_expect_jobs / cmd_expect_job). * - * Passes (returns 0) when a job whose `fmt_jobid` string equals argv[1] is - * present in the current job list; otherwise prints the missing id and returns a - * negative errno-style value so preprocess_and_run records the session as failed - * (Req 1.1). + * argv[1] is parsed with cmdutil_job_arg() (job-array-one-based-index Req + * 4.7-4.9, 5.13-5.15): a rejected identity or a range of elements fails the + * assertion without querying. Passes (returns 0) when a job whose number + * equals the parsed ID is present in the current job list (4.9: compared as + * numbers, not text); otherwise prints the missing id -- with the + * `[k]` hint when no [k] was given (4.7), e.g. `expect-job $jobid` + * after `jobsub -a N` -- and returns a negative errno-style value so + * preprocess_and_run records the session as failed (Req 1.1). * - * `$jobid` substitution is transparent: autovar substitution (task 3.1) replaces - * `$jobid` with the captured id string BEFORE this handler runs, so argv[1] is - * already the literal id. The membership test in joblist_query uses the SAME - * `fmt_jobid` representation that cmd_jobsub captured into `$jobid` (task 4.1), - * so `expect-job $jobid` matches the joblist entry. + * `$jobid` substitution is transparent: autovar substitution (task 3.1) + * replaces `$jobid` with the captured id string BEFORE this handler runs, so + * argv[1] is already the literal id. * * A missing argument is itself an assertion failure, not a no-op. A failed * joblist query is also a failure: we cannot determine membership, so we report - * the query failure distinctly rather than silently equating it with "absent". */ + * the query failure distinctly rather than silently equating it with "absent". + */ int cmd_expect_job(int argc, char* argv[]) { - int contains; - int rc; + int contains; + int rc; + uint64_t jobid; + int had_index; if (argc != 2) { fprintf(stderr, "expect-job \n"); return -EINVAL; } + rc = job_arg_or_report("expect-job", argv[1], &jobid, &had_index); + if (rc < 0) { + return rc; + } + /* Print-free membership query via the shared helper. A negative rc means the * query itself failed (e.g. no reachable master) — that is an assertion * failure, distinct from "job absent". */ - rc = joblist_query(argv[1], &contains, 0, /*quiet=*/0, /*json_mode=*/0); + rc = joblist_query(&jobid, &contains, 0, /*quiet=*/0, /*json_mode=*/0); if (rc < 0) { fprintf(stderr, @@ -2725,7 +2769,8 @@ int cmd_expect_job(int argc, char* argv[]) } if (!contains) { - fprintf(stderr, "expect-job: job %s not found\n", argv[1]); + fprintf(stderr, "expect-job: "); + cmdutil_job_not_found(stderr, argv[1], had_index); return -ENOENT; } @@ -3235,16 +3280,17 @@ int cmd_jobinfo(int argc, char* argv[]) while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL)) != -1) { uint64_t jobid; + int had_index; xce_job_t* job; int rc = 0; switch (opt) { case 'j': ngiven++; - parse_jobid(&jobid, optarg); - if (jobid == 0) { - /* bug-1709: a malformed jobid used to be skipped silently */ - fprintf(stderr, "jobinfo: invalid jobid '%s'\n", optarg); + /* bug-1709: a malformed jobid used to be skipped silently. + * Each -j is judged on its own (job-array-one-based-index + * Terms bullet): a rejected one fails only itself. */ + if (job_arg_or_report("jobinfo", optarg, &jobid, &had_index) < 0) { lastrc = -EINVAL; continue; } @@ -3276,8 +3322,20 @@ int cmd_jobinfo(int argc, char* argv[]) job = call->job; free(call); + /* Req 4.7, 5.13: the controller answers an unknown job + * with rc 0 and no row -- that is "not found", not + * success; each -j is still judged on its own */ + rc = cmdutil_job_lookup_report(stderr, "jobinfo", rc, job, + optarg, had_index); if (rc < 0) { - cmdutil_rpc_error("xcecli_jobinfo", rc); + if (rc != -ENOENT) { + cmdutil_rpc_error("xcecli_jobinfo", rc); + } else { + errno = ENOENT; + } + if (job != NULL) { + free_Job_ptr(&job); + } lastrc = rc; continue; } @@ -3377,6 +3435,8 @@ int cmd_jobrun(int argc, char* argv[]) int nhosts = 0; char* host; int count; + const char* jobarg = NULL; + int had_index = 0; struct option long_opts[JOBRUN_NFLAGS + 1]; char optstring[16]; @@ -3431,11 +3491,14 @@ int cmd_jobrun(int argc, char* argv[]) nhosts++; break; case 'j': - parse_jobid(&jobid, optarg); + if (job_arg_or_report("jobrun", optarg, &jobid, &had_index) < 0) { + return -EINVAL; + } if (jobid == 0) { fprintf(stderr, "you need to specify a positive jobid with -j"); return -1; } + jobarg = optarg; break; case '?': return flagspec_reject( @@ -3477,7 +3540,7 @@ int cmd_jobrun(int argc, char* argv[]) } if (rc < 0) { - cmdutil_rpc_error("xce_jobdispatch", rc); + job_rpc_error("jobrun", "xce_jobdispatch", rc, jobarg, had_index); return rc; } @@ -3757,6 +3820,11 @@ cleanup: FREEUP(submit.start_info->execcwd); FREEUP(submit.start_info); + if ((rc < 0) && refused) { + SET_ERROR(-rc); + return rc; + } + /* Report success only on a clean ack that echoes back the EXACT JobId we * submitted (bug-786, Req 7.1/7.2/7.3). xce_jobsubmit() writes *jobid only * when the reply status is >= 0; XCECLI_FAILOVER_CALL surfaces a non-clean @@ -3776,11 +3844,6 @@ cleanup: * reply.data.jobid = job_submit() = base_id = submit->jobid; verified * round-trip). `jobid != jobsubmit.jobid` covers BOTH the never-written * (zero) case AND the stale non-zero garbage case in one uint64 compare. */ - if ((rc < 0) && refused) { - SET_ERROR(-rc); - return rc; - } - if (rc < 0) { char ebuf[128]; @@ -4076,6 +4139,8 @@ int cmd_jobkill(int argc, char* argv[]) xce_jobctrl_t jobctrl; xce_signal_t siginfo; char jobidStr[64]; + const char* jobarg = NULL; + int had_index = 0; struct option long_opts[JOBKILL_NFLAGS + 1]; char optstring[16]; @@ -4109,7 +4174,10 @@ int cmd_jobkill(int argc, char* argv[]) break; } case 'j': - parse_jobid(&jobid, optarg); + if (job_arg_or_report("jobkill", optarg, &jobid, &had_index) < 0) { + return -EINVAL; + } + jobarg = optarg; break; case 'y': yes = 1; @@ -4175,7 +4243,7 @@ int cmd_jobkill(int argc, char* argv[]) } if (rc < 0) { - cmdutil_rpc_error("xcecli_jobctrl", rc); + job_rpc_error("jobkill", "xcecli_jobctrl", rc, jobarg, had_index); return rc; } @@ -4223,6 +4291,8 @@ int cmd_jobckpt(int argc, char* argv[]) xce_jobctrl_t jobctrl; xce_ckpt_options_t ckptoption; char jobidStr[64]; + const char* jobarg = NULL; + int had_index = 0; struct option long_opts[JOBCKPT_NFLAGS + 1]; char optstring[16]; @@ -4245,7 +4315,10 @@ int cmd_jobckpt(int argc, char* argv[]) ckptoption.kill = 1; break; case 'j': - parse_jobid(&jobid, optarg); + if (job_arg_or_report("jobckpt", optarg, &jobid, &had_index) < 0) { + return -EINVAL; + } + jobarg = optarg; break; case 'm': ckptoption.method = optarg; @@ -4292,7 +4365,7 @@ int cmd_jobckpt(int argc, char* argv[]) } if (rc < 0) { - cmdutil_rpc_error("xcecli_jobctrl", rc); + job_rpc_error("jobckpt", "xcecli_jobctrl", rc, jobarg, had_index); return rc; } @@ -4820,7 +4893,9 @@ int cmd_runtasks(int argc, char* argv[]) char* host = NULL; char* launch_type = NULL; int i; - uint64_t jobid = 0; + uint64_t jobid = 0; + const char* jobarg = NULL; + int had_index = 0; struct option long_opts[RUNTASKS_NFLAGS + 1]; char optstring[32]; @@ -4890,7 +4965,19 @@ int cmd_runtasks(int argc, char* argv[]) dir = strdup(optarg); break; case 'j': - parse_jobid(&jobid, optarg); + /* a rejected -j used to leave jobid at 0 and send job 0 */ + if (job_arg_or_report("taskrun", optarg, &jobid, &had_index) < 0) { + for (i = 0; i < nhosts; i++) { + free(hosts[i].host); + } + free(user); + free(cmdline); + free(exdata); + free(dir); + free(launch_type); + return -EINVAL; + } + jobarg = optarg; break; case 't': launch_type = strdup(optarg); @@ -4948,7 +5035,9 @@ int cmd_runtasks(int argc, char* argv[]) } } - if (rc < 0) { + if ((rc < 0) && (jobarg != NULL)) { + job_rpc_error("taskrun", "xcecli_runtask", rc, jobarg, had_index); + } else if (rc < 0) { cmdutil_rpc_error("xcecli_runtask", rc); } else { printf("run task ok\n"); @@ -5017,6 +5106,8 @@ static int setpend_thunk(void* ctxp) int cmd_setpend(int argc, char* argv[]) { + const char* jobarg = NULL; + int had_index = 0; int opt; xce_job_preason_t jreasons; xce_pendreason_t preason = { .reasons = &jreasons }; @@ -5042,7 +5133,12 @@ int cmd_setpend(int argc, char* argv[]) int rc = 0; switch (opt) { case 'j': - parse_jobid(&(preason.jobid), optarg); + if (job_arg_or_report("setpend", optarg, &(preason.jobid), + &had_index) + < 0) { + return -EINVAL; + } + jobarg = optarg; break; case 'p': rc = add_pendreason(&jreasons, optarg); @@ -5101,7 +5197,8 @@ int cmd_setpend(int argc, char* argv[]) } if (rc < 0) { - cmdutil_rpc_error("xcecli_set_pend_reason", rc); + job_rpc_error("setpend", "xcecli_set_pend_reason", rc, jobarg, + had_index); erase_job_preason(&jreasons); return rc; } @@ -5212,6 +5309,8 @@ static int jobstart_thunk(void* ctxp) int cmd_jobstart(int argc, char* argv[]) { + const char* jobarg = NULL; + int had_index = 0; int rc; int opt; uint64_t jobid = 0; @@ -5237,7 +5336,10 @@ int cmd_jobstart(int argc, char* argv[]) != -1) { switch (opt) { case 'j': - parse_jobid(&jobid, optarg); + if (job_arg_or_report("jstart", optarg, &jobid, &had_index) < 0) { + return -EINVAL; + } + jobarg = optarg; break; case 'p': pid = (unsigned int)atoi(optarg); @@ -5283,7 +5385,7 @@ int cmd_jobstart(int argc, char* argv[]) } if (rc < 0) { - cmdutil_rpc_error("xcecli_jobstart", rc); + job_rpc_error("jstart", "xcecli_jobstart", rc, jobarg, had_index); return rc; } @@ -5314,6 +5416,8 @@ static int jobfinish_thunk(void* ctxp) int cmd_jobfinish(int argc, char* argv[]) { + const char* jobarg = NULL; + int had_index = 0; int rc; int opt; uint64_t jobid = 0; @@ -5346,7 +5450,10 @@ int cmd_jobfinish(int argc, char* argv[]) != -1) { switch (opt) { case 'j': - parse_jobid(&jobid, optarg); + if (job_arg_or_report("jfinish", optarg, &jobid, &had_index) < 0) { + return -EINVAL; + } + jobarg = optarg; break; case 'c': code = atoi(optarg); @@ -5385,7 +5492,7 @@ int cmd_jobfinish(int argc, char* argv[]) } if (rc < 0) { - cmdutil_rpc_error("xcecli_jobfinish", rc); + job_rpc_error("jfinish", "xcecli_jobfinish", rc, jobarg, had_index); return rc; } @@ -5425,6 +5532,8 @@ static int jobfail_thunk(void* ctxp) int cmd_jobfail(int argc, char* argv[]) { + const char* jobarg = NULL; + int had_index = 0; int rc; int opt; uint64_t jobid = 0; @@ -5450,7 +5559,10 @@ int cmd_jobfail(int argc, char* argv[]) != -1) { switch (opt) { case 'j': - parse_jobid(&jobid, optarg); + if (job_arg_or_report("jfail", optarg, &jobid, &had_index) < 0) { + return -EINVAL; + } + jobarg = optarg; break; case 'c': code = atoi(optarg); @@ -5494,7 +5606,7 @@ int cmd_jobfail(int argc, char* argv[]) } if (rc < 0) { - cmdutil_rpc_error("xcecli_jobfail", rc); + job_rpc_error("jfail", "xcecli_jobfail", rc, jobarg, had_index); return rc; } diff --git a/src/harness/testsuite/test_cmdutil.c b/src/harness/testsuite/test_cmdutil.c index be9d625b..752ada1c 100644 --- a/src/harness/testsuite/test_cmdutil.c +++ b/src/harness/testsuite/test_cmdutil.c @@ -14,6 +14,8 @@ #include #include "cmdutil.h" +#include "config.h" /* TAG_JOB_ARRAY */ +#include "utils.h" /* MAKE_JOBID, jobsel_strerror */ #include "xce/errnox.h" void setUp(void) {} @@ -192,6 +194,251 @@ static void test_parse_array_size_rejects_above_limit(void) TEST_ASSERT_EQUAL_UINT32(77, n); } +/* ── cmdutil_job_arg / cmdutil_job_not_found (job-array-one-based-index + * Req 4.7, 4.8, 4.9, 5.7, 5.8, 5.13, 5.14, 5.15, 8.2) ─────────────────── */ + +#define SINGLE_ELEMENT_MSG "takes a single element [k]" + +static void test_job_arg_bare_identity_trimmed_exact_id(void) +{ + uint64_t jobid = 77; + int had_index = 77; + char err[256] = ""; + + TEST_ASSERT_EQUAL_INT(0, cmdutil_job_arg(" \t1-2-3-5\t ", &jobid, + &had_index, err, sizeof(err))); + TEST_ASSERT_EQUAL_UINT64(MAKE_JOBID(1, 2, 3, 0x0, 5), jobid); + TEST_ASSERT_EQUAL_INT(0, had_index); +} + +static void test_job_arg_element_gives_element_id(void) +{ + uint64_t jobid = 77; + int had_index = 0; + char err[256] = ""; + + TEST_ASSERT_EQUAL_INT(0, cmdutil_job_arg("1-2-3-32768[2]", &jobid, + &had_index, err, sizeof(err))); + TEST_ASSERT_EQUAL_UINT64(MAKE_JOBID(1, 2, 3, TAG_JOB_ARRAY, 32768 | 2), + jobid); + TEST_ASSERT_EQUAL_INT(1, had_index); + + /* leading zeros read as the same number (1.5) */ + TEST_ASSERT_EQUAL_INT(0, cmdutil_job_arg("1-2-3-32768[002]", &jobid, + &had_index, err, sizeof(err))); + TEST_ASSERT_EQUAL_UINT64(MAKE_JOBID(1, 2, 3, TAG_JOB_ARRAY, 32768 | 2), + jobid); +} + +/* 8.2: a pre-upgrade element 0 is a single element, never the whole array */ +static void test_job_arg_element_zero_accepted(void) +{ + uint64_t jobid = 77; + int had_index = 0; + char err[256] = ""; + + TEST_ASSERT_EQUAL_INT(0, cmdutil_job_arg("1-2-3-32768[0]", &jobid, + &had_index, err, sizeof(err))); + TEST_ASSERT_EQUAL_UINT64(MAKE_JOBID(1, 2, 3, TAG_JOB_ARRAY, 32768), jobid); + TEST_ASSERT_TRUE(jobid != MAKE_JOBID(1, 2, 3, 0x0, 32768)); + TEST_ASSERT_EQUAL_INT(1, had_index); +} + +/* 4.8: ranges -- reversed ones included -- are refused with the + * single-element message and leave the outputs untouched */ +static void test_job_arg_ranges_refused_single_element(void) +{ + static const char* ranges[] + = { "1-1-1-0[1-3]", "1-1-1-0[1-1]", "1-1-1-0[0-0]", "1-1-1-0[3-1]", + " 1-1-1-32768[2-5] " }; + + for (size_t i = 0; i < sizeof(ranges) / sizeof(ranges[0]); i++) { + uint64_t jobid = 77; + int had_index = 77; + char err[256] = ""; + + TEST_ASSERT_EQUAL_INT_MESSAGE( + -EINVAL, + cmdutil_job_arg(ranges[i], &jobid, &had_index, err, sizeof(err)), + ranges[i]); + TEST_ASSERT_NOT_NULL_MESSAGE(strstr(err, SINGLE_ELEMENT_MSG), + ranges[i]); + TEST_ASSERT_EQUAL_UINT64(77, jobid); + TEST_ASSERT_EQUAL_INT(77, had_index); + } +} + +/* 5.15 / 5.14 / 5.8 / 5.7: invalid identities are rejected with the libxce + * reason, the argument named, and never the single-element message */ +static void test_job_arg_invalid_identities_rejected(void) +{ + static const struct { + const char* arg; + jobsel_err_t why; + } cases[] = { + { "garbage", JOBSEL_ESYNTAX }, + { "", JOBSEL_ESYNTAX }, + { "12345", JOBSEL_ESYNTAX }, + { "1-1-1-0 [1]", JOBSEL_ESYNTAX }, + { "1-1-1-0[a]", JOBSEL_ESYNTAX }, + { "1-1-1-0[-1]", JOBSEL_ESYNTAX }, + { "1-1-1-0[+1]", JOBSEL_ESYNTAX }, + { "1-1-1-0[ 1]", JOBSEL_ESYNTAX }, + { "1-1-1-0[1", JOBSEL_ESYNTAX }, + { "1-1-1-0[]", JOBSEL_ESYNTAX }, + { "1-1-1-0[1]x", JOBSEL_ESYNTAX }, + { "64-0-0-0", JOBSEL_EWIDTH }, + { "0-0-0-1048576[1]", JOBSEL_EWIDTH }, + { "1-1-1-5[1]", JOBSEL_EALIGN }, + { "1-1-1-0[40000]", JOBSEL_EINDEX }, + { "1-1-1-0[1-40000]", JOBSEL_EINDEX }, + }; + + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + uint64_t jobid = 77; + int had_index = 77; + char err[512] = ""; + + TEST_ASSERT_EQUAL_INT_MESSAGE( + -EINVAL, + cmdutil_job_arg(cases[i].arg, &jobid, &had_index, err, sizeof(err)), + cases[i].arg); + TEST_ASSERT_NOT_NULL_MESSAGE(strstr(err, jobsel_strerror(cases[i].why)), + cases[i].arg); + TEST_ASSERT_NOT_NULL_MESSAGE(strstr(err, "invalid jobid"), + cases[i].arg); + TEST_ASSERT_NULL_MESSAGE(strstr(err, SINGLE_ELEMENT_MSG), cases[i].arg); + TEST_ASSERT_EQUAL_UINT64(77, jobid); + TEST_ASSERT_EQUAL_INT(77, had_index); + } +} + +static void test_job_arg_null_and_tiny_buffers_are_safe(void) +{ + uint64_t jobid = 77; + int had_index = 77; + char tiny[8]; + + TEST_ASSERT_EQUAL_INT( + -EINVAL, cmdutil_job_arg(NULL, &jobid, &had_index, tiny, sizeof(tiny))); + TEST_ASSERT_EQUAL_UINT(sizeof(tiny) - 1, strlen(tiny)); + TEST_ASSERT_EQUAL_INT( + -EINVAL, cmdutil_job_arg("1-1-1-0[1-3]", &jobid, &had_index, NULL, 0)); + TEST_ASSERT_EQUAL_INT( + -EINVAL, cmdutil_job_arg("1-1-1-0", NULL, &had_index, NULL, 0)); + TEST_ASSERT_EQUAL_UINT64(77, jobid); +} + +static char* capture_not_found(const char* arg, int had_index) +{ + char* buf = NULL; + size_t len = 0; + FILE* fp = open_memstream(&buf, &len); + + TEST_ASSERT_NOT_NULL(fp); + cmdutil_job_not_found(fp, arg, had_index); + fclose(fp); + return buf; +} + +/* 4.7: an identity without [k] gets the [k] hint */ +static void test_job_not_found_bare_names_element_form(void) +{ + char* text = capture_not_found("1-1-1-32768", 0); + + TEST_ASSERT_NOT_NULL(strstr(text, "job 1-1-1-32768 not found")); + TEST_ASSERT_NOT_NULL( + strstr(text, "array elements must be named as [k]")); + free(text); +} + +/* 5.13: a missing element is just not found -- no hint */ +static void test_job_not_found_element_has_no_hint(void) +{ + char* text = capture_not_found("1-1-1-32768[4]", 1); + + TEST_ASSERT_NOT_NULL(strstr(text, "job 1-1-1-32768[4] not found")); + TEST_ASSERT_NULL(strstr(text, "[k]")); + free(text); +} + +/* ── cmdutil_job_lookup_report (job-array-one-based-index Req 4.7, 5.13): + * the controller answers a lookup of an unknown job with rc 0 and no + * row, which must be reported as not found, never as success ─────────── */ + +static char* capture_lookup(int rc, const void* job, const char* arg, + int had_index, int* ret) +{ + char* buf = NULL; + size_t len = 0; + FILE* fp = open_memstream(&buf, &len); + + TEST_ASSERT_NOT_NULL(fp); + *ret = cmdutil_job_lookup_report(fp, "jobinfo", rc, job, arg, had_index); + fclose(fp); + return buf; +} + +/* 4.7: rc 0 with no row for a bare identity is not found, with the hint */ +static void test_job_lookup_no_row_bare_is_not_found_with_hint(void) +{ + int ret = 77; + char* text = capture_lookup(0, NULL, "1-1-1-32768", 0, &ret); + + TEST_ASSERT_EQUAL_INT(-ENOENT, ret); + TEST_ASSERT_NOT_NULL(strstr(text, "jobinfo: job 1-1-1-32768 not found")); + TEST_ASSERT_NOT_NULL( + strstr(text, "array elements must be named as [k]")); + free(text); +} + +/* 5.13: rc 0 with no row for a missing element is not found, no hint */ +static void test_job_lookup_no_row_element_is_not_found_without_hint(void) +{ + int ret = 77; + char* text = capture_lookup(0, NULL, "1-1-1-32768[4]", 1, &ret); + + TEST_ASSERT_EQUAL_INT(-ENOENT, ret); + TEST_ASSERT_NOT_NULL(strstr(text, "jobinfo: job 1-1-1-32768[4] not found")); + TEST_ASSERT_NULL(strstr(text, "[k]")); + free(text); +} + +/* an explicit -ENOENT from the RPC is the same not-found report */ +static void test_job_lookup_enoent_is_not_found(void) +{ + int ret = 77; + char* text = capture_lookup(-ENOENT, NULL, "1-1-1-32768", 0, &ret); + + TEST_ASSERT_EQUAL_INT(-ENOENT, ret); + TEST_ASSERT_NOT_NULL(strstr(text, "job 1-1-1-32768 not found")); + free(text); +} + +/* any other rc < 0 is an RPC error: returned as is, left to the caller to + * report, and never called "not found" */ +static void test_job_lookup_rpc_error_is_error(void) +{ + int ret = 77; + char* text = capture_lookup(-ETIMEDOUT, NULL, "1-1-1-32768", 0, &ret); + + TEST_ASSERT_EQUAL_INT(-ETIMEDOUT, ret); + TEST_ASSERT_NULL(strstr(text, "not found")); + free(text); +} + +/* a returned row is found, silently */ +static void test_job_lookup_row_is_found(void) +{ + int ret = 77; + int row = 1; + char* text = capture_lookup(0, &row, "1-1-1-32768[4]", 1, &ret); + + TEST_ASSERT_EQUAL_INT(0, ret); + TEST_ASSERT_EQUAL_STRING("", text); + free(text); +} + /* ── cmdutil_default_exdata (bug-1705 jobsub -x) ────────────────────────── */ static void test_default_exdata_keeps_caller_value(void) @@ -254,6 +501,19 @@ int main(void) RUN_TEST(test_parse_array_size_accepts_digits); RUN_TEST(test_parse_array_size_rejects_non_digits_and_zero); RUN_TEST(test_parse_array_size_rejects_above_limit); + RUN_TEST(test_job_arg_bare_identity_trimmed_exact_id); + RUN_TEST(test_job_arg_element_gives_element_id); + RUN_TEST(test_job_arg_element_zero_accepted); + RUN_TEST(test_job_arg_ranges_refused_single_element); + RUN_TEST(test_job_arg_invalid_identities_rejected); + RUN_TEST(test_job_arg_null_and_tiny_buffers_are_safe); + RUN_TEST(test_job_not_found_bare_names_element_form); + RUN_TEST(test_job_not_found_element_has_no_hint); + RUN_TEST(test_job_lookup_no_row_bare_is_not_found_with_hint); + RUN_TEST(test_job_lookup_no_row_element_is_not_found_without_hint); + RUN_TEST(test_job_lookup_enoent_is_not_found); + RUN_TEST(test_job_lookup_rpc_error_is_error); + RUN_TEST(test_job_lookup_row_is_found); RUN_TEST(test_default_exdata_keeps_caller_value); RUN_TEST(test_default_exdata_builds_path_json_when_unset); RUN_TEST(test_errstr_reports_the_rpc_code_not_errno); diff --git a/src/harness/testsuite/test_test_cli.c b/src/harness/testsuite/test_test_cli.c index 12cc6189..c09972b5 100644 --- a/src/harness/testsuite/test_test_cli.c +++ b/src/harness/testsuite/test_test_cli.c @@ -1352,7 +1352,10 @@ static void test_migrated_commands_sample_reject_unrecognized_flag(void) * 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")); + /* "1-1-1-1", not "1": a bare decimal is itself rejected by -j's own + * identity check (job-array-one-based-index Req 5.15) before getopt + * ever reaches -z. */ + TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "jobkill -j 1-1-1-1 -z")); TEST_ASSERT_TRUE_MESSAGE( clitest_wait_for_text(&sess, &out, &err, "jobkill: unrecognized flag '-z'", true, 5000), @@ -1382,8 +1385,8 @@ static void test_migrated_commands_sample_reject_unrecognized_flag(void) 5000), "expected to enter the scheduler context"); - TEST_ASSERT_EQUAL_INT(0, - clitest_send_line(&sess, "setpend -j 1 --bogus")); + TEST_ASSERT_EQUAL_INT( + 0, clitest_send_line(&sess, "setpend -j 1-1-1-1 --bogus")); TEST_ASSERT_TRUE_MESSAGE( clitest_wait_for_text(&sess, &out, &err, "setpend: unrecognized flag '--bogus'", true, @@ -2319,13 +2322,12 @@ static void test_destructive_commands_prompt_by_default_and_yes_bypasses( "expected to enter the client context"); /* jobkill: unconditionally destructive -- prompts by default. - * "1-1-1-1" (not a bare "1") is required: parse_jobid() only accepts - * the dash-delimited cluster-apiserver-timestamp-job encoding - * (REGEX_JOBID, config.h) or its bracketed array-index form - * (REGEX_JOBARRAY_ID) -- a bare decimal never matches either regex and - * silently leaves jobid at 0, which would hit cmd_jobkill's OWN "you - * need to specify jobid with -j" rejection before ever reaching the - * confirmation gate under test here. */ + * "1-1-1-1" (not a bare "1") is required: cmd_jobkill parses -j with + * cmdutil_job_arg() (parse_jobsel(), libxce), which only accepts the + * dash-delimited cluster-apiserver-timestamp-sequence form, optionally + * followed by [k] -- a bare decimal is rejected as an invalid job + * identity before ever reaching the confirmation gate under test + * here. */ TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "jobkill -j 1-1-1-1")); TEST_ASSERT_TRUE_MESSAGE( clitest_wait_for_text(&sess, &out, &err, "About to kill job", true, @@ -3631,6 +3633,86 @@ static void test_jobinfo_without_valid_jobid_fails(void) batch_free(&out, &err); } +/* job-array-one-based-index Req 4.8: a single-job command given a range of + * elements is refused with the single-element message before any RPC (and, + * for jobkill, before the confirmation prompt). */ +static void test_jobkill_element_range_refused_before_rpc(void) +{ + clitest_buf_t out, err; + + TEST_ASSERT_EQUAL_INT( + 1, batch_run("client", "jobkill -j 1-1-1-0[1-3]", &out, &err)); + TEST_ASSERT_NOT_NULL_MESSAGE( + strstr(err.data, "takes a single element [k]"), + "a range must be refused with the 4.8 single-element message"); + TEST_ASSERT_NULL_MESSAGE(strstr(err.data, "About to"), + "a refused -j must not reach the prompt"); + TEST_ASSERT_NULL_MESSAGE(strstr(err.data, "xcecli_jobctrl"), + "a refused -j must not reach the RPC"); + batch_free(&out, &err); +} + +/* Req 4.8, 5.15: every single-job command stops on a rejected argument + * instead of sending job 0 (taskrun had no jobid == 0 guard at all). */ +static void test_single_job_commands_stop_on_rejected_jobid(void) +{ + static const struct { + const char* ctx; + const char* line; + const char* rpc; + } cases[] = { + { "client", "taskrun -j 1-1-1-0[1-3] -m h -c ls", "xcecli_runtask" }, + { "client", "taskrun -j 1-1-1-0x -m h -c ls", "xcecli_runtask" }, + { "client", "jobrun -j 1-1-1-0[2-1] -m h", "xce_jobdispatch" }, + { "client", "jobckpt -j 1-1-1-5[1]", "xcecli_jobctrl" }, + { "worker", "jstart -j 1-1-1-0[1-3]", "xcecli_jobstart" }, + }; + + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + clitest_buf_t out, err; + + TEST_ASSERT_EQUAL_INT_MESSAGE( + 1, batch_run(cases[i].ctx, cases[i].line, &out, &err), + cases[i].line); + TEST_ASSERT_TRUE_MESSAGE( + (strstr(err.data, "takes a single element [k]") != NULL) + || (strstr(err.data, "invalid jobid") != NULL), + cases[i].line); + TEST_ASSERT_NULL_MESSAGE(strstr(err.data, cases[i].rpc), cases[i].line); + batch_free(&out, &err); + } +} + +/* Req 4.8 / Terms: expect-job refuses a range without querying, and the + * assertion fails. */ +static void test_expect_job_element_range_refused(void) +{ + clitest_buf_t out, err; + + TEST_ASSERT_EQUAL_INT( + 1, batch_run("client", "expect-job 1-1-1-0[1-3]", &out, &err)); + TEST_ASSERT_NOT_NULL( + strstr(err.data, "takes a single element [k]")); + TEST_ASSERT_NULL_MESSAGE(strstr(err.data, "joblist query failed"), + "a refused argument must not query the job list"); + batch_free(&out, &err); +} + +/* Terms bullet: jobinfo judges each -j on its own -- a refused range does + * not stop the next -j from being looked up. */ +static void test_jobinfo_range_refused_per_jobid(void) +{ + clitest_buf_t out, err; + + TEST_ASSERT_EQUAL_INT( + 1, + batch_run("client", "jobinfo -j 1-1-1-0[1-3] -j garbage", &out, &err)); + TEST_ASSERT_NOT_NULL( + strstr(err.data, "takes a single element [k]")); + TEST_ASSERT_NOT_NULL(strstr(err.data, "jobinfo: invalid jobid 'garbage'")); + batch_free(&out, &err); +} + /* bug-1710: hello -c 0 sent nothing and still reported success. */ static void test_hello_nonpositive_count_rejected(void) { @@ -3734,6 +3816,10 @@ void test_test_cli(void) RUN_TEST(test_jobkill_non_numeric_signal_rejected); RUN_TEST(test_jobinfo_without_valid_jobid_fails); RUN_TEST(test_hello_nonpositive_count_rejected); + RUN_TEST(test_jobkill_element_range_refused_before_rpc); + RUN_TEST(test_single_job_commands_stop_on_rejected_jobid); + RUN_TEST(test_expect_job_element_range_refused); + RUN_TEST(test_jobinfo_range_refused_per_jobid); RUN_TEST(test_resupdate_trailing_name_without_value_warns); RUN_TEST(test_resupdate_duplicate_value_named_correctly); RUN_TEST(test_resupdate_rpc_failure_fails_the_session);