diff --git a/.kiro/specs/xce-test-cli-ux/tasks.md b/.kiro/specs/xce-test-cli-ux/tasks.md index a1225910..a91aa2fc 100644 --- a/.kiro/specs/xce-test-cli-ux/tasks.md +++ b/.kiro/specs/xce-test-cli-ux/tasks.md @@ -82,7 +82,7 @@ - _Requirements: 13.1, 13.2, 13.3, 13.4, 13.5, 13.6_ - _Boundary: Source Command_ -- [ ] 2.5 (P) Fix bug-1711's display and text defects +- [x] 2.5 (P) Fix bug-1711's display and text defects - Correct the misspelled unrecognized-worker-status text and the misspelled default launch-type label (the malformed help-not-found grammar is fixed in 3.5, alongside that same code path's broader rework) - Left-justify the resource-listing command's text columns to match the worker-listing command's convention, and label/prefix its flags column as hexadecimal - Format the job-listing command's submit/start/finish time columns using the existing human-readable time formatting instead of raw epoch seconds, split its combined task-count column into two correctly labeled columns, and add a raw-state fallback for its ambiguous run-plus-exit state @@ -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) +- When a fix touches a pure function stuck inside `test.c` (no networking/REPL/global-state dependency), extract it into its own module with a standalone meson `test()` target rather than settling for a throwaway/manual verification — this codebase already has two precedents for exactly this (`resval_typeresolve.c`, `misc.c`'s runusage helpers; now also `joblist_display.c` for `worker_status_str`/`job_state_str`/`format_job_time`). `test.c` itself is never linked into `controller_unit_test`, so this extract-to-standalone-module pattern is the only way to get permanent automated coverage for such functions — a scratch/deleted verification harness is not sufficient and will be sent back on review. `cmd_reslist`/`joblist_query`/`cmd_getconf`'s table-printing bodies remain genuinely untestable without a live cluster (fetch+print are inlined together, confirmed on review) and stay uncovered by permanent tests for now. (task 2.5) - `cmd_source` translates `rcfile_load`'s raw positive "aborted" return to `-1` rather than forwarding it verbatim, because `preprocess_and_run` only treats a *negative* return as failure (`if (rc < 0)`) — forwarding the raw positive value would let an enclosing `source` line silently read a transitively-aborted nested source as success. No test yet exercises this specific nested-abort-propagation case (all 3 required scenarios pass either way) — worth a dedicated test in Validation phase (4.3 or nearby). (task 2.4) - libedit (this project's pinned NetBSD libedit 3.1) provides GNU-readline-compatible `rl_attempted_completion_function`/`rl_completion_matches` verbatim — confirmed against the actual linked header, not assumed. An ambiguous match needs a SECOND consecutive Tab to list candidates (standard readline/bash double-Tab convention) — later black-box tests using Tab must account for this. Also: `test_test_cli.c`'s `clitest_spawn()` disabling kernel ECHO also silently suppressed libedit's own completion feedback in this libedit build (bug-1792) — use the new `clitest_spawn_ex(sess, path, argv, disable_echo)` with `disable_echo=false` for any future test that needs to observe libedit UI behavior (completion, etc.), not just raw command output. (task 2.3) - `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) diff --git a/src/controller/meson.build b/src/controller/meson.build index 569c5484..df325eed 100644 --- a/src/controller/meson.build +++ b/src/controller/meson.build @@ -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', 'testsuite/listfmt_json.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/joblist_display.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()/ @@ -397,3 +397,29 @@ test('host runusage test', host_runusage_unit_test, timeout: 60, env: ['ASAN_OPTIONS=detect_leaks=0']) +# xce-test-cli-ux task 2.5 remediation: joblist_display.c is a small, +# dependency-light module (no networking/REPL/global state) extracted from +# test.c's worker_status_str()/job_state_str()/format_job_time() specifically +# so their bug-1711 display-formatting fixes can be unit-tested in isolation +# -- test.c itself cannot be white-box included into a Unity test, same +# rationale as resval_typeresolve_unit_test above. Standalone binary (not +# folded into controller_unit_test_src) since it has zero dependency on +# controller.c/cluster.c internals, mirroring resval_typeresolve_unit_test's/ +# host_runusage_unit_test's own standalone-binary rationale. +joblist_display_unit_test_src = [ + 'testsuite/test_joblist_display.c', + 'testsuite/joblist_display.c', +] + +joblist_display_unit_test = executable('joblist_display_unit_test', + joblist_display_unit_test_src, + include_directories: [incdir], + dependencies: controller_unit_test_deps, + c_args: ['-DXCE_TESTING'], + install: false) + +test('joblist display test', joblist_display_unit_test, + suite: 'controller', + timeout: 60, + env: ['ASAN_OPTIONS=detect_leaks=0']) + diff --git a/src/controller/testsuite/joblist_display.c b/src/controller/testsuite/joblist_display.c new file mode 100644 index 00000000..192bd4f4 --- /dev/null +++ b/src/controller/testsuite/joblist_display.c @@ -0,0 +1,99 @@ +/* joblist_display.c -- xce-test-cli-ux task 2.5 remediation (bug-1711 fixes) + * + * See joblist_display.h for the full rationale. Verbatim extraction of + * test.c's worker_status_str()/job_state_str()/format_job_time() -- no + * behavior change from the pre-extraction versions. + */ +#include "joblist_display.h" + +#include /* snprintf */ + +#include "ctrlstatus_fmt.h" /* ctrlstatus_format_time_ms */ +#include "xce/types.h" /* XCE_WORKER_*, JOB_STAT_* bit definitions */ + +char* worker_status_str(uint32_t status) +{ + /* Admin-set CLOSED takes priority over the dynamic UNAVAIL bit: the + * config-upgrade-snapshot soak (task 8.6) needs to assert a host's + * administratively-set status specifically, and a host closed via + * hostctrl that later disconnects (e.g. its xce_test `worker`/`join` + * connection exits after the script ends) would otherwise print only + * "UNAVAIL", masking the admin bit this check exists to verify. */ + if ((status & XCE_WORKER_CLOSED) != 0) { + return "CLOSED"; + } else if ((status & XCE_WORKER_UNAVAIL) != 0) { + return "UNAVAIL"; + } else if ((status & XCE_WORKER_LOCKEDU) != 0) { + return "LOCKU"; + } else if ((status & XCE_WORKER_LOCKEDW) != 0) { + return "LOCKW"; + } else if ((status & XCE_WORKER_BUSY) != 0) { + return "BUSY"; + } else if (status == 0) { + return "OK"; + } + + return "UNKNOWN"; +} + +char* job_state_str(int state) +{ + /* bug-1711 (Requirement 11.7): raw-state fallback mirroring + * print_jobinfo's existing state_raw guard against the same ambiguity + * (see print_jobinfo's comment above its "state_raw: 0x%x" print) -- a + * job that is simultaneously RUN and EXIT (e.g. a worker-reported FAIL + * received without a prior FINISH; apply_job_fail_transition ORs in + * EXIT|PERR without clearing RUN) would otherwise silently print as + * plain "RUN" via the if/else-if priority ordering below, hiding the + * EXIT bit. Unlike print_jobinfo (which always has room for a separate + * state_raw line), job_state_str's result is a single cmd_joblist table + * cell, so the raw state is folded into the same string only for this + * specific ambiguous case. */ + static char ambiguous_buf[32]; + + if ((state & JOB_STAT_PEND) != 0) { + if ((state & JOB_STAT_PSUSP) != 0) { + return "PSUSP"; + } + + return "PEND"; + } else if ((state & JOB_STAT_RUN) != 0) { + if ((state & JOB_STAT_USUSP) != 0) { + return "USUSP"; + } else if ((state & JOB_STAT_SSUSP) != 0) { + return "SSUSP"; + } else if ((state & JOB_STAT_EXIT) != 0) { + snprintf(ambiguous_buf, sizeof(ambiguous_buf), "RUN(0x%x)", state); + return ambiguous_buf; + } + return "RUN"; + } else if ((state & JOB_STAT_EXIT) != 0) { + return "EXIT"; + } else if ((state & JOB_STAT_DONE) != 0) { + return "DONE"; + } else if ((state & JOB_STAT_ERR) != 0) { + return "ERROR"; + } + + return "UNKNOWN"; +} + +/* bug-1711 (Requirement 11.5): format a job time_t field (epoch seconds) using + * ctrlstatus_fmt.h's ctrlstatus_format_time_ms human-readable formatter + * (which takes epoch milliseconds), instead of printing the raw epoch-seconds + * value directly as cmd_joblist did before. A job time of 0 means the + * corresponding event has not happened yet (e.g. an unstarted job's + * start_time/finish_time) -- print "-" rather than a misleading 1970-01-01 + * epoch date. */ +const char* format_job_time(time_t t, char* buf, size_t bufsize) +{ + if (t == 0) { + return "-"; + } + + if (ctrlstatus_format_time_ms((uint64_t)t * 1000u, buf, bufsize) == NULL) { + return "-"; + } + + return buf; +} diff --git a/src/controller/testsuite/joblist_display.h b/src/controller/testsuite/joblist_display.h new file mode 100644 index 00000000..20205285 --- /dev/null +++ b/src/controller/testsuite/joblist_display.h @@ -0,0 +1,81 @@ +#ifndef JOBLIST_DISPLAY_H +#define JOBLIST_DISPLAY_H + +/* joblist_display.h -- xce-test-cli-ux task 2.5 remediation (bug-1711 fixes) + * + * Extracted from test.c's worker_status_str()/job_state_str()/ + * format_job_time(): pure, dependency-light display-formatting helpers, with + * NO networking, REPL, or global-session-state dependency (job_state_str's + * only state is a function-local `static` scratch buffer, used solely to + * render its own return value -- no cross-call or cross-session state). + * This isolation is what makes them unit-testable outside of test.c, which + * cannot be white-box #include'd into a Unity test (thousands of lines, its + * own main(), heavy libedit/network dependencies) -- same rationale as + * resval_typeresolve.c's extraction from set_resval() (resource-value- + * verification task 1.1) and misc.c's update_host_runusage()/ + * aggregate_job_runusage() (bug-1735). + * + * test.c calls these directly at their existing call sites (cmd_workerlist, + * joblist_query, print_jobinfo) -- no behavior change, extraction only. + */ + +#include /* size_t */ +#include /* uint32_t */ +#include /* time_t */ + +/* Render a worker's status bitmask (xce_worker_t.status, XCE_WORKER_* bits + * in xce/types.h) as a short display string for cmd_workerlist's Status + * column. + * + * Priority order (first match wins, unchanged from the pre-extraction + * logic): CLOSED > UNAVAIL > LOCKEDU > LOCKEDW > BUSY > (status == 0) OK. + * Admin-set CLOSED intentionally takes priority over the dynamic UNAVAIL + * bit: a host closed via hostctrl that later disconnects would otherwise + * print only "UNAVAIL", masking the admin bit this check exists to verify + * (config-upgrade-snapshot soak, task 8.6). + * + * Any other nonzero bit combination not covered above returns "UNKNOWN" + * (Requirement 11.1 -- bug-1711's fix; previously misspelled "UNKOWN"). + * + * Returns a string literal -- never a caller-owned buffer, safe to use + * across calls with no ownership/lifetime concerns. */ +char* worker_status_str(uint32_t status); + +/* Render a job's state bitmask (xce_job_t.state, JOB_STAT_* bits in + * xce/types.h) as a short display string for cmd_joblist's State column + * (joblist_query) and print_jobinfo's "state:" line. + * + * Priority order (first match wins, unchanged from the pre-extraction + * logic): PEND (+PSUSP) > RUN (+USUSP/+SSUSP/+EXIT) > EXIT > DONE > ERROR > + * UNKNOWN. + * + * Requirement 11.7 (bug-1711): a job that is simultaneously RUN and EXIT + * (e.g. a worker-reported FAIL received without a prior FINISH; + * apply_job_fail_transition ORs in EXIT|PERR without clearing RUN) is + * reported via a raw-state fallback "RUN(0x%x)", consistent with the + * fallback print_jobinfo already applies for that same ambiguity via its + * separate state_raw line -- checked AFTER USUSP/SSUSP in the same + * else-if chain, so it never masks either of those. + * + * Returns a string literal for every branch except the RUN+EXIT fallback, + * which is rendered into a function-local `static` scratch buffer valid + * until the next call (the same lifetime contract test.c relied on before + * extraction). */ +char* job_state_str(int state); + +/* Format a job time_t field (epoch seconds) using ctrlstatus_fmt.h's + * ctrlstatus_format_time_ms human-readable formatter (which takes epoch + * milliseconds) into `buf` (at least 24 bytes -- see + * ctrlstatus_format_time_ms's own size contract). + * + * A job time of 0 means the corresponding event has not happened yet (e.g. + * an unstarted job's start_time/finish_time) -- returns "-" rather than a + * misleading 1970-01-01 epoch date (Requirement 11.5). Also returns "-" if + * the underlying formatter itself fails (e.g. `buf`/`bufsize` too small). + * + * Returns `buf` on success (nonzero `t`), or a string literal ("-") for the + * zero/failure cases -- callers must not assume the return value is always + * `buf`. */ +const char* format_job_time(time_t t, char* buf, size_t bufsize); + +#endif /* JOBLIST_DISPLAY_H */ diff --git a/src/controller/testsuite/test.c b/src/controller/testsuite/test.c index 912444f4..8dfdb29f 100644 --- a/src/controller/testsuite/test.c +++ b/src/controller/testsuite/test.c @@ -33,6 +33,8 @@ #include "snapverify.h" /* offline checkpoint-file structural validator (cmd_snapverify) */ #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 "joblist_display.h" /* worker_status_str/job_state_str/format_job_time + * (xce-test-cli-ux task 2.5 remediation) */ #include "rcfile.h" /* startup rc-file path resolution + loading (xce-test-rc-file task 2.2) */ #include "resval_typeresolve.h" /* resval_resolve_type (resource-value-verification task 2.1) */ #include "xce/cluster.h" /* xce_parse_peers, xce_cluster_config_t */ @@ -1719,10 +1721,23 @@ int cmd_reslist(int argc, char* argv[]) return rc; } - printf("%10s%10s%10s%64s\n", "Name", "Flags", "Interval", "Description"); + /* Note the extra literal space before the Description specifier: unlike + * cmd_workerlist's all-left-justified text columns (which self-separate + * via their own trailing padding), Interval stays right-justified here, + * so without an explicit separator a short Interval value would run + * directly into the now-left-justified Description text. */ + printf("%-10s%10s%10s %-64s\n", "Name", "Flags", "Interval", + "Description"); for (i = 0; i < rc; i++) { - printf("%10s%10x%10d%64s\n", res[i]->name, res[i]->flags, + /* bug-1711 (Requirement 11.4): prefix as hex ("0x%x"), matching this + * file's own state_raw convention (print_jobinfo), instead of an + * unlabeled bare %x that is ambiguous vs decimal. */ + char flags_buf[16]; + + snprintf(flags_buf, sizeof(flags_buf), "0x%x", res[i]->flags); + + printf("%-10s%10s%10d %-64s\n", res[i]->name, flags_buf, res[i]->interval, res[i]->desc); } @@ -1731,30 +1746,12 @@ int cmd_reslist(int argc, char* argv[]) return 0; } -static char* worker_status_str(uint32_t status) -{ - /* Admin-set CLOSED takes priority over the dynamic UNAVAIL bit: the - * config-upgrade-snapshot soak (task 8.6) needs to assert a host's - * administratively-set status specifically, and a host closed via - * hostctrl that later disconnects (e.g. its xce_test `worker`/`join` - * connection exits after the script ends) would otherwise print only - * "UNAVAIL", masking the admin bit this check exists to verify. */ - if ((status & XCE_WORKER_CLOSED) != 0) { - return "CLOSED"; - } else if ((status & XCE_WORKER_UNAVAIL) != 0) { - return "UNAVAIL"; - } else if ((status & XCE_WORKER_LOCKEDU) != 0) { - return "LOCKU"; - } else if ((status & XCE_WORKER_LOCKEDW) != 0) { - return "LOCKW"; - } else if ((status & XCE_WORKER_BUSY) != 0) { - return "BUSY"; - } else if (status == 0) { - return "OK"; - } - - return "UNKOWN"; -} +/* worker_status_str() -- extracted to joblist_display.c/.h (xce-test-cli-ux + * task 2.5 remediation): pure, dependency-light display formatting, no + * networking/REPL/global-state coupling, so it can be unit-tested outside + * of this file (see test_joblist_display.c), mirroring set_resval()'s own + * extraction to resval_typeresolve.c below. Call sites are unchanged -- + * they now resolve through joblist_display.h's declaration. */ /* forward decl: defined near cmd_resval, below, which also uses it */ static void format_bytes_human(double bytes, char* buf, size_t bufsize); @@ -1804,31 +1801,11 @@ int cmd_workerlist(int argc, char* argv[]) return 0; } -static char* job_state_str(int state) -{ - if ((state & JOB_STAT_PEND) != 0) { - if ((state & JOB_STAT_PSUSP) != 0) { - return "PSUSP"; - } - - return "PEND"; - } else if ((state & JOB_STAT_RUN) != 0) { - if ((state & JOB_STAT_USUSP) != 0) { - return "USUSP"; - } else if ((state & JOB_STAT_SSUSP) != 0) { - return "SSUSP"; - } - return "RUN"; - } else if ((state & JOB_STAT_EXIT) != 0) { - return "EXIT"; - } else if ((state & JOB_STAT_DONE) != 0) { - return "DONE"; - } else if ((state & JOB_STAT_ERR) != 0) { - return "ERROR"; - } - - return "UNKNOWN"; -} +/* job_state_str()/format_job_time() -- extracted to joblist_display.c/.h + * (xce-test-cli-ux task 2.5 remediation), same rationale as + * worker_status_str() above: pure, dependency-light display formatting, + * unit-tested outside of this file in test_joblist_display.c. Call sites + * (below, and print_jobinfo's "state:" line) are unchanged. */ /* Shared, print-free joblist core for `cmd_joblist` and the `expect-jobs` / * `expect-job` assertions (xce-test-e2e-primitives task 5.1, design.md:81, @@ -1869,9 +1846,9 @@ static int joblist_query(const char* find_jobid, int* contains, int print_table, } if (print_table) { - printf("%-26s%-10s%-10s%-8s%-16s%-16s%-16s%10s%30s\n", "JobId", "User", - "Queue", "State", "SubmitTime", "StartTime", "FinishTime", - "NTasks", "Command"); + printf("%-26s%-10s%-10s%-8s%-24s%-24s%-24s%12s%12s%30s\n", "JobId", + "User", "Queue", "State", "SubmitTime", "StartTime", + "FinishTime", "MinTasks", "MaxTasks", "Command"); } for (i = 0; i < rc; i++) { @@ -1885,10 +1862,19 @@ static int joblist_query(const char* find_jobid, int* contains, int print_table, } if (print_table) { - printf("%-26s%-10s%-10s%-8s%-16ld%-16ld%-16ld%4d,%-5d%30s\n", buf, + char submit_buf[32]; + char start_buf[32]; + char finish_buf[32]; + + printf("%-26s%-10s%-10s%-8s%-24s%-24s%-24s%12d%12d%30s\n", buf, jobs[i]->submit->user, jobs[i]->submit->queue, - job_state_str(jobs[i]->state), jobs[i]->queue_time, - jobs[i]->start_time, jobs[i]->finish_time, + job_state_str(jobs[i]->state), + format_job_time(jobs[i]->queue_time, submit_buf, + sizeof(submit_buf)), + format_job_time(jobs[i]->start_time, start_buf, + sizeof(start_buf)), + format_job_time(jobs[i]->finish_time, finish_buf, + sizeof(finish_buf)), jobs[i]->submit->mintasks, jobs[i]->submit->maxtasks, jobs[i]->submit->start_info->cmdline); } @@ -3257,7 +3243,7 @@ int cmd_runtasks(int argc, char* argv[]) if (totaltasks > 1) { hosts[i].launch_type = strdup("mpi"); } else { - hosts[i].launch_type = strdup("sequentail"); + hosts[i].launch_type = strdup("sequential"); } } } @@ -3950,9 +3936,43 @@ static int print_workerlist(xce_workerlist_t wlist) return 0; } -static int print_grouplist(xce_group_configlist_t glist) { return 0; } +/* bug-1711 (Requirement 11.8): print the already-fetched group records + * (conf->uglist / conf->hglist) instead of the previous no-op stub, which + * always left the "User Groups"/"Host Groups" header followed by silent + * nothing -- indistinguishable from a genuinely empty cluster. Mirrors + * print_reslist/print_queuelist/print_workerlist's own one-record-per-block + * convention just above. */ +static int print_grouplist(xce_group_configlist_t glist) +{ + int i, j; -static int print_configlist(xce_configlist_t clist) { return 0; } + for (i = 0; i < glist.count; i++) { + printf("name: %s\nmembers:", glist.groups[i]->name); + for (j = 0; j < glist.groups[i]->members.count; j++) { + printf(" %s", glist.groups[i]->members.data[j]); + } + printf("\nex_members:"); + for (j = 0; j < glist.groups[i]->ex_members.count; j++) { + printf(" %s", glist.groups[i]->ex_members.data[j]); + } + printf("\n\n"); + } + return 0; +} + +/* bug-1711 (Requirement 11.8): print the already-fetched config records + * (conf->cfglist) instead of the previous no-op stub -- see print_grouplist + * above for the same fix applied to the "Configs" section. */ +static int print_configlist(xce_configlist_t clist) +{ + int i; + + for (i = 0; i < clist.count; i++) { + printf("name: %s\nvalue: %s\n\n", clist.configs[i]->name, + clist.configs[i]->value); + } + return 0; +} int cmd_getconf(int argc, char* argv[]) { diff --git a/src/controller/testsuite/test_joblist_display.c b/src/controller/testsuite/test_joblist_display.c new file mode 100644 index 00000000..7b9f0911 --- /dev/null +++ b/src/controller/testsuite/test_joblist_display.c @@ -0,0 +1,256 @@ +/* test_joblist_display.c -- xce-test-cli-ux task 2.5 remediation + * + * Unity unit tests for joblist_display.c's worker_status_str()/ + * job_state_str()/format_job_time(), extracted from test.c (see + * joblist_display.h for the full extraction rationale). All three fixes + * these functions carry (bug-1711) were already verified correct by review + * against the pre-extraction code; this file is the permanent, automated + * proof that (a) the fixes behave as intended and (b) the extraction did not + * change any observed behavior -- in particular, that the pre-existing + * branches these fixes sit alongside do NOT regress into the new fallback + * branches. + * + * Standalone Unity main (this binary is its own test executable -- see + * meson.build's joblist_display_unit_test), mirroring + * resval_typeresolve_unit_test's/host_runusage_unit_test's own pattern. + */ +#include +#include +#include + +#include + +#include "ctrlstatus_fmt.h" /* ctrlstatus_format_time_ms -- used directly + * below to prove format_job_time() genuinely + * delegates to it (byte-identical output), + * rather than a parallel reimplementation that + * could silently drift. */ +#include "joblist_display.h" +#include "xce/types.h" /* XCE_WORKER_*, JOB_STAT_* bit definitions */ + +/* ------------------------------------------------------------------ */ +/* worker_status_str() */ +/* ------------------------------------------------------------------ */ + +/* Requirement 11.1's non-regression proof: admin-set CLOSED must still take + * priority over the dynamic UNAVAIL bit when both are set (task 8.6's + * config-upgrade-snapshot soak relies on this precedence). */ +static void test_worker_status_closed_over_unavail_priority(void) +{ + char* rc = worker_status_str(XCE_WORKER_CLOSED | XCE_WORKER_UNAVAIL); + + TEST_ASSERT_EQUAL_STRING_MESSAGE( + "CLOSED", rc, + "CLOSED must take priority over UNAVAIL when both bits are set"); +} + +static void test_worker_status_unavail_alone(void) +{ + TEST_ASSERT_EQUAL_STRING("UNAVAIL", + worker_status_str(XCE_WORKER_UNAVAIL)); +} + +static void test_worker_status_lockedu_alone(void) +{ + TEST_ASSERT_EQUAL_STRING("LOCKU", worker_status_str(XCE_WORKER_LOCKEDU)); +} + +static void test_worker_status_lockedw_alone(void) +{ + TEST_ASSERT_EQUAL_STRING("LOCKW", worker_status_str(XCE_WORKER_LOCKEDW)); +} + +static void test_worker_status_busy_alone(void) +{ + TEST_ASSERT_EQUAL_STRING("BUSY", worker_status_str(XCE_WORKER_BUSY)); +} + +static void test_worker_status_zero_is_ok(void) +{ + TEST_ASSERT_EQUAL_STRING("OK", worker_status_str(0)); +} + +/* Requirement 11.1 (bug-1711's actual fix): an unrecognized status bit + * (XCE_WORKER_CLOSED_BY_ADMIN -- a real bit in xce/types.h, but not one + * worker_status_str() checks for) must fall through to "UNKNOWN", correctly + * spelled -- previously misspelled "UNKOWN". */ +static void test_worker_status_unrecognized_bit_is_unknown(void) +{ + char* rc = worker_status_str(XCE_WORKER_CLOSED_BY_ADMIN); + + TEST_ASSERT_EQUAL_STRING_MESSAGE( + "UNKNOWN", rc, + "an unrecognized status bit must report correctly-spelled UNKNOWN " + "(Requirement 11.1)"); +} + +/* ------------------------------------------------------------------ */ +/* job_state_str() */ +/* ------------------------------------------------------------------ */ + +static void test_job_state_pend(void) +{ + TEST_ASSERT_EQUAL_STRING("PEND", job_state_str(JOB_STAT_PEND)); +} + +static void test_job_state_pend_psusp(void) +{ + TEST_ASSERT_EQUAL_STRING( + "PSUSP", job_state_str(JOB_STAT_PEND | JOB_STAT_PSUSP)); +} + +static void test_job_state_run(void) +{ + TEST_ASSERT_EQUAL_STRING("RUN", job_state_str(JOB_STAT_RUN)); +} + +/* Non-regression proof (review's specific request): RUN+USUSP must still + * resolve to "USUSP", not fall through to the new RUN+EXIT fallback. */ +static void test_job_state_run_ususp(void) +{ + TEST_ASSERT_EQUAL_STRING_MESSAGE( + "USUSP", job_state_str(JOB_STAT_RUN | JOB_STAT_USUSP), + "RUN+USUSP must not regress into the new RUN+EXIT fallback"); +} + +/* Non-regression proof (review's specific request): RUN+SSUSP must still + * resolve to "SSUSP", not fall through to the new RUN+EXIT fallback. */ +static void test_job_state_run_ssusp(void) +{ + TEST_ASSERT_EQUAL_STRING_MESSAGE( + "SSUSP", job_state_str(JOB_STAT_RUN | JOB_STAT_SSUSP), + "RUN+SSUSP must not regress into the new RUN+EXIT fallback"); +} + +/* Requirement 11.7 (bug-1711's actual fix): RUN+EXIT (e.g. a worker-reported + * FAIL received without a prior FINISH) must report the raw-state fallback + * "RUN(0x%x)", mirroring print_jobinfo's own state_raw guard for the same + * ambiguity, instead of silently printing plain "RUN". */ +static void test_job_state_run_exit_fallback(void) +{ + int state = JOB_STAT_RUN | JOB_STAT_EXIT; + char expected[32]; + char* rc = job_state_str(state); + + snprintf(expected, sizeof(expected), "RUN(0x%x)", state); + + TEST_ASSERT_EQUAL_STRING_MESSAGE( + expected, rc, + "RUN+EXIT must report the raw-state fallback RUN(0x%x) " + "(Requirement 11.7)"); +} + +static void test_job_state_exit(void) +{ + TEST_ASSERT_EQUAL_STRING("EXIT", job_state_str(JOB_STAT_EXIT)); +} + +static void test_job_state_done(void) +{ + TEST_ASSERT_EQUAL_STRING("DONE", job_state_str(JOB_STAT_DONE)); +} + +static void test_job_state_error(void) +{ + TEST_ASSERT_EQUAL_STRING("ERROR", job_state_str(JOB_STAT_ERR)); +} + +/* Default/unknown branch: no recognized bit set at all. */ +static void test_job_state_unknown_default(void) +{ + TEST_ASSERT_EQUAL_STRING("UNKNOWN", job_state_str(0)); +} + +/* ------------------------------------------------------------------ */ +/* format_job_time() */ +/* ------------------------------------------------------------------ */ + +/* Requirement 11.5: a zero job time_t (event has not happened yet) must + * print "-", not a misleading 1970-01-01 epoch date. */ +static void test_format_job_time_zero_is_dash(void) +{ + char buf[32]; + const char* rc = format_job_time((time_t)0, buf, sizeof(buf)); + + TEST_ASSERT_EQUAL_STRING("-", rc); +} + +/* Requirement 11.5's delegation proof: format_job_time() is supposed to be a + * thin seconds->milliseconds adapter directly in front of + * ctrlstatus_format_time_ms() -- this asserts the two are BYTE-IDENTICAL for + * the equivalent millisecond input, proving genuine delegation rather than a + * parallel reimplementation that could silently drift from ctrlstatus's own + * formatting. */ +static void test_format_job_time_matches_ctrlstatus_formatter(void) +{ + time_t t = (time_t)1700000000; /* arbitrary fixed nonzero epoch */ + char via_format_job_time[32]; + char via_ctrlstatus_direct[32]; + const char* rc1; + char* rc2; + + rc1 = format_job_time(t, via_format_job_time, sizeof(via_format_job_time)); + rc2 = ctrlstatus_format_time_ms((uint64_t)t * 1000u, + via_ctrlstatus_direct, + sizeof(via_ctrlstatus_direct)); + + TEST_ASSERT_NOT_NULL(rc1); + TEST_ASSERT_NOT_NULL(rc2); + TEST_ASSERT_EQUAL_STRING_MESSAGE( + rc2, rc1, + "format_job_time() must be byte-identical to a direct " + "ctrlstatus_format_time_ms() call on the equivalent millisecond " + "input -- it must delegate, not reimplement"); +} + +/* Bonus branch coverage: format_job_time()'s own "-" fallback when the + * underlying formatter declines (buffer too small per + * ctrlstatus_format_time_ms's documented >= 24-byte contract). */ +static void test_format_job_time_buffer_too_small_is_dash(void) +{ + char buf[4]; /* below ctrlstatus_format_time_ms's 24-byte floor */ + const char* rc = format_job_time((time_t)1700000000, buf, sizeof(buf)); + + TEST_ASSERT_EQUAL_STRING("-", rc); +} + +/* ------------------------------------------------------------------ */ +/* Standalone Unity main (this binary is its own test executable -- see */ +/* meson.build's joblist_display_unit_test) */ +/* ------------------------------------------------------------------ */ + +void setUp(void) { return; } +void tearDown(void) { return; } + +int main(void) +{ + printf("\n>> xce-test-cli-ux task 2.5 remediation: joblist_display " + "worker_status_str/job_state_str/format_job_time\n\n"); + + UNITY_BEGIN(); + + RUN_TEST(test_worker_status_closed_over_unavail_priority); + RUN_TEST(test_worker_status_unavail_alone); + RUN_TEST(test_worker_status_lockedu_alone); + RUN_TEST(test_worker_status_lockedw_alone); + RUN_TEST(test_worker_status_busy_alone); + RUN_TEST(test_worker_status_zero_is_ok); + RUN_TEST(test_worker_status_unrecognized_bit_is_unknown); + + RUN_TEST(test_job_state_pend); + RUN_TEST(test_job_state_pend_psusp); + RUN_TEST(test_job_state_run); + RUN_TEST(test_job_state_run_ususp); + RUN_TEST(test_job_state_run_ssusp); + RUN_TEST(test_job_state_run_exit_fallback); + RUN_TEST(test_job_state_exit); + RUN_TEST(test_job_state_done); + RUN_TEST(test_job_state_error); + RUN_TEST(test_job_state_unknown_default); + + RUN_TEST(test_format_job_time_zero_is_dash); + RUN_TEST(test_format_job_time_matches_ctrlstatus_formatter); + RUN_TEST(test_format_job_time_buffer_too_small_is_dash); + + return UNITY_END(); +}