feat(xce-test-cli-ux): build JSON list-output helper library

listfmt_json_begin/new_row/add_string/add_number/finish wrap cJSON
(already linked via struct2json_dep) into a minimal array/object
lifecycle library with no generic "record" abstraction -- each command's
own row struct maps its fields directly when wired in later (task 3.4).
Proven via a real stdout-capture-then-reparse round trip, including a
733-character field with no truncation, and stderr staying empty.
1306/1306 controller suite passing (+9).

Task 2.2 of .kiro/specs/xce-test-cli-ux/tasks.md.
Requirements: 5.1, 5.3

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
This commit is contained in:
Rongsong Shen 2026-09-09 12:13:12 +00:00
parent a2686516f7
commit 07d238eada
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
6 changed files with 464 additions and 2 deletions

View file

@ -61,7 +61,7 @@
- _Boundary: Confirmation Gate_
- _Depends: 1.1, 1.3_
- [ ] 2.2 (P) Build the JSON list-output helper library
- [x] 2.2 (P) Build the JSON list-output helper library
- Implement shared helpers for building a JSON array of row objects and printing it, usable by any listing command's existing per-row data
- Observable completion: the unit test suite shows a multi-row, multi-field test payload serializing to valid JSON with no field truncated regardless of value length
- _Requirements: 5.1, 5.3_
@ -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)
- `listfmt_json.c`'s tests only exercise power-of-two double values (Unity's `TEST_ASSERT_EQUAL_DOUBLE` is disabled project-wide, matching `test_host_runusage.c`/`test_e2e_capnp.c`); cJSON's real print path uses shortest-round-trippable-double formatting so fractional values are fine, but task 3.4 should add a real fractional-value (e.g. a resource value like 12.34) round-trip test once actual command fields are wired in, since nothing here proves that end-to-end yet. (task 2.2)
- `confirm_destructive()`'s EINTR-as-decline (9.9) works because it's a single, non-looping `fgets()` call — a naive "robust I/O" retry-on-EINTR wrapper would silently defeat this requirement. `g_interactive` is exposed via a new `xcetest_is_interactive()` accessor (declared in `confirm.h`, mirroring `xcetest_interrupted()`'s pattern) for task 3.3 to reuse. Task 3.3 must short-circuit around `confirm_destructive()` entirely (not pass a parameter into it) when a command's own `--yes` flag was given — the function has no bypass-flag parameter by design. (task 2.1)
- The `test_test_cli.c` black-box harness (`clitest_*` functions) is file-static and single-file by design (matching design.md's File Structure Plan and this directory's own no-split norm) — tasks 2.3, 3.5, 4.1-4.4 should add their black-box test functions directly into this same file rather than a new one. `clitest_wait_for_text()` is generic (arbitrary needle/stream) and safe to call repeatedly mid-session; `clitest_send_signal()` targets the child's real pid via `kill()`. (task 1.7)
- `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)

View file

@ -55,7 +55,7 @@ controller = executable('xce_controller',
link_args: wrap_link_args,
install: true)
xce_test_src = [ 'testsuite/test.c', 'testsuite/waldump.c', 'testsuite/walcompare.c', 'testsuite/snapverify.c', 'testsuite/snapdiff.c', 'testsuite/configcompare.c', 'testsuite/resval_typeresolve.c', 'testsuite/rcfile.c', 'testsuite/flagspec.c', 'testsuite/interruptcall.c', 'testsuite/confirm.c']
xce_test_src = [ 'testsuite/test.c', 'testsuite/waldump.c', 'testsuite/walcompare.c', 'testsuite/snapverify.c', 'testsuite/snapdiff.c', 'testsuite/configcompare.c', 'testsuite/resval_typeresolve.c', 'testsuite/rcfile.c', 'testsuite/flagspec.c', 'testsuite/interruptcall.c', 'testsuite/confirm.c', 'testsuite/listfmt_json.c']
# BSD-licensed readline replacement (scheduler-core spec, task 12.1 review
# finding): GNU readline is GPL; xce_test's REPL only needs readline()/
@ -176,6 +176,8 @@ controller_unit_test_src = [
'testsuite/interruptcall.c',
'testsuite/test_confirm.c',
'testsuite/confirm.c',
'testsuite/test_listfmt_json.c',
'testsuite/listfmt_json.c',
'testsuite/test_xcetest_process_entry.c',
'testsuite/test_test_cli.c',
'testsuite/test_persist_snapshot_entities.c',
@ -240,6 +242,8 @@ controller_unit_test_deps = [
zlib_dep,
c_capnproto_dep,
ssl_dep,
struct2json_dep, # testsuite/listfmt_json.c + test_listfmt_json.c: cJSON
# (xce-test-cli-ux task 2.2)
]
controller_unit_test = executable('controller_unit_test',

View file

@ -75,6 +75,7 @@ extern void test_flagspec(void);
extern void test_xcetest_interrupt(void);
extern void test_interruptcall(void);
extern void test_confirm(void);
extern void test_listfmt_json(void);
extern void test_xcetest_process_entry(void);
extern void test_test_cli(void);
extern void test_drive_async_config(void);
@ -261,6 +262,7 @@ int main(int argc, char* argv[])
TEST(test_xcetest_interrupt);
TEST(test_interruptcall);
TEST(test_confirm);
TEST(test_listfmt_json);
TEST(test_xcetest_process_entry);
TEST(test_test_cli);
TEST(test_drive_async_config);

View file

@ -0,0 +1,59 @@
#include "listfmt_json.h"
#include <stdio.h>
cJSON* listfmt_json_begin(void)
{
return cJSON_CreateArray();
}
void listfmt_json_add_string(cJSON* row, const char* key, const char* val)
{
if (!row || !key)
return;
if (val)
cJSON_AddStringToObject(row, key, val);
else
cJSON_AddNullToObject(row, key);
}
void listfmt_json_add_number(cJSON* row, const char* key, double val)
{
if (!row || !key)
return;
cJSON_AddNumberToObject(row, key, val);
}
cJSON* listfmt_json_new_row(cJSON* array)
{
if (!array)
return NULL;
cJSON* row = cJSON_CreateObject();
if (!row)
return NULL;
if (!cJSON_AddItemToArray(array, row)) {
cJSON_Delete(row);
return NULL;
}
return row;
}
void listfmt_json_finish(cJSON* array)
{
if (!array)
return;
char* text = cJSON_Print(array);
if (text) {
fputs(text, stdout);
fputc('\n', stdout);
cJSON_free(text);
}
cJSON_Delete(array);
}

View file

@ -0,0 +1,54 @@
#pragma once
/*
* listfmt_json.h -- shared cJSON array/object plumbing for xce_test's
* listing commands' JSON output mode (xce-test-cli-ux spec, task 2.2;
* Requirements 5.1, 5.3).
*
* This module owns ONLY the JSON array/object lifecycle: start an array,
* append a new row object to it, add string/number fields to a row, then
* print + free the whole array. It does NOT know about workerlist's/
* joblist's/reslist's own field sets -- each command's existing row
* struct already exists and is stable (design.md's Simplification lens),
* so this library introduces no generic "record"/"row" abstraction beyond
* the bare cJSON object handle itself. Wiring this into cmd_workerlist/
* cmd_joblist/cmd_reslist (their field mapping, their --json flag) is
* task 3.4, not this one.
*
* cJSON is already linked into xce_test (struct2json_dep, per
* research.md) and already used for job exdata blobs (test.c's
* build_job_envs()) -- this module reuses that exact same cJSON API
* surface, not a different one.
*/
#include <cjson/cJSON.h>
/* Creates a new, empty JSON array meant to hold one object per output
* row. The caller passes the returned array to listfmt_json_new_row()
* once per row, then to listfmt_json_finish() exactly once when done.
* Returns NULL on allocation failure. */
cJSON* listfmt_json_begin(void);
/* Adds a string field `key`: `val` to `row`. `val` is copied in full,
* regardless of its length -- no field is ever truncated or dropped for
* being "too long" (Requirement 5.3). A NULL `val` is stored as an
* explicit JSON null rather than silently omitted or crashing. No-op if
* `row` or `key` is NULL. */
void listfmt_json_add_string(cJSON* row, const char* key, const char* val);
/* Adds a numeric field `key`: `val` to `row`. No-op if `row` or `key` is
* NULL. */
void listfmt_json_add_number(cJSON* row, const char* key, double val);
/* Creates a new, empty JSON object, appends it to `array`, and returns
* that object so the caller can then call listfmt_json_add_string()/
* listfmt_json_add_number() on it for each of this row's fields. Returns
* NULL (leaving `array` unmodified) if `array` is NULL or the new object
* could not be allocated. */
cJSON* listfmt_json_new_row(cJSON* array);
/* Prints `array` to stdout as JSON -- command-RESULT data, per
* Requirement 3's stdout/stderr discipline, so it is written to stdout
* only, never stderr -- and then frees the entire cJSON structure
* (`array` and every row/field it holds). `array` must not be
* referenced again after this call. No-op if `array` is NULL. */
void listfmt_json_finish(cJSON* array);

View file

@ -0,0 +1,342 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <unity/unity.h>
#include "listfmt_json.h"
/* ---- stream-capture helpers --------------------------------------------
* Mirrors test_flagspec.c's dup/dup2/freopen stdout+stderr capture
* technique, so this test can prove Requirement 3's routing: JSON output
* is command-RESULT data and must land on stdout only, never stderr. Uses
* a fresh temp dir per test (mkdtemp) so capture files never collide with
* anything else in this shared /tmp. */
static char g_dir[256];
static void fresh_dir(void)
{
strcpy(g_dir, "/tmp/xce_listfmt_json_test_XXXXXX");
TEST_ASSERT_NOT_NULL(mkdtemp(g_dir));
}
static void cleanup_dir(void)
{
char cmd[512];
snprintf(cmd, sizeof(cmd), "rm -rf %s", g_dir);
system(cmd);
}
typedef struct {
int saved_stdout_fd;
int saved_stderr_fd;
char stdout_path[300];
char stderr_path[300];
} capture_t;
static void capture_begin(capture_t* cap)
{
fflush(stdout);
fflush(stderr);
cap->saved_stdout_fd = dup(fileno(stdout));
cap->saved_stderr_fd = dup(fileno(stderr));
TEST_ASSERT_TRUE(cap->saved_stdout_fd >= 0);
TEST_ASSERT_TRUE(cap->saved_stderr_fd >= 0);
snprintf(cap->stdout_path, sizeof(cap->stdout_path), "%s/out", g_dir);
snprintf(cap->stderr_path, sizeof(cap->stderr_path), "%s/err", g_dir);
TEST_ASSERT_NOT_NULL(freopen(cap->stdout_path, "w", stdout));
TEST_ASSERT_NOT_NULL(freopen(cap->stderr_path, "w", stderr));
}
/* Restores the real stdout/stderr fds and reads both captured files back
* into caller-provided, NUL-terminated, heap-allocated buffers (the long-
* string test needs more than a fixed small stack buffer can safely
* hold). Caller must free() both *out_stdout and *out_stderr. */
static void capture_end(capture_t* cap, char** out_stdout, char** out_stderr)
{
fflush(stdout);
fflush(stderr);
TEST_ASSERT_TRUE(dup2(cap->saved_stdout_fd, fileno(stdout)) >= 0);
TEST_ASSERT_TRUE(dup2(cap->saved_stderr_fd, fileno(stderr)) >= 0);
close(cap->saved_stdout_fd);
close(cap->saved_stderr_fd);
clearerr(stdout);
clearerr(stderr);
FILE* f;
long len;
f = fopen(cap->stdout_path, "r");
TEST_ASSERT_NOT_NULL(f);
fseek(f, 0, SEEK_END);
len = ftell(f);
fseek(f, 0, SEEK_SET);
*out_stdout = malloc((size_t)len + 1);
TEST_ASSERT_NOT_NULL(*out_stdout);
size_t n = fread(*out_stdout, 1, (size_t)len, f);
(*out_stdout)[n] = '\0';
fclose(f);
f = fopen(cap->stderr_path, "r");
TEST_ASSERT_NOT_NULL(f);
fseek(f, 0, SEEK_END);
len = ftell(f);
fseek(f, 0, SEEK_SET);
*out_stderr = malloc((size_t)len + 1);
TEST_ASSERT_NOT_NULL(*out_stderr);
n = fread(*out_stderr, 1, (size_t)len, f);
(*out_stderr)[n] = '\0';
fclose(f);
}
/* ---- lifecycle tests: array/object plumbing, no printing involved ----
* These build a cJSON tree directly with the module under test, inspect
* it with cJSON's own accessors, and clean up with a plain cJSON_Delete
* (NOT listfmt_json_finish(), which also prints -- these tests are about
* the tree shape, not stdout). */
static void test_begin_returns_empty_array(void)
{
cJSON* array = listfmt_json_begin();
TEST_ASSERT_NOT_NULL(array);
TEST_ASSERT_TRUE(cJSON_IsArray(array));
TEST_ASSERT_EQUAL_INT(0, cJSON_GetArraySize(array));
cJSON_Delete(array);
}
static void test_new_row_appends_object_and_returns_it(void)
{
cJSON* array = listfmt_json_begin();
TEST_ASSERT_NOT_NULL(array);
cJSON* row1 = listfmt_json_new_row(array);
TEST_ASSERT_NOT_NULL(row1);
TEST_ASSERT_TRUE(cJSON_IsObject(row1));
TEST_ASSERT_EQUAL_INT(1, cJSON_GetArraySize(array));
cJSON* row2 = listfmt_json_new_row(array);
TEST_ASSERT_NOT_NULL(row2);
TEST_ASSERT_TRUE(row2 != row1);
TEST_ASSERT_EQUAL_INT(2, cJSON_GetArraySize(array));
TEST_ASSERT_TRUE(cJSON_GetArrayItem(array, 0) == row1);
TEST_ASSERT_TRUE(cJSON_GetArrayItem(array, 1) == row2);
cJSON_Delete(array);
}
static void test_new_row_returns_null_on_null_array(void)
{
TEST_ASSERT_NULL(listfmt_json_new_row(NULL));
}
static void test_add_string_and_number_populate_row_fields(void)
{
cJSON* array = listfmt_json_begin();
cJSON* row = listfmt_json_new_row(array);
listfmt_json_add_string(row, "host", "worker-07");
listfmt_json_add_number(row, "ntasks", 4);
cJSON* host = cJSON_GetObjectItem(row, "host");
TEST_ASSERT_NOT_NULL(host);
TEST_ASSERT_TRUE(cJSON_IsString(host));
TEST_ASSERT_EQUAL_STRING("worker-07", cJSON_GetStringValue(host));
cJSON* ntasks = cJSON_GetObjectItem(row, "ntasks");
TEST_ASSERT_NOT_NULL(ntasks);
TEST_ASSERT_TRUE(cJSON_IsNumber(ntasks));
/* Unity is built without UNITY_INCLUDE_DOUBLE (this codebase's
* convention, see test_host_runusage.c/test_e2e_capnp.c) -- compare
* via TEST_ASSERT_EQUAL_FLOAT instead of the disabled DOUBLE variant. */
TEST_ASSERT_EQUAL_FLOAT(4.0f, (float)cJSON_GetNumberValue(ntasks));
cJSON_Delete(array);
}
static void test_add_string_and_number_are_noop_on_null_row(void)
{
/* Must not crash. */
listfmt_json_add_string(NULL, "key", "val");
listfmt_json_add_number(NULL, "key", 1.0);
}
static void test_add_string_null_value_stored_as_json_null(void)
{
cJSON* array = listfmt_json_begin();
cJSON* row = listfmt_json_new_row(array);
listfmt_json_add_string(row, "note", NULL);
cJSON* note = cJSON_GetObjectItem(row, "note");
TEST_ASSERT_NOT_NULL(note);
TEST_ASSERT_TRUE(cJSON_IsNull(note));
cJSON_Delete(array);
}
/* ---- listfmt_json_finish() tests: valid JSON, stdout-only, no
* truncation (task 2.2's stated Observable Completion criterion) ------- */
static void test_finish_noop_on_null_array(void)
{
fresh_dir();
capture_t cap;
capture_begin(&cap);
listfmt_json_finish(NULL);
char *out, *err;
capture_end(&cap, &out, &err);
TEST_ASSERT_EQUAL_STRING("", out);
TEST_ASSERT_EQUAL_STRING("", err);
free(out);
free(err);
cleanup_dir();
}
static void test_finish_prints_valid_multirow_multifield_json_to_stdout_only(void)
{
cJSON* array = listfmt_json_begin();
TEST_ASSERT_NOT_NULL(array);
/* Row 0: worker-like fields. */
cJSON* row0 = listfmt_json_new_row(array);
listfmt_json_add_string(row0, "host", "worker-01");
listfmt_json_add_string(row0, "status", "UP");
listfmt_json_add_number(row0, "maxmem", 67108864);
/* Row 1: a different field mix, to prove per-row field independence. */
cJSON* row1 = listfmt_json_new_row(array);
listfmt_json_add_string(row1, "host", "worker-02");
listfmt_json_add_string(row1, "status", "DOWN");
listfmt_json_add_number(row1, "maxmem", 134217728);
/* Row 2: a third row, with a numeric jobid-shaped field too. */
cJSON* row2 = listfmt_json_new_row(array);
listfmt_json_add_string(row2, "host", "worker-03");
listfmt_json_add_string(row2, "status", "UP");
listfmt_json_add_number(row2, "maxmem", 268435456);
listfmt_json_add_number(row2, "jobcount", 12);
fresh_dir();
capture_t cap;
capture_begin(&cap);
listfmt_json_finish(array);
char *out, *err;
capture_end(&cap, &out, &err);
/* Requirement 3: result data on stdout only, never stderr. */
TEST_ASSERT_EQUAL_STRING("", err);
TEST_ASSERT_TRUE(strlen(out) > 0);
/* Genuinely valid, parseable JSON -- don't just eyeball the string,
* parse it back with cJSON's own parser and confirm the round-trip. */
cJSON* parsed = cJSON_Parse(out);
TEST_ASSERT_NOT_NULL(parsed);
TEST_ASSERT_TRUE(cJSON_IsArray(parsed));
TEST_ASSERT_EQUAL_INT(3, cJSON_GetArraySize(parsed));
/* Unity is built without UNITY_INCLUDE_DOUBLE (this codebase's
* convention, see test_host_runusage.c/test_e2e_capnp.c) -- compare
* numeric fields via TEST_ASSERT_EQUAL_FLOAT instead of the disabled
* DOUBLE variant. Every value here is an exact power of two (or a
* small integer), so the float cast loses no precision. */
cJSON* p0 = cJSON_GetArrayItem(parsed, 0);
TEST_ASSERT_EQUAL_STRING(
"worker-01", cJSON_GetStringValue(cJSON_GetObjectItem(p0, "host")));
TEST_ASSERT_EQUAL_STRING(
"UP", cJSON_GetStringValue(cJSON_GetObjectItem(p0, "status")));
TEST_ASSERT_EQUAL_FLOAT(
67108864.0f,
(float)cJSON_GetNumberValue(cJSON_GetObjectItem(p0, "maxmem")));
cJSON* p1 = cJSON_GetArrayItem(parsed, 1);
TEST_ASSERT_EQUAL_STRING(
"worker-02", cJSON_GetStringValue(cJSON_GetObjectItem(p1, "host")));
TEST_ASSERT_EQUAL_STRING(
"DOWN", cJSON_GetStringValue(cJSON_GetObjectItem(p1, "status")));
TEST_ASSERT_EQUAL_FLOAT(
134217728.0f,
(float)cJSON_GetNumberValue(cJSON_GetObjectItem(p1, "maxmem")));
cJSON* p2 = cJSON_GetArrayItem(parsed, 2);
TEST_ASSERT_EQUAL_STRING(
"worker-03", cJSON_GetStringValue(cJSON_GetObjectItem(p2, "host")));
TEST_ASSERT_EQUAL_STRING(
"UP", cJSON_GetStringValue(cJSON_GetObjectItem(p2, "status")));
TEST_ASSERT_EQUAL_FLOAT(
268435456.0f,
(float)cJSON_GetNumberValue(cJSON_GetObjectItem(p2, "maxmem")));
TEST_ASSERT_EQUAL_FLOAT(
12.0f, (float)cJSON_GetNumberValue(cJSON_GetObjectItem(p2, "jobcount")));
cJSON_Delete(parsed);
free(out);
free(err);
cleanup_dir();
}
static void test_finish_does_not_truncate_long_string_field(void)
{
/* A deliberately long value -- several hundred characters, built from
* a repeating, easily-verified pattern -- to prove Requirement 5.3:
* no field is truncated or misaligned regardless of value length. */
enum { LONG_LEN = 733 };
char long_val[LONG_LEN + 1];
for (int i = 0; i < LONG_LEN; i++)
long_val[i] = (char)('a' + (i % 26));
long_val[LONG_LEN] = '\0';
cJSON* array = listfmt_json_begin();
cJSON* row = listfmt_json_new_row(array);
listfmt_json_add_string(row, "description", long_val);
listfmt_json_add_string(row, "host", "worker-09");
fresh_dir();
capture_t cap;
capture_begin(&cap);
listfmt_json_finish(array);
char *out, *err;
capture_end(&cap, &out, &err);
TEST_ASSERT_EQUAL_STRING("", err);
cJSON* parsed = cJSON_Parse(out);
TEST_ASSERT_NOT_NULL(parsed);
TEST_ASSERT_EQUAL_INT(1, cJSON_GetArraySize(parsed));
cJSON* p0 = cJSON_GetArrayItem(parsed, 0);
const char* desc =
cJSON_GetStringValue(cJSON_GetObjectItem(p0, "description"));
TEST_ASSERT_NOT_NULL(desc);
TEST_ASSERT_EQUAL_INT(LONG_LEN, (int)strlen(desc));
TEST_ASSERT_EQUAL_STRING(long_val, desc);
/* A field appearing after the long one in the same row must not have
* been clipped/misaligned by it either. */
TEST_ASSERT_EQUAL_STRING(
"worker-09", cJSON_GetStringValue(cJSON_GetObjectItem(p0, "host")));
cJSON_Delete(parsed);
free(out);
free(err);
cleanup_dir();
}
void test_listfmt_json(void)
{
UnitySetTestFile(__FILE__);
RUN_TEST(test_begin_returns_empty_array);
RUN_TEST(test_new_row_appends_object_and_returns_it);
RUN_TEST(test_new_row_returns_null_on_null_array);
RUN_TEST(test_add_string_and_number_populate_row_fields);
RUN_TEST(test_add_string_and_number_are_noop_on_null_row);
RUN_TEST(test_add_string_null_value_stored_as_json_null);
RUN_TEST(test_finish_noop_on_null_array);
RUN_TEST(test_finish_prints_valid_multirow_multifield_json_to_stdout_only);
RUN_TEST(test_finish_does_not_truncate_long_string_field);
}