fix(xce-test-cli-ux): fix bug-1711's display and text defects
Eight display/text defects in test.c: worker_status_str's "UNKOWN" typo, cmd_runtasks's "sequentail" typo (display-only, verified inert against the real launch_type consumer), cmd_reslist's right-justified columns and unlabeled hex Flags (now matching workerlist/state_raw's conventions), joblist's raw-epoch timestamps (now via a format_job_time helper delegating to the same formatter ctrlstatus already uses) and combined NTasks column split into MinTasks/MaxTasks, job_state_str's missing raw-state fallback for ambiguous RUN+EXIT (mirroring print_jobinfo's existing pattern), and getconf's no-op group/config list stubs (now genuinely implemented from already-fetched data, matching the three sibling print functions' convention). worker_status_str/job_state_str/format_job_time are pure functions with no networking/REPL/global-state dependency, extracted into a new joblist_display.c/.h module with a permanent 20-test standalone suite -- following this codebase's own resval_typeresolve.c/misc.c precedent for testing functions that would otherwise be stranded inside untестable test.c. cmd_reslist/joblist_query/cmd_getconf's table-printing bodies inline fetch+print and remain untestable without a live cluster. 1348/1348 across the full controller suite (+20), 0 regressions. Task 2.5 of .kiro/specs/xce-test-cli-ux/tasks.md. Requirements: 11.1, 11.3, 11.4, 11.5, 11.6, 11.7, 11.8 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
This commit is contained in:
parent
c709eba813
commit
a8154acb0a
6 changed files with 545 additions and 62 deletions
|
|
@ -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_<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)
|
||||
- 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)
|
||||
|
|
|
|||
|
|
@ -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'])
|
||||
|
||||
|
|
|
|||
99
src/controller/testsuite/joblist_display.c
Normal file
99
src/controller/testsuite/joblist_display.c
Normal file
|
|
@ -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 <stdio.h> /* 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;
|
||||
}
|
||||
81
src/controller/testsuite/joblist_display.h
Normal file
81
src/controller/testsuite/joblist_display.h
Normal file
|
|
@ -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 <stddef.h> /* size_t */
|
||||
#include <stdint.h> /* uint32_t */
|
||||
#include <time.h> /* 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 */
|
||||
|
|
@ -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[])
|
||||
{
|
||||
|
|
|
|||
256
src/controller/testsuite/test_joblist_display.c
Normal file
256
src/controller/testsuite/test_joblist_display.c
Normal file
|
|
@ -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 <stdint.h>
|
||||
#include <stdio.h>
|
||||
#include <string.h>
|
||||
|
||||
#include <unity/unity.h>
|
||||
|
||||
#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();
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue