feat(xce-test-cli-ux): add process-level help/version discovery
argp already provided --help/-?/--version with exit-without-REPL behavior, but had no -h alias (its default short help flag is -? per GNU argp convention) despite Requirement 4.1 naming -h explicitly. Added a hidden -h option triggering the same help path, and extended the doc string to describe the in-session help command. Proven against the real compiled binary via subprocess tests, not just argp's own in-process behavior. 1289/1289 controller suite passing (+4). Task 1.6 of .kiro/specs/xce-test-cli-ux/tasks.md. Requirements: 4.1, 4.2, 4.3 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
This commit is contained in:
parent
204f81d134
commit
b70228c558
5 changed files with 270 additions and 2 deletions
|
|
@ -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_<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)
|
||||
- `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)
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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 <command>' 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;
|
||||
}
|
||||
|
|
|
|||
243
src/controller/testsuite/test_xcetest_process_entry.c
Normal file
243
src/controller/testsuite/test_xcetest_process_entry.c
Normal file
|
|
@ -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 <fcntl.h>
|
||||
#include <libgen.h>
|
||||
#include <limits.h>
|
||||
#include <signal.h>
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#include <sys/wait.h>
|
||||
#include <unistd.h>
|
||||
|
||||
#include <unity/unity.h>
|
||||
|
||||
#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 <timestamp>"
|
||||
* 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);
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue