diff --git a/.kiro/specs/xce-test-cli-ux/tasks.md b/.kiro/specs/xce-test-cli-ux/tasks.md index bb50d776..b7586235 100644 --- a/.kiro/specs/xce-test-cli-ux/tasks.md +++ b/.kiro/specs/xce-test-cli-ux/tasks.md @@ -39,7 +39,7 @@ - _Boundary: Interruptible Call Adapter_ - _Depends: 1.4_ -- [ ] 1.6 Add process-level help and version discovery +- [x] 1.6 Add process-level help and version discovery - Ensure invoking the process with a help or version flag prints the appropriate output and exits without starting the interactive/scripted session - Extend the process-level help output to describe the in-session help command as the way to list and learn available commands - Observable completion: invoking the process with the help flag prints usage text mentioning the in-session help command, and invoking with the version flag prints version information, both without entering the REPL @@ -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_(void);` + `TEST(test_);` in `src/controller/testsuite/controller_test.c`, mirroring the existing `test_rcfile` registration, or the suite silently never runs. (task 1.1) +- `argp` has no built-in `-h` alias (its default short help flag is `-?`) — Requirement 4.1 literally names `-h`, so it needed an explicit `OPTION_HIDDEN` option added, not just verification of existing defaults. Task 1.6's new `test_xcetest_process_entry.c` already covers "`-h`/`--help`/`--version` exit without entering the REPL" end-to-end against the real binary; task 1.7's `test_test_cli.c` should either skip re-adding this exact scenario or consolidate the two rather than duplicating coverage. (task 1.6) - The per-connection lock is a POSIX semaphore (`sem_t`), not a `pthread_mutex_t` — a mutex was tried first but rejected on review (POSIX classifies a non-owning-thread `pthread_mutex_unlock()` as UB for every mutex type, not just error-checking ones, and this design needs the worker thread to release a lock the caller thread acquired). `sem_t` has no ownership concept and is fully POSIX-defined for this pattern. See bug-1791. (task 1.5) - `interruptcall.c`'s forced-abandonment test proves `-EINTR` via return-value disambiguation against a slow `fn`, not a direct wall-clock elapsed-time assertion — sound today (poll interval is 50ms, matching design.md) but a future regression widening the poll interval wouldn't be caught unless the test also asserts an elapsed-time bound. Non-blocking; noted for task 4.1's end-to-end Ctrl-C validation to consider. (task 1.4) - After task 1.3's SIGINT/EINTR fix alone (before task 3.2 wires the REPL loop to check `xcetest_interrupted()`), a real Ctrl-C at `xce_test`'s top-level interactive prompt now causes the process to EXIT (readline's EINTR-driven NULL return is currently treated the same as EOF) rather than gracefully redisplaying the prompt. This is expected/deferred, not a regression — no current test exercises that path — but task 3.2's implementer must fix it as part of wiring the REPL loop, not just the RPC call sites. (task 1.3) diff --git a/src/controller/meson.build b/src/controller/meson.build index 82fbf4a0..61eb1c4d 100644 --- a/src/controller/meson.build +++ b/src/controller/meson.build @@ -174,6 +174,7 @@ controller_unit_test_src = [ 'testsuite/test_xcetest_interrupt.c', 'testsuite/test_interruptcall.c', 'testsuite/interruptcall.c', + 'testsuite/test_xcetest_process_entry.c', 'testsuite/test_persist_snapshot_entities.c', 'testsuite/test_apply_seam.c', 'testsuite/test_apply_committed_entry.c', diff --git a/src/controller/testsuite/controller_test.c b/src/controller/testsuite/controller_test.c index ce070346..bb024c31 100644 --- a/src/controller/testsuite/controller_test.c +++ b/src/controller/testsuite/controller_test.c @@ -74,6 +74,7 @@ extern void test_rcfile(void); extern void test_flagspec(void); extern void test_xcetest_interrupt(void); extern void test_interruptcall(void); +extern void test_xcetest_process_entry(void); extern void test_drive_async_config(void); extern void test_slowlane(void); extern void test_commit_admission_pool(void); @@ -257,6 +258,7 @@ int main(int argc, char* argv[]) TEST(test_flagspec); TEST(test_xcetest_interrupt); TEST(test_interruptcall); + TEST(test_xcetest_process_entry); TEST(test_drive_async_config); TEST(test_slowlane); TEST(test_commit_admission_pool); diff --git a/src/controller/testsuite/test.c b/src/controller/testsuite/test.c index 43391331..79ff8b57 100644 --- a/src/controller/testsuite/test.c +++ b/src/controller/testsuite/test.c @@ -657,7 +657,15 @@ static command_ctx_t* active_ctx = &topctx; static char prompt[256]; static int break_out = 0; -static char doc[] = "xce_test"; +/* xce-test-cli-ux task 1.6, Requirement 4.3: describes the in-session + * `help' command as the way to list/learn available commands, in addition + * to argp's own auto-generated --help/-?/-V flag documentation below. */ +static char doc[] = + "xce_test -- interactive/scripted test client for the controller, " + "worker, and scheduler\v" + "Once a session is running, type `help' at the prompt to list the " + "commands available in the current context, or `help ' for " + "details on a specific one."; static char args_doc[] = ""; /* build-time-versioning task 3.2: no longer a compile-time initializer -- @@ -681,6 +689,13 @@ struct arguments { static struct argp_option options[] = { { "verbose", 'v', 0, 0, "Produce verbose logs" }, { "norc", OPT_NORC, 0, 0, "Skip loading the rc file for this invocation" }, + /* xce-test-cli-ux task 1.6, Requirement 4.1: argp's OWN built-in help + * alias is `-?` (via --help), NOT `-h` -- glibc argp does not add `-h` + * automatically. `-h` is hidden from the rendered help/usage text + * (OPTION_HIDDEN) so it doesn't duplicate the auto-generated + * "-?, --help" entry there; it is still a fully functional alias, + * handled explicitly in parse_opt() below via argp_state_help(). */ + { 0, 'h', 0, OPTION_HIDDEN, 0 }, { 0 } }; @@ -695,6 +710,12 @@ static error_t parse_opt(int key, char* arg, struct argp_state* state) case OPT_NORC: arguments->norc = 1; break; + case 'h': + /* ARGP_HELP_STD_HELP includes ARGP_HELP_EXIT_OK, so this prints + * the same help text as --help/-? and exits(0) -- it does not + * return. */ + argp_state_help(state, stdout, ARGP_HELP_STD_HELP); + break; default: return ARGP_ERR_UNKNOWN; } diff --git a/src/controller/testsuite/test_xcetest_process_entry.c b/src/controller/testsuite/test_xcetest_process_entry.c new file mode 100644 index 00000000..7ba8540c --- /dev/null +++ b/src/controller/testsuite/test_xcetest_process_entry.c @@ -0,0 +1,243 @@ +/* + * test_xcetest_process_entry.c -- xce-test-cli-ux task 1.6, Requirements + * 4.1, 4.2, 4.3 (Process-Level Help and Version Discovery). + * + * These are black-box, subprocess tests: they exec the REAL, built + * xce_test binary (a sibling of this test binary in the same build + * directory -- see resolve_xce_test_path()) with a process-level flag and + * inspect its exit code and captured stdout. This is deliberately NOT a + * plain unit test that calls argp_parse()/parse_opt() in-process: what's + * under test is main()'s own process-level argv handling, including + * whether it reaches the interactive/scripted REPL loop at all, which can + * only be observed by actually running the process to completion. + * + * This is a small, self-contained subprocess harness scoped to this one + * task's needs (process-level -h/--help/--version) -- NOT the reusable + * pty-driven black-box CLI harness that task 1.7 builds separately (that + * one will live in test_test_cli.c and is needed for interactive-only + * surfaces like tab completion that a plain pipe can't exercise). + */ +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define CAPTURE_BUF_SIZE 16384 +#define CHILD_TIMEOUT_TENTHS 50 /* 5 seconds, polled every 100ms */ + +/* xce_test is always built as a sibling of this test binary in the same + * meson build directory (src/controller/meson.build builds both `xce_test` + * and `controller_unit_test` into the same target dir) -- resolving via + * /proc/self/exe's directory avoids depending on the caller's cwd or an + * env var that could go stale. */ +static int resolve_xce_test_path(char* out, size_t outsz) +{ + char self[PATH_MAX]; + ssize_t n; + char* dir; + + n = readlink("/proc/self/exe", self, sizeof(self) - 1); + if (n <= 0) { + return -1; + } + self[n] = '\0'; + + dir = dirname(self); /* dirname() may modify its argument in place; + * `self` is a disposable local copy, so that's + * fine here. */ + + if (snprintf(out, outsz, "%s/xce_test", dir) >= (int)outsz) { + return -1; + } + return 0; +} + +/* Runs xce_test_path with argv (NULL-terminated; argv[0] conventionally + * the program name) with stdin attached to /dev/null -- so that if a + * regression makes it fall through into the interactive REPL instead of + * exiting, readline() sees immediate EOF rather than hanging -- and + * captures its stdout into out_buf. Kills and fails the test if the child + * does not exit within CHILD_TIMEOUT_TENTHS/10 seconds, so a genuine hang + * can't wedge the whole suite. */ +static void run_and_capture(const char* xce_test_path, char* const argv[], + char* out_buf, size_t out_buf_size, + int* exit_code) +{ + int outpipe[2]; + pid_t pid; + int status = 0; + int elapsed = 0; + pid_t waited; + + TEST_ASSERT_EQUAL_INT(0, pipe(outpipe)); + + pid = fork(); + TEST_ASSERT_TRUE(pid >= 0); + + if (pid == 0) { + int devnull = open("/dev/null", O_RDONLY); + + if (devnull >= 0) { + dup2(devnull, STDIN_FILENO); + close(devnull); + } + dup2(outpipe[1], STDOUT_FILENO); + close(outpipe[0]); + close(outpipe[1]); + + execv(xce_test_path, argv); + _exit(127); /* only reached if execv() itself failed */ + } + + close(outpipe[1]); + + for (;;) { + waited = waitpid(pid, &status, WNOHANG); + if (waited == pid) { + break; + } + if (elapsed >= CHILD_TIMEOUT_TENTHS) { + kill(pid, SIGKILL); + waitpid(pid, &status, 0); + close(outpipe[0]); + TEST_FAIL_MESSAGE( + "xce_test did not exit within timeout -- it likely fell " + "through into the REPL instead of exiting after a " + "process-level help/version flag"); + } + usleep(100000); + elapsed++; + } + + { + ssize_t total = 0; + ssize_t r; + + while (total < (ssize_t)out_buf_size - 1) { + r = read(outpipe[0], out_buf + total, + out_buf_size - 1 - (size_t)total); + if (r <= 0) { + break; + } + total += r; + } + out_buf[total] = '\0'; + } + close(outpipe[0]); + + TEST_ASSERT_TRUE(WIFEXITED(status)); + *exit_code = WEXITSTATUS(status); +} + +/* Word-wrapped help text can break a multi-word phrase across lines at any + * space -- argp wraps the doc string to the terminal width. Folding + * newlines to spaces before substring-matching makes assertions robust to + * exactly where those wraps land. */ +static void flatten_newlines(char* s) +{ + for (; *s; s++) { + if (*s == '\n') { + *s = ' '; + } + } +} + +static void test_help_flag_prints_usage_and_exits_without_repl(void) +{ + char xce_test_path[PATH_MAX]; + char out[CAPTURE_BUF_SIZE]; + int exit_code; + char* const argv[] = { "xce_test", "--help", NULL }; + + TEST_ASSERT_EQUAL_INT( + 0, resolve_xce_test_path(xce_test_path, sizeof(xce_test_path))); + + run_and_capture(xce_test_path, argv, out, sizeof(out), &exit_code); + + TEST_ASSERT_EQUAL_INT(0, exit_code); + TEST_ASSERT_NOT_NULL(strstr(out, "Usage:")); + /* Never reached the interactive session's top-level prompt. */ + TEST_ASSERT_NULL(strstr(out, "xce>")); +} + +/* Requirement 4.3: the process-level help output must describe the + * in-session `help` command as the way to list/learn available commands + * -- not just document its own --help/-?/-h flags. */ +static void test_help_flag_describes_insession_help_command(void) +{ + char xce_test_path[PATH_MAX]; + char out[CAPTURE_BUF_SIZE]; + int exit_code; + char* const argv[] = { "xce_test", "--help", NULL }; + + TEST_ASSERT_EQUAL_INT( + 0, resolve_xce_test_path(xce_test_path, sizeof(xce_test_path))); + + run_and_capture(xce_test_path, argv, out, sizeof(out), &exit_code); + TEST_ASSERT_EQUAL_INT(0, exit_code); + + flatten_newlines(out); + TEST_ASSERT_NOT_NULL_MESSAGE( + strstr(out, "type `help' at the prompt to list"), + "xce_test --help must describe the in-session `help' command as " + "the way to list/learn available commands (Requirement 4.3)"); +} + +/* Requirement 4.1 explicitly calls out `-h` alongside `--help`. GNU argp's + * OWN built-in short help alias is `-?`, not `-h` -- so unlike --help/-?, + * this is not automatic and must be verified (and, if missing, added). */ +static void test_dash_h_flag_prints_usage_and_exits_without_repl(void) +{ + char xce_test_path[PATH_MAX]; + char out[CAPTURE_BUF_SIZE]; + int exit_code; + char* const argv[] = { "xce_test", "-h", NULL }; + + TEST_ASSERT_EQUAL_INT( + 0, resolve_xce_test_path(xce_test_path, sizeof(xce_test_path))); + + run_and_capture(xce_test_path, argv, out, sizeof(out), &exit_code); + + TEST_ASSERT_EQUAL_INT(0, exit_code); + TEST_ASSERT_NOT_NULL(strstr(out, "Usage:")); + TEST_ASSERT_NULL(strstr(out, "xce>")); +} + +static void test_version_flag_prints_version_and_exits_without_repl(void) +{ + char xce_test_path[PATH_MAX]; + char out[CAPTURE_BUF_SIZE]; + int exit_code; + char* const argv[] = { "xce_test", "--version", NULL }; + + TEST_ASSERT_EQUAL_INT( + 0, resolve_xce_test_path(xce_test_path, sizeof(xce_test_path))); + + run_and_capture(xce_test_path, argv, out, sizeof(out), &exit_code); + + TEST_ASSERT_EQUAL_INT(0, exit_code); + TEST_ASSERT_NOT_NULL(strstr(out, "xce")); + /* xce_compose_version_ex()'s output includes a "built " + * segment -- distinguishes real version info from a coincidental + * "xce" substring elsewhere. */ + TEST_ASSERT_NOT_NULL(strstr(out, "built")); + TEST_ASSERT_NULL(strstr(out, "Usage:")); + TEST_ASSERT_NULL(strstr(out, "xce>")); +} + +void test_xcetest_process_entry(void) +{ + UnitySetTestFile(__FILE__); + RUN_TEST(test_help_flag_prints_usage_and_exits_without_repl); + RUN_TEST(test_help_flag_describes_insession_help_command); + RUN_TEST(test_dash_h_flag_prints_usage_and_exits_without_repl); + RUN_TEST(test_version_flag_prints_version_and_exits_without_repl); +}