From 1812a80d19456301b00f5d22cca4f9de747e5c90 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 1 Oct 2026 10:56:15 +0000 Subject: [PATCH] feat(job-array-one-based-index): harness $jobid[...] substitution (task 4.2) Auto-variable substitution moves out of test.c into its own unit-tested module (autovars.c) with an explicit context for $jobid, $? and $jobcount, keeping today's token rule, messages and buffer handling. New: a token $jobid[...] expands $jobid and keeps the rest of the token, so $jobid[3] names element 3 of the array just submitted; it fails like an unset $jobid when none is set. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../specs/job-array-one-based-index/tasks.md | 2 +- src/harness/autovars.c | 126 +++++++++++ src/harness/include/autovars.h | 41 ++++ src/harness/meson.build | 17 ++ src/harness/test.c | 127 ++--------- src/harness/testsuite/test_autovars.c | 206 ++++++++++++++++++ 6 files changed, 411 insertions(+), 108 deletions(-) create mode 100644 src/harness/autovars.c create mode 100644 src/harness/include/autovars.h create mode 100644 src/harness/testsuite/test_autovars.c diff --git a/.kiro/specs/job-array-one-based-index/tasks.md b/.kiro/specs/job-array-one-based-index/tasks.md index 45b0e3a3..2a47c4ee 100644 --- a/.kiro/specs/job-array-one-based-index/tasks.md +++ b/.kiro/specs/job-array-one-based-index/tasks.md @@ -78,7 +78,7 @@ - _Requirements: 4.1, 4.2, 4.3, 4.4, 4.5, 4.6, 9.3_ - _Depends: 1.3_ -- [ ] 4.2 Substitute `$jobid[...]` in harness command lines +- [x] 4.2 Substitute `$jobid[...]` in harness command lines - Move the existing autovariable substitution into its own unit-testable module with an explicit context for `$jobid`, `$?` and `$jobcount`, keeping today's token rule, messages and buffer-size result - Expand `$jobid` followed by `[` keeping the rest of the token; fail as for an unset `$jobid` when it is unset; leave `!` and `#` lines untouched - Register the module in the harness build and its new unit test diff --git a/src/harness/autovars.c b/src/harness/autovars.c new file mode 100644 index 00000000..3fa81e7e --- /dev/null +++ b/src/harness/autovars.c @@ -0,0 +1,126 @@ +/* + * autovars.c -- xce_test auto-variable substitution (see include/autovars.h). + */ +#include +#include +#include + +#include "autovars.h" + +static void set_err(char* errbuf, size_t errlen, const char* msg) +{ + if ((errbuf != NULL) && (errlen > 0)) { + snprintf(errbuf, errlen, "%s", msg); + } +} + +int autovar_substitute(const char* line, const autovar_ctx_t* ctx, char* out, + size_t outlen, char* errbuf, size_t errlen) +{ + const char* p = line; + const char* first; + size_t o = 0; + + if ((errbuf != NULL) && (errlen > 0)) { + errbuf[0] = '\0'; + } + if ((line == NULL) || (ctx == NULL) || (out == NULL) || (outlen == 0)) { + return -EINVAL; + } + + /* `!` shell-escape and `#` comment lines are left untouched. */ + first = line; + while ((*first == ' ') || (*first == '\t')) { + first++; + } + if ((*first == '!') || (*first == '#')) { + size_t len = strlen(line); + + if (len >= outlen) { + return -ENOSPC; + } + memcpy(out, line, len + 1); + return 0; + } + + while (*p != '\0') { + /* A `$` starts a candidate auto-var only when it begins a token, i.e. + * the previous char is the line start or whitespace. */ + int at_token_start + = (p == line) || (*(p - 1) == ' ') || (*(p - 1) == '\t'); + + if ((*p == '$') && at_token_start) { + const char* name = p + 1; + const char* end = name; + const char* repl = NULL; + const char* stop; /* where the replaced text ends */ + char numbuf[32]; + + /* Scan to the token end (whitespace or NUL). */ + while ((*end != '\0') && (*end != ' ') && (*end != '\t')) { + end++; + } + + size_t namelen = (size_t)(end - name); + /* `$jobid[...]` (Req 4.10): only `$jobid` is replaced; the rest + * of the token, from the `[`, is copied unchanged. */ + int jobid_index + = (namelen > 5) && (strncmp(name, "jobid[", 6) == 0); + + stop = end; + if ((namelen == 1) && (name[0] == '?')) { + snprintf(numbuf, sizeof(numbuf), "%d", ctx->last_status); + repl = numbuf; + } else if (jobid_index + || ((namelen == 5) + && (strncmp(name, "jobid", 5) == 0))) { + /* Req 4.11: an unset $jobid fails `$jobid[...]` the same + * way as `$jobid`. */ + if ((ctx->jobid == NULL) || (ctx->jobid[0] == '\0')) { + set_err(errbuf, errlen, + "autovar: $jobid referenced before set"); + return -EINVAL; + } + repl = ctx->jobid; + stop = name + 5; + } else if ((namelen == 8) && (strncmp(name, "jobcount", 8) == 0)) { + if (ctx->jobcount < 0) { + set_err(errbuf, errlen, + "autovar: $jobcount referenced before set"); + return -EINVAL; + } + snprintf(numbuf, sizeof(numbuf), "%d", ctx->jobcount); + repl = numbuf; + } + + if (repl != NULL) { + size_t rlen = strlen(repl); + + if ((o + rlen) >= outlen) { + return -ENOSPC; + } + memcpy(out + o, repl, rlen); + o += rlen; + p = stop; /* consume `$name`; a `[...]` rest is copied */ + continue; + } + + /* Not a recognized auto-var: copy the whole token verbatim. */ + while (p < end) { + if ((o + 1) >= outlen) { + return -ENOSPC; + } + out[o++] = *p++; + } + continue; + } + + if ((o + 1) >= outlen) { + return -ENOSPC; + } + out[o++] = *p++; + } + + out[o] = '\0'; + return 0; +} diff --git a/src/harness/include/autovars.h b/src/harness/include/autovars.h new file mode 100644 index 00000000..1662cd9e --- /dev/null +++ b/src/harness/include/autovars.h @@ -0,0 +1,41 @@ +/* + * autovars.h -- xce_test auto-variable substitution (`$?`, `$jobid`, + * `$jobid[...]`, `$jobcount`), moved out of test.c's former + * substitute_autovars() so it can be unit-tested without the REPL + * (testsuite/test_autovars.c; job-array-one-based-index Req 4.10, 4.11). + */ +#ifndef AUTOVARS_H +#define AUTOVARS_H + +#include /* size_t */ + +/* The values the substitution reads (test.c fills it from its globals). */ +typedef struct { + const char* jobid; /* $jobid; NULL when unset */ + int last_status; /* $? */ + int jobcount; /* $jobcount; -1 when unset */ +} autovar_ctx_t; + +/* Substitute auto-variables in `line` into `out` (outlen bytes, always + * NUL-terminated on success). + * + * A `$` starts a candidate only at a token start (line start, or after a + * space or tab); the token runs to the next space, tab or end of line. + * Recognized tokens: + * $? -> decimal ctx->last_status + * $jobcount -> decimal ctx->jobcount (unset => -EINVAL) + * $jobid -> ctx->jobid (unset => -EINVAL) + * $jobid[...] -> ctx->jobid followed by the rest of the token unchanged + * (unset => the same -EINVAL as $jobid) + * Any other token, and a `$` glued to preceding text, is copied verbatim. + * A line whose first non-blank character is `!` (shell escape) or `#` + * (comment) is copied unchanged. + * + * Returns 0, -EINVAL (NULL argument, or an unset $jobid / $jobcount; the + * message, e.g. "autovar: $jobid referenced before set", is written into + * errbuf when errbuf/errlen allow), or -ENOSPC (out too small). On error the + * content of `out` is unspecified. */ +int autovar_substitute(const char* line, const autovar_ctx_t* ctx, char* out, + size_t outlen, char* errbuf, size_t errlen); + +#endif /* AUTOVARS_H */ diff --git a/src/harness/meson.build b/src/harness/meson.build index ce748f49..de8f37cf 100644 --- a/src/harness/meson.build +++ b/src/harness/meson.build @@ -27,6 +27,7 @@ xce_test_src = [ 'listfmt_json.c', 'listrow_json.c', 'cmdutil.c', + 'autovars.c', ] xce_test_deps = [ @@ -253,3 +254,19 @@ test('cmdutil test', cmdutil_unit_test, suite: 'harness', timeout: 60, env: ['ASAN_OPTIONS=detect_leaks=0']) + +# autovars.c: xce_test's `$?`/`$jobid`/`$jobid[...]`/`$jobcount` line +# substitution (job-array-one-based-index Req 4.10, 4.11), moved out of +# test.c so the token rule can be tested without the REPL -- same +# standalone-binary rationale as the tests above. +autovars_unit_test = executable('autovars_unit_test', + ['testsuite/test_autovars.c', 'autovars.c'], + include_directories: [incdir], + dependencies: harness_unit_test_deps, + c_args: ['-DXCE_TESTING'], + install: false) + +test('autovars test', autovars_unit_test, + suite: 'harness', + timeout: 60, + env: ['ASAN_OPTIONS=detect_leaks=0']) diff --git a/src/harness/test.c b/src/harness/test.c index d7382d93..9c6d112a 100644 --- a/src/harness/test.c +++ b/src/harness/test.c @@ -64,6 +64,7 @@ #include "configcompare.h" /* cross-member active-config comparator (cmd_configcompare) */ #include "confirm.h" /* destructive-command confirmation gate (xce-test-cli-ux task 2.1) */ #include "cmdutil.h" /* argument parsing + RPC-error reporting helpers */ +#include "autovars.h" /* $?, $jobid, $jobid[...], $jobcount substitution */ #include "joblist_display.h" /* worker_status_str/job_state_str/format_job_time * (xce-test-cli-ux task 2.5 remediation) */ #include "join_worker_args.h" /* join_worker_parse_attrs (xce-test-cli-ux @@ -1266,105 +1267,6 @@ int execute_line(char* line) return rc; } -/* Substitute the fixed auto-variables `$?`, `$jobid`, `$jobcount` on a raw line - * before tokenizing (xce-test-e2e-primitives task 3.1, Req 3.1/3.3/3.5/3.6). - * - * Only WHOLE-TOKEN occurrences are recognized: a `$name` must be bounded by the - * start of the line / whitespace on its left and whitespace / end-of-line on its - * right. A `$name` glued to other non-whitespace (e.g. `x$jobid`, `$jobidx`) is - * NOT a recognized auto-variable and is copied through verbatim. No other `$...` - * syntax is recognized: an unknown whole token such as `$other` is also copied - * verbatim (it is simply not one of the three names). - * - * Replacements: - * $? -> decimal of g_last_status (always available). - * $jobid -> g_last_jobid if set (non-empty); unset => command failure. - * $jobcount -> decimal of g_last_jobcount if set (>=0); unset => failure. - * - * Writes the expanded line into `out` (capacity `outsz`, always NUL-terminated - * on success). Returns 0 on success, -EINVAL on an unset auto-var reference, - * -ENOSPC if the expansion would overflow `out`. On any error `out` content is - * unspecified and the caller must fail the command rather than run it. */ -static int substitute_autovars(const char* line, char* out, size_t outsz) -{ - const char* p = line; - size_t o = 0; - - if ((line == NULL) || (out == NULL) || (outsz == 0)) { - return -EINVAL; - } - - while (*p != '\0') { - /* A `$` starts a candidate auto-var only when it begins a token, i.e. - * the previous char is the line start or whitespace. */ - int at_token_start - = (p == line) || (*(p - 1) == ' ') || (*(p - 1) == '\t'); - - if ((*p == '$') && at_token_start) { - const char* name = p + 1; - const char* end = name; - const char* repl = NULL; - char numbuf[32]; - - /* Scan to the token end (whitespace or NUL). */ - while ((*end != '\0') && (*end != ' ') && (*end != '\t')) { - end++; - } - - size_t namelen = (size_t)(end - name); - - if ((namelen == 1) && (name[0] == '?')) { - snprintf(numbuf, sizeof(numbuf), "%d", g_last_status); - repl = numbuf; - } else if ((namelen == 5) && (strncmp(name, "jobid", 5) == 0)) { - if (g_last_jobid[0] == '\0') { - fprintf(stderr, - "autovar: $jobid referenced before set\n"); - return -EINVAL; - } - repl = g_last_jobid; - } else if ((namelen == 8) && (strncmp(name, "jobcount", 8) == 0)) { - if (g_last_jobcount < 0) { - fprintf(stderr, - "autovar: $jobcount referenced before set\n"); - return -EINVAL; - } - snprintf(numbuf, sizeof(numbuf), "%d", g_last_jobcount); - repl = numbuf; - } - - if (repl != NULL) { - size_t rlen = strlen(repl); - - if ((o + rlen) >= outsz) { - return -ENOSPC; - } - memcpy(out + o, repl, rlen); - o += rlen; - p = end; /* consume the whole `$name` token */ - continue; - } - - /* Not a recognized auto-var: copy the whole token verbatim. */ - while (p < end) { - if ((o + 1) >= outsz) { - return -ENOSPC; - } - out[o++] = *p++; - } - continue; - } - - if ((o + 1) >= outsz) { - return -ENOSPC; - } - out[o++] = *p++; - } - - out[o] = '\0'; - return 0; -} - /* Main-loop wrapper around execute_line that implements the session verdict * (xce-test-e2e-primitives task 1.1): * - blank / whitespace-only lines and `#`-comment lines are ignored; @@ -1428,18 +1330,29 @@ int preprocess_and_run(char* line) return 0; } - /* Substitute the fixed auto-variables (`$?`, `$jobid`, `$jobcount`) before - * tokenizing (xce-test-e2e-primitives task 3.1). An unset `$jobid` / - * `$jobcount` reference (Req 3.6) — or an overflow — fails the command: - * we do NOT run execute_line, and the result is treated exactly like a - * failing command (g_last_status non-zero, g_session_failed, abort vs - * continue). The leading `!` shell-escape above is intentionally NOT + /* Substitute the fixed auto-variables (`$?`, `$jobid`, `$jobid[...]`, + * `$jobcount`; autovars.c) before tokenizing (xce-test-e2e-primitives + * task 3.1, job-array-one-based-index Req 4.10). An unset `$jobid` / + * `$jobcount` reference (Req 3.6, 4.11) — or an overflow — fails the + * command: we do NOT run execute_line, and the result is treated exactly + * like a failing command (g_last_status non-zero, g_session_failed, abort + * vs continue). The leading `!` shell-escape above is intentionally NOT * substituted: that line goes raw to system() and the shell owns `$?`. */ { - char expanded[MAX_LINE_LEN]; - int subrc = substitute_autovars(p, expanded, sizeof(expanded)); + char expanded[MAX_LINE_LEN]; + char suberr[128]; + autovar_ctx_t actx = { + .jobid = (g_last_jobid[0] != '\0') ? g_last_jobid : NULL, + .last_status = g_last_status, + .jobcount = g_last_jobcount, + }; + int subrc = autovar_substitute(p, &actx, expanded, sizeof(expanded), + suberr, sizeof(suberr)); if (subrc < 0) { + if (suberr[0] != '\0') { + fprintf(stderr, "%s\n", suberr); + } g_last_status = subrc; g_session_failed = 1; if (!g_continue_on_error && !g_interactive) { diff --git a/src/harness/testsuite/test_autovars.c b/src/harness/testsuite/test_autovars.c new file mode 100644 index 00000000..66bbfc02 --- /dev/null +++ b/src/harness/testsuite/test_autovars.c @@ -0,0 +1,206 @@ +/* + * test_autovars.c -- unit tests for autovars.c, xce_test's auto-variable + * substitution (job-array-one-based-index Req 4.10, 4.11; behaviour for + * `$?`, `$jobid`, `$jobcount` preserved from xce-test-e2e-primitives 3.1). + */ +#include +#include + +#include + +#include "autovars.h" + +void setUp(void) { } +void tearDown(void) { } + +static const autovar_ctx_t ctx_set = { "1-2-3-1024", 7, 4 }; +static const autovar_ctx_t ctx_unset = { NULL, 0, -1 }; + +static int subst(const char* line, const autovar_ctx_t* ctx, char* out, + size_t outlen, char* err, size_t errlen) +{ + memset(out, 'Z', outlen); + return autovar_substitute(line, ctx, out, outlen, err, errlen); +} + +/* ── Req 4.10: $jobid[...] ──────────────────────────────────────────── */ + +static void test_jobid_index_expands_and_keeps_suffix(void) +{ + char out[256]; + char err[128]; + + TEST_ASSERT_EQUAL_INT(0, subst("jobkill -j $jobid[3]", &ctx_set, out, + sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("jobkill -j 1-2-3-1024[3]", out); +} + +static void test_jobid_index_invalid_remainder_copied_unchanged(void) +{ + char out[256]; + char err[128]; + + TEST_ASSERT_EQUAL_INT(0, subst("jobinfo -j $jobid[3]x -v", &ctx_set, out, + sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("jobinfo -j 1-2-3-1024[3]x -v", out); +} + +static void test_jobid_index_at_line_start_and_after_tab(void) +{ + char out[256]; + char err[128]; + + TEST_ASSERT_EQUAL_INT(0, subst("$jobid[1]\t$jobid[2-5]", &ctx_set, out, + sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("1-2-3-1024[1]\t1-2-3-1024[2-5]", out); +} + +/* ── Req 4.11: $jobid[...] while $jobid is unset ──────────────────────── */ + +static void test_jobid_index_unset_fails_like_jobid(void) +{ + char out[256]; + char err[128]; + const autovar_ctx_t empty = { "", 0, -1 }; + + TEST_ASSERT_EQUAL_INT(-EINVAL, subst("jobkill -j $jobid[3]", &ctx_unset, + out, sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("autovar: $jobid referenced before set", err); + + TEST_ASSERT_EQUAL_INT(-EINVAL, subst("jobkill -j $jobid[3]", &empty, out, + sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("autovar: $jobid referenced before set", err); +} + +/* ── Preserved behaviour ─────────────────────────────────────────────── */ + +static void test_jobid_whole_token(void) +{ + char out[256]; + char err[128]; + + TEST_ASSERT_EQUAL_INT(0, subst("expect-job $jobid done", &ctx_set, out, + sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("expect-job 1-2-3-1024 done", out); +} + +static void test_jobid_unset_fails(void) +{ + char out[256]; + char err[128]; + + TEST_ASSERT_EQUAL_INT(-EINVAL, subst("expect-job $jobid", &ctx_unset, out, + sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("autovar: $jobid referenced before set", err); +} + +static void test_jobid_glued_tokens_copied_verbatim(void) +{ + char out[256]; + char err[128]; + + TEST_ASSERT_EQUAL_INT(0, + subst("echo x$jobid $jobidx x$jobid[3] $other", + &ctx_set, out, sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("echo x$jobid $jobidx x$jobid[3] $other", out); + + /* Not a $jobid reference, so an unset $jobid does not matter. */ + TEST_ASSERT_EQUAL_INT(0, subst("echo $jobidx", &ctx_unset, out, sizeof(out), + err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("echo $jobidx", out); +} + +static void test_status_expands(void) +{ + char out[256]; + char err[128]; + const autovar_ctx_t neg = { NULL, -22, -1 }; + + TEST_ASSERT_EQUAL_INT( + 0, subst("assert $? 7", &ctx_set, out, sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("assert 7 7", out); + + TEST_ASSERT_EQUAL_INT( + 0, subst("assert $?", &neg, out, sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("assert -22", out); +} + +static void test_jobcount_set_and_unset(void) +{ + char out[256]; + char err[128]; + + TEST_ASSERT_EQUAL_INT(0, subst("assert $jobcount 4", &ctx_set, out, + sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("assert 4 4", out); + + TEST_ASSERT_EQUAL_INT(-EINVAL, subst("assert $jobcount 4", &ctx_unset, out, + sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("autovar: $jobcount referenced before set", err); +} + +static void test_shell_escape_and_comment_lines_untouched(void) +{ + char out[256]; + char err[128]; + + TEST_ASSERT_EQUAL_INT(0, subst("!echo $jobid[3] $? $jobcount", &ctx_unset, + out, sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("!echo $jobid[3] $? $jobcount", out); + + TEST_ASSERT_EQUAL_INT(0, subst(" # $jobid[3] $jobid", &ctx_unset, out, + sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING(" # $jobid[3] $jobid", out); +} + +static void test_out_too_small_is_enospc(void) +{ + char out[12]; + char err[128]; + + TEST_ASSERT_EQUAL_INT(-ENOSPC, subst("jobkill -j $jobid[3]", &ctx_set, out, + sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_INT(-ENOSPC, subst("jobkill -j $jobid", &ctx_set, out, + sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_INT(-ENOSPC, subst("0123456789ab", &ctx_set, out, + sizeof(out), err, sizeof(err))); + + /* Exactly fits: 11 chars + NUL. */ + TEST_ASSERT_EQUAL_INT( + 0, subst("0123456789a", &ctx_set, out, sizeof(out), err, sizeof(err))); + TEST_ASSERT_EQUAL_STRING("0123456789a", out); +} + +static void test_null_args_rejected(void) +{ + char out[16]; + + TEST_ASSERT_EQUAL_INT( + -EINVAL, autovar_substitute(NULL, &ctx_set, out, sizeof(out), NULL, 0)); + TEST_ASSERT_EQUAL_INT( + -EINVAL, autovar_substitute("x", NULL, out, sizeof(out), NULL, 0)); + TEST_ASSERT_EQUAL_INT(-EINVAL, + autovar_substitute("x", &ctx_set, NULL, 0, NULL, 0)); + TEST_ASSERT_EQUAL_INT(-EINVAL, + autovar_substitute("x", &ctx_set, out, 0, NULL, 0)); +} + +int main(void) +{ + UNITY_BEGIN(); + + RUN_TEST(test_jobid_index_expands_and_keeps_suffix); + RUN_TEST(test_jobid_index_invalid_remainder_copied_unchanged); + RUN_TEST(test_jobid_index_at_line_start_and_after_tab); + RUN_TEST(test_jobid_index_unset_fails_like_jobid); + RUN_TEST(test_jobid_whole_token); + RUN_TEST(test_jobid_unset_fails); + RUN_TEST(test_jobid_glued_tokens_copied_verbatim); + RUN_TEST(test_status_expands); + RUN_TEST(test_jobcount_set_and_unset); + RUN_TEST(test_shell_escape_and_comment_lines_untouched); + RUN_TEST(test_out_too_small_is_enospc); + RUN_TEST(test_null_args_rejected); + + return UNITY_END(); +}