From a98fad24dafc48b911521c8d3f342997b4182de2 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 10 Sep 2026 00:59:38 +0000 Subject: [PATCH] fix(xce-test-cli-ux): route diagnostic/warning messages to stderr uniformly Audit every printf() call site in test.c not already covered by the flagspec-rejection or perror() paths; convert 33 genuine error/warning messages (expect-jobs, expect-job, wait-primary, help's not-found path, jobrun/jobsub/jobstart/jobfinish/jobfail validation, resupdate, set) from stdout to stderr, so a command's successful result data and its diagnostics never share a stream. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f --- .kiro/specs/xce-test-cli-ux/tasks.md | 3 +- src/controller/testsuite/test.c | 132 ++++++++----- src/controller/testsuite/test_test_cli.c | 225 ++++++++++++++++++++++- 3 files changed, 305 insertions(+), 55 deletions(-) diff --git a/.kiro/specs/xce-test-cli-ux/tasks.md b/.kiro/specs/xce-test-cli-ux/tasks.md index 378bd8c0..49d1281f 100644 --- a/.kiro/specs/xce-test-cli-ux/tasks.md +++ b/.kiro/specs/xce-test-cli-ux/tasks.md @@ -140,7 +140,7 @@ - _Boundary: Help Renderer_ - _Depends: 1.2, 3.1, 2.6, 3.4_ -- [ ] 3.6 Audit and correct stdout/stderr stream routing across existing error paths +- [x] 3.6 Audit and correct stdout/stderr stream routing across existing error paths - Review every command's remaining error paths not already covered by the shared flag-rejection path built in 1.1 — request failures and internal errors — and ensure diagnostic/error text goes to stderr while a command's successful result data goes to stdout only - Confirm the same routing holds identically whether stdout and stderr are both attached to a terminal or redirected to separate destinations (a script or pipe) - Observable completion: redirecting only stdout to a file while triggering a request failure or an internal error on a representative command shows the diagnostic text still appearing on the terminal while the redirected file contains no diagnostic text; separately, running a successful command with only stdout redirected shows its result data captured in the redirected file with nothing sent to stderr @@ -176,6 +176,7 @@ - _Depends: 1.6, 2.3, 3.5, 1.7_ ## Implementation Notes +- Audited all 139 `printf(` call sites in test.c; 33 were genuine diagnostic/warning/failure-path messages misrouted to stdout (not covered by the already-correct `flagspec_reject`/`perror` mechanisms) and were converted to `fprintf(stderr, ...)` — concentrated in `cmd_expect_jobs`/`cmd_expect_job`/`cmd_wait_primary` (entirely unrouted; these predate the flagspec migration since they take positional args, not flags), plus `get_setting`, `cmd_help`'s not-found path (a shared `FILE* out` var, stderr only when not-found; bare `help`'s listing and a successful lookup stay stdout), `cmd_ctrlstatus`, `cmd_jobrun`, `cmd_jobsub`, `cmd_jobstart`/`jobfinish`/`jobfail`, `resupdate_report_one`/`cmd_resupdate`, `cmd_set`. Deliberately left as stdout (Requirement 3 doesn't apply): `cmd_resval`'s intentional always-succeeds "not found" design, and the entire async worker/scheduler RPC-handler subsystem (`handle_notify`/`run_job`/etc.) — these are inbound-request callbacks dispatched outside the REPL's `cmd_*` table, not "a command's own diagnostic or result data." Found but explicitly left alone as pre-existing and out of THIS task's scope (stream routing, not return-code correctness): `cmd_resupdate` still unconditionally `return 0`s even on an outright RPC failure or a per-resource rejection — worth a follow-up bug entry if this area is touched again. Requirement 3.4 (redirection-invariance) holds by construction: no code branches a stream CHOICE on `isatty`/`g_interactive` anywhere in the file. (task 3.6) - `command_t` gained `flagspec`/`flagspec_count`/`usage_tail` fields; `cmd_help` now composes description(`usage` head) -> generated `flagspec_render_help()` block -> `usage_tail` (example/related notes) for all 22 flag-accepting commands, with hand-written per-flag lines deleted from their `USAGE_X`/`USAGE_X_TAIL` constants so the flagspec table is the only place a flag's short/long/required/default lives. Since the four `command_t` tables (`top_commands` etc.) appear before the 22 `flagspec_t` tables in the file, each is forward-declared with an explicit literal size (e.g. `static const flagspec_t jobkill_flags[3];`) rather than the previous `#define X_NFLAGS (sizeof(x_flags)/sizeof(x_flags[0]))` self-computing macro — the literal must now be kept in sync by hand, though any mismatch between the forward declaration and the real definition is a compiler "conflicting types" build error, never a silent drift (review verified this is standard, portable C, and hand-counted several tables' entries against their literals). A follow-up could reorder the flagspec_t tables above the command_t tables to restore the sizeof-based macro and drop this literal-maintenance requirement entirely, but it wasn't required for this task. `join_flags`/`JOIN_FLAGS_COUNT` (a separate translation unit, join_worker_args.c/.h) uses the standard `extern const T arr[N]` header/source split instead, since `extern` against a `static` definition isn't valid — no literal-sync risk there. The five destructive commands' help text now names the confirmation gate and `-y`/`set -y` bypass, scoped accurately (`qctrl`'s note covers only `-i`/`-c`, `hostctrl`'s only `-c`, matching Requirement 9.5's exact enumeration, not their full flag sets). (task 3.5) - `workerlist`/`joblist`/`reslist` all take the identical long-only `--json` flag (via each command's own `flagspec_t` table); JSON rows emit RAW numeric values for fields the table displays in a humanized/formatted form (`flags` as an int not a hex string, timestamps as raw epoch seconds not `format_job_time`'s human string, `maxmem`/`maxswap`/`maxtmp` as raw bytes not `format_bytes_human`'s string) — deliberate, since Requirement 5.1 asks for "the same fields" (value present) not identical display formatting, and a raw value is strictly more script-consumable. The field-mapping logic was extracted to a new pure module, `listrow_json.c`/`.h` (no RPC/networking/global-state coupling), following the `joblist_display.c` extraction precedent, with full unit coverage in `test_listrow_json.c` (a new standalone `listrow_json_unit_test` meson target, not counted in the main suite's test total) including the fractional-value (`cpufactor` 12.34) real cJSON-print-then-reparse round-trip the task-2.2 note asked for. `joblist_query` gained a 5th `json_mode` parameter; its other two callers (`expect-jobs`/`expect-job`) now pass a literal `0` and are behaviorally unchanged (verified: both `print_table` and `json_mode` false skips both branches exactly as before). Live-cluster E2E visual inspection of real JSON output was not done — same "genuinely untestable without a live cluster" situation task 2.5 already documented for these commands' inlined fetch+print bodies; coverage instead comes from the extracted module's unit tests plus a black-box test proving the flag parses/executes without a flag-parse error. (task 3.4) - `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) diff --git a/src/controller/testsuite/test.c b/src/controller/testsuite/test.c index 7b2b111d..1bf6bbbe 100644 --- a/src/controller/testsuite/test.c +++ b/src/controller/testsuite/test.c @@ -997,7 +997,7 @@ static char* get_setting(char* key) return active_ctx->settings[i].defval; } - printf("Key `%s' is not in settings\n", key); + fprintf(stderr, "Key `%s' is not in settings\n", key); return NULL; } @@ -1467,8 +1467,15 @@ int preprocess_and_run(char* line) int cmd_help(int argc, char* argv[]) { - int i; - int rc = 0; + int i; + int rc = 0; + /* xce-test-cli-ux task 3.6, Requirement 3.1/3.2: the command-list loop + * below is shared by two callers -- bare `help` (success, Req 3.2 result + * data) and the not-found fallback just above it (Req 3.1 diagnostic, + * "here's what IS available"). Route BOTH the not-found headline and + * that shared list to `out`, selected by which case we're in, rather + * than duplicating the loop. */ + FILE* out = stdout; if (argc > 1) { char* cmd = argv[1]; @@ -1521,13 +1528,14 @@ int cmd_help(int argc, char* argv[]) /* bug-1711 / Requirement 11.2: grammar fix only ("match", not * "matchs") -- does NOT add the cross-context lookup Requirement * 8.7 explicitly excludes. */ - printf("No commands match `%s'. Possibilities are:\n", cmd); + out = stderr; /* Req 3.1: not-found is a diagnostic, not result data */ + fprintf(out, "No commands match `%s'. Possibilities are:\n", cmd); rc = -1; } for (i = 0; active_ctx->commands[i].name != NULL; i++) { - printf("\t%-12s\t%s\n", active_ctx->commands[i].name, - active_ctx->commands[i].doc); + fprintf(out, "\t%-12s\t%s\n", active_ctx->commands[i].name, + active_ctx->commands[i].doc); } return rc; @@ -2709,7 +2717,7 @@ int cmd_expect_jobs(int argc, char* argv[]) int count; if (argc != 2) { - printf("expect-jobs \n"); + fprintf(stderr, "expect-jobs \n"); return -EINVAL; } @@ -2717,9 +2725,10 @@ int cmd_expect_jobs(int argc, char* argv[]) expected = strtol(argv[1], &end, 10); if ((errno != 0) || (end == argv[1]) || (*end != '\0') || (expected < 0)) { - printf("expect-jobs: invalid count '%s' (expected a non-negative " - "integer)\n", - argv[1]); + fprintf(stderr, + "expect-jobs: invalid count '%s' (expected a non-negative " + "integer)\n", + argv[1]); return -EINVAL; } @@ -2729,13 +2738,15 @@ int cmd_expect_jobs(int argc, char* argv[]) count = joblist_query(NULL, NULL, 0, /*quiet=*/0, /*json_mode=*/0); if (count < 0) { - printf("expect-jobs: could not determine job count (joblist query " - "failed)\n"); + fprintf(stderr, + "expect-jobs: could not determine job count (joblist query " + "failed)\n"); return count; } if (count != (int)expected) { - printf("expect-jobs: expected %ld, actual %d\n", expected, count); + fprintf(stderr, "expect-jobs: expected %ld, actual %d\n", expected, + count); return -EINVAL; } @@ -2767,7 +2778,7 @@ int cmd_expect_job(int argc, char* argv[]) int rc; if (argc != 2) { - printf("expect-job \n"); + fprintf(stderr, "expect-job \n"); return -EINVAL; } @@ -2777,14 +2788,15 @@ int cmd_expect_job(int argc, char* argv[]) rc = joblist_query(argv[1], &contains, 0, /*quiet=*/0, /*json_mode=*/0); if (rc < 0) { - printf("expect-job: could not determine membership of '%s' (joblist " - "query failed)\n", - argv[1]); + fprintf(stderr, + "expect-job: could not determine membership of '%s' (joblist " + "query failed)\n", + argv[1]); return rc; } if (!contains) { - printf("expect-job: job %s not found\n", argv[1]); + fprintf(stderr, "expect-job: job %s not found\n", argv[1]); return -ENOENT; } @@ -2858,7 +2870,7 @@ int cmd_wait_primary(int argc, char* argv[]) long start_ms, elapsed_ms; if (argc > 2) { - printf("wait-primary [timeout_ms]\n"); + fprintf(stderr, "wait-primary [timeout_ms]\n"); return -EINVAL; } @@ -2870,9 +2882,10 @@ int cmd_wait_primary(int argc, char* argv[]) if ((errno != 0) || (end == argv[1]) || (*end != '\0') || (timeout_ms < 0)) { - printf("wait-primary: invalid timeout '%s' (expected a " - "non-negative integer in ms)\n", - argv[1]); + fprintf(stderr, + "wait-primary: invalid timeout '%s' (expected a " + "non-negative integer in ms)\n", + argv[1]); return -EINVAL; } } @@ -2909,11 +2922,13 @@ int cmd_wait_primary(int argc, char* argv[]) * 1.1) rather than retrying -- ownership of `call` has passed * to the still-running worker thread (interruptcall.h). */ mylog_setlevel(g_active_loglevel); - /* Same printf-based reporting style as this command's own - * success/timeout messages just below, rather than a one-off - * perror() this loop otherwise never uses. */ - printf("wait-primary: interrupted after %ld ms\n", - monotonic_ms() - start_ms); + /* xce-test-cli-ux task 3.6, Requirement 3.1: an interrupted wait + * is a genuine command failure (rc == -EINTR here, same + * convention cmd_resval's EINTR path documents), so this goes + * to stderr like this command's other failure paths -- not + * grouped with its success message below anymore. */ + fprintf(stderr, "wait-primary: interrupted after %ld ms\n", + monotonic_ms() - start_ms); return rc; } @@ -2945,9 +2960,10 @@ int cmd_wait_primary(int argc, char* argv[]) if (elapsed_ms >= timeout_ms) { mylog_setlevel(g_active_loglevel); - printf("wait-primary: no primary served within %ld ms (waited " - "%ld ms)\n", - timeout_ms, elapsed_ms); + fprintf(stderr, + "wait-primary: no primary served within %ld ms (waited " + "%ld ms)\n", + timeout_ms, elapsed_ms); return -ETIMEDOUT; } @@ -3022,7 +3038,7 @@ int cmd_ctrlstatus(int argc, char* argv[]) (void)argv; if (argc != 1) { - printf("ctrlstatus\n"); + fprintf(stderr, "ctrlstatus\n"); return -EINVAL; } @@ -3406,7 +3422,7 @@ int cmd_jobrun(int argc, char* argv[]) case 'j': parse_jobid(&jobid, optarg); if (jobid == 0) { - printf("you need to specify a positive jobid with -j"); + fprintf(stderr, "you need to specify a positive jobid with -j"); return -1; } break; @@ -3418,7 +3434,7 @@ int cmd_jobrun(int argc, char* argv[]) } if (jobid == 0) { - printf("no jobid specified"); + fprintf(stderr, "no jobid specified"); return -1; } @@ -3579,8 +3595,9 @@ int cmd_jobsub(int argc, char* argv[]) min = atoi(buf); if (max == 0) { - printf("Incorrect setting of max tasks count, set to same " - "as min task value\n"); + fprintf(stderr, + "Incorrect setting of max tasks count, set to " + "same as min task value\n"); max = min; } } else { @@ -3604,24 +3621,25 @@ int cmd_jobsub(int argc, char* argv[]) } if (min == 0) { - printf("Incorrect setting of min task count, set to 1 as default\n"); + fprintf(stderr, + "Incorrect setting of min task count, set to 1 as default\n"); min = 1; } if (NULLSTR(submit.user)) { - printf("you need to specify job user with -u\n"); + fprintf(stderr, "you need to specify job user with -u\n"); rc = -EINVAL; goto cleanup; } if (NULLSTR(submit.queue)) { - printf("you need to specify job queue with -q\n"); + fprintf(stderr, "you need to specify job queue with -q\n"); rc = -EINVAL; goto cleanup; } if (NULLSTR(submit.start_info->cmdline)) { - printf("you need to specify job command with -c\n"); + fprintf(stderr, "you need to specify job command with -c\n"); rc = -EINVAL; goto cleanup; } @@ -5183,7 +5201,7 @@ int cmd_jobstart(int argc, char* argv[]) } if (jobid == 0) { - printf(" you need specify a positive jobid\n"); + fprintf(stderr, " you need specify a positive jobid\n"); return -EINVAL; } @@ -5293,7 +5311,7 @@ int cmd_jobfinish(int argc, char* argv[]) } if (jobid == 0) { - printf(" you need specify a positive jobid\n"); + fprintf(stderr, " you need specify a positive jobid\n"); return -EINVAL; } @@ -5401,7 +5419,7 @@ int cmd_jobfail(int argc, char* argv[]) } if (jobid == 0) { - printf(" you need specify a positive jobid\n"); + fprintf(stderr, " you need specify a positive jobid\n"); return -EINVAL; } @@ -5650,10 +5668,19 @@ static int resupdate_report_one(char* name, char* host, xce_val_t* sent) } } + /* xce-test-cli-ux task 3.6, Requirement 3.1: unlike the "accepted" line + * above (legitimate per-resource outcome data on the success path), a + * "rejected" verdict reports that this specific update did NOT take + * effect -- a problem the operator/script needs to notice -- so it goes + * to stderr. This does NOT change cmd_resupdate()'s own return value: + * the caller still returns 0 overall even when one or more resources are + * reported rejected here (see this task's CONCERNS for why that return- + * value question is left as a follow-up, out of this stream-routing + * task's scope). */ if (ever_found) { - printf("%s: rejected: value mismatch\n", name); + fprintf(stderr, "%s: rejected: value mismatch\n", name); } else { - printf("%s: rejected: unknown resource\n", name); + fprintf(stderr, "%s: rejected: unknown resource\n", name); } return 0; @@ -5725,7 +5752,8 @@ int cmd_resupdate(int argc, char* argv[]) case 'n': if (resname != NULL) { if (resval_str == NULL) { - printf( + fprintf( + stderr, "res `%s' has no value specified, default will be used\n", resname); } @@ -5750,7 +5778,9 @@ int cmd_resupdate(int argc, char* argv[]) if (resname != NULL && resval_str == NULL) { resval_str = optarg; } else { - printf("ignore res value `%s' as no res name found\n", optarg); + fprintf(stderr, + "ignore res value `%s' as no res name found\n", + optarg); } break; case 't': @@ -5760,13 +5790,15 @@ int cmd_resupdate(int argc, char* argv[]) } else if (strcasecmp(optarg, "string") == 0) { type_override = Value_u_svalue; } else { - printf( + fprintf( + stderr, "ignoring unrecognized -t value `%s' (expected " "`numeric' or `string')\n", optarg); } } else { - printf("ignore -t `%s' as no res name found\n", optarg); + fprintf(stderr, "ignore -t `%s' as no res name found\n", + optarg); } break; case '?': @@ -6075,7 +6107,7 @@ int cmd_set(int argc, char* argv[]) } if (argc != 3) { - printf("set \n"); + fprintf(stderr, "set \n"); return -EINVAL; } @@ -6090,7 +6122,7 @@ int cmd_set(int argc, char* argv[]) } } - printf("Can not find setting key: %s\n", argv[1]); + fprintf(stderr, "Can not find setting key: %s\n", argv[1]); return -EINVAL; } diff --git a/src/controller/testsuite/test_test_cli.c b/src/controller/testsuite/test_test_cli.c index fa54b2e8..e09a5725 100644 --- a/src/controller/testsuite/test_test_cli.c +++ b/src/controller/testsuite/test_test_cli.c @@ -727,8 +727,11 @@ static void test_tab_completion_context_scoped_single_and_multi_match(void) TEST_ASSERT_EQUAL_INT(0, clitest_send_partial(&sess, "js")); TEST_ASSERT_EQUAL_INT(0, clitest_send_tab(&sess)); TEST_ASSERT_EQUAL_INT(0, clitest_send_raw(&sess, &enter, 1)); + /* xce-test-cli-ux task 3.6, Requirement 3.1: cmd_jobstart's bare- + * invocation validation message now goes to stderr (in_stderr=true), + * not stdout. */ TEST_ASSERT_TRUE_MESSAGE( - clitest_wait_for_text(&sess, &out, &err, "positive jobid", false, + clitest_wait_for_text(&sess, &out, &err, "positive jobid", true, 3000), "single unambiguous match 'js' must complete in full to 'jstart' " "in the worker context, proving candidates are drawn from the " @@ -2727,16 +2730,23 @@ static void test_help_unknown_command_shows_corrected_grammar(void) TEST_ASSERT_EQUAL_INT( 0, clitest_send_line(&sess, "help bogus_nonexistent_command")); + /* xce-test-cli-ux task 3.6, Requirement 3.1: a help lookup that finds + * nothing is a diagnostic about the request, not result data -- it now + * belongs on stderr (in_stderr=true), not stdout. */ TEST_ASSERT_TRUE_MESSAGE( clitest_wait_for_text( &sess, &out, &err, - "No commands match `bogus_nonexistent_command'", false, 5000), + "No commands match `bogus_nonexistent_command'", true, 5000), "bug-1711/Requirement 11.2: the not-found message must use " "correct grammar ('match', not 'matchs')"); TEST_ASSERT_NULL_MESSAGE( - strstr(out.data, "matchs"), + strstr(err.data, "matchs"), "the corrected message must not contain the old 'matchs' typo " "anywhere in the captured output"); + TEST_ASSERT_NULL_MESSAGE( + strstr(out.data, "No commands match"), + "xce-test-cli-ux task 3.6, Requirement 3.2: the not-found " + "diagnostic must not also appear on stdout"); TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "quit")); TEST_ASSERT_EQUAL_INT_MESSAGE( @@ -2792,9 +2802,11 @@ test_help_context_scoped_not_found_no_cross_context_hint(void) * client_commands[] has it) -- must be reported not-found, never the * client context's jobkill help text (which mentions "signal"). */ TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "help jobkill")); + /* xce-test-cli-ux task 3.6, Requirement 3.1: not-found is a diagnostic, + * so it belongs on stderr (in_stderr=true), not stdout. */ TEST_ASSERT_TRUE_MESSAGE( clitest_wait_for_text(&sess, &out, &err, - "No commands match `jobkill'", false, 5000), + "No commands match `jobkill'", true, 5000), "Requirement 8.7: from the top-level context, 'help jobkill' must " "be reported as not found there, without checking the client " "context's table"); @@ -2802,6 +2814,10 @@ test_help_context_scoped_not_found_no_cross_context_hint(void) strstr(out.data, "Sends a signal to a running job"), "the top-level not-found response must never leak client " "context's jobkill help content"); + TEST_ASSERT_NULL_MESSAGE( + strstr(err.data, "Sends a signal to a running job"), + "the top-level not-found response must never leak client " + "context's jobkill help content, on stderr either"); /* Now enter the client context, where jobkill DOES exist -- the same * lookup must now succeed and print jobkill's own detailed help. */ @@ -2836,6 +2852,204 @@ test_help_context_scoped_not_found_no_cross_context_hint(void) clitest_buf_free(&err); } +/* + * test_expect_jobs_malformed_count_diagnostic_on_stderr_not_stdout -- + * xce-test-cli-ux task 3.6, Requirement 3.1/3.2/3.3/3.4: `expect-jobs` given + * a non-numeric count fails its local strtol validation before any network + * call, so this is a fast, deterministic way to prove the fix without a live + * controller. The diagnostic must land on stderr (this harness keeps stderr + * on a genuine separate pipe, never sharing the pty with stdout -- see this + * file's header comment) and must NOT also appear on stdout, whether or not + * stdout/stderr are redirected to separate destinations (Req 3.4 -- C's + * stdio routing is redirection-agnostic, so the pty-vs-pipe split here + * already proves the same thing a real shell redirection would). + */ +static void +test_expect_jobs_malformed_count_diagnostic_on_stderr_not_stdout(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, "expect-jobs notanumber")); + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, + "expect-jobs: invalid count 'notanumber'", + true, 5000), + "xce-test-cli-ux task 3.6, Requirement 3.1: a malformed 'expect-jobs' " + "argument must report its diagnostic on stderr"); + TEST_ASSERT_NULL_MESSAGE( + strstr(out.data, "invalid count"), + "xce-test-cli-ux task 3.6, Requirement 3.2: the diagnostic must not " + "also appear on stdout"); + + 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 expect-jobs invocation must mark the session failed"); + + clitest_kill_and_reap(&sess); + clitest_close(&sess); + clitest_buf_free(&out); + clitest_buf_free(&err); +} + +/* + * test_wait_primary_invalid_timeout_diagnostic_on_stderr_not_stdout -- + * xce-test-cli-ux task 3.6, Requirement 3.1/3.2/3.3/3.4: `wait-primary` + * given a non-numeric timeout fails its local strtol validation before any + * network call (the same fast/deterministic shape as the expect-jobs case + * above), proving the fix holds across more than one command. + */ +static void +test_wait_primary_invalid_timeout_diagnostic_on_stderr_not_stdout(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, "wait-primary notanumber")); + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, + "wait-primary: invalid timeout 'notanumber'", + true, 5000), + "xce-test-cli-ux task 3.6, Requirement 3.1: a malformed " + "'wait-primary' timeout argument must report its diagnostic on " + "stderr"); + TEST_ASSERT_NULL_MESSAGE( + strstr(out.data, "invalid timeout"), + "xce-test-cli-ux task 3.6, Requirement 3.2: the diagnostic must not " + "also appear on stdout"); + + 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 wait-primary invocation must mark the session " + "failed"); + + clitest_kill_and_reap(&sess); + clitest_close(&sess); + clitest_buf_free(&out); + clitest_buf_free(&err); +} + +/* + * test_set_list_success_result_on_stdout_not_stderr -- xce-test-cli-ux task + * 3.6, Requirement 3.2/3.3/3.4: the companion success-path check for this + * task's observable completion criteria. `set` with no arguments (cmd_set, + * also touched by this task's audit -- its two error paths were converted to + * stderr alongside the commands above) is a purely local, network-free + * success path: it always lists every known setting and returns 0. Its + * result data must appear on stdout, and this successful invocation must + * put nothing on stderr. + */ +static void test_set_list_success_result_on_stdout_not_stderr(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, "set")); + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, "masters", false, 5000), + "xce-test-cli-ux task 3.6, Requirement 3.2: bare 'set' must list " + "its known settings (result data) on stdout"); + TEST_ASSERT_NOT_NULL_MESSAGE( + strstr(out.data, "default:"), + "expected the 'set' listing's 'default:' column on stdout"); + + 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( + 0, exit_code, + "a successful bare 'set' must not mark the session failed"); + TEST_ASSERT_EQUAL_STRING_MESSAGE( + "", err.data, + "xce-test-cli-ux task 3.6, Requirement 3.1: a successful 'set' " + "invocation must put nothing diagnostic on stderr"); + + clitest_kill_and_reap(&sess); + clitest_close(&sess); + clitest_buf_free(&out); + clitest_buf_free(&err); +} + void test_test_cli(void) { UnitySetTestFile(__FILE__); @@ -2860,4 +3074,7 @@ void test_test_cli(void) RUN_TEST(test_help_reslist_shows_json_example); RUN_TEST(test_help_unknown_command_shows_corrected_grammar); RUN_TEST(test_help_context_scoped_not_found_no_cross_context_hint); + RUN_TEST(test_expect_jobs_malformed_count_diagnostic_on_stderr_not_stdout); + RUN_TEST(test_wait_primary_invalid_timeout_diagnostic_on_stderr_not_stdout); + RUN_TEST(test_set_list_success_result_on_stdout_not_stderr); }