lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(xce-test-cli-ux): fix bug-1557's join maxmem default

Browse source 
cmd_join_worker's simulated-worker maxmem default was 64 bytes -- a
leftover from when the field was MB-scale, unfixed when maxmem became
bytes-canonical elsewhere. Now 64 * 1024 * 1024. Extracted the
argument-parsing/defaulting logic into join_worker_parse_attrs()
(join_worker_args.c), since it runs entirely before the function's
network call and is a genuine testable seam -- same precedent as
task 2.5's joblist_display.c. Verified live against a real deployed
cluster (workerlist reports MaxMem: 64.00 MB) plus a permanent 5-test
regression suite. maxswap/maxtmp have the identical stale-default bug
(logged as bug-1793) but are deliberately left unfixed here since
USAGE_JOIN_WORKER's help text currently documents their stale values
accurately -- fixing only the code would create a new docs mismatch;
a locked-in sibling-defaults test documents this for the follow-up.
1353/1353 across the full controller suite (+5), 0 regressions.

Task 2.6 of .kiro/specs/xce-test-cli-ux/tasks.md.
Requirements: 12.1

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 16:24:36 +00:00
parent a8154acb0a
commit 878a42fb48
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
7 changed files with 304 additions and 40 deletions
Download patch file Download diff file Expand all files Collapse all files

28
src/controller/meson.build
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/joblist_display.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/join_worker_args.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()/
@ -423,3 +423,29 @@ test('joblist display test', joblist_display_unit_test,
timeout: 60,
env: ['ASAN_OPTIONS=detect_leaks=0'])
# xce-test-cli-ux task 2.6 remediation: join_worker_args.c is a small,
# dependency-light module (no networking/REPL/global state) extracted from
# test.c's cmd_join_worker() specifically so the bug-1557 maxmem-default fix
# can be unit-tested in isolation -- test.c itself cannot be white-box
# included into a Unity test, same rationale as joblist_display_unit_test/
# 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 those tests' own
# standalone-binary rationale.
join_worker_args_unit_test_src = [
'testsuite/test_join_worker_args.c',
'testsuite/join_worker_args.c',
]
join_worker_args_unit_test = executable('join_worker_args_unit_test',
join_worker_args_unit_test_src,
include_directories: [incdir],
dependencies: controller_unit_test_deps,
c_args: ['-DXCE_TESTING'],
install: false)
test('join worker args test', join_worker_args_unit_test,
suite: 'controller',
timeout: 60,
env: ['ASAN_OPTIONS=detect_leaks=0'])

70
src/controller/testsuite/join_worker_args.c Normal file
View file

@ -0,0 +1,70 @@
/* join_worker_args.c -- xce-test-cli-ux task 2.6 remediation (bug-1557 fix)
*
* See join_worker_args.h for the full extraction rationale. Verbatim
* extraction of test.c's cmd_join_worker() parsing/defaulting block -- no
* behavior change from the pre-extraction version.
*/
#include "join_worker_args.h"
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h> /* getopt, optarg, optind */
int join_worker_parse_attrs(int argc, char* argv[], hostattrs_t* attrs)
{
int opt;
if (attrs == NULL) {
return -EINVAL;
}
/* bug-1557: maxmem is bytes-canonical (see resources.yaml, hardware_
* monitor.c's total_memory) -- this default used to read plain "64"
* back when maxmem was MB-scale (64 MB); an earlier session converted
* the codebase's canonical unit to bytes but missed this test-tool
* default, leaving it at a nonsensical 64 BYTES. Expressed here as
* 64 * 1024 * 1024 (64MB in bytes) to preserve the original intent,
* matching this file's own byte-constant convention (see test.c's
* main() `logsize = 64 * 1024 * 1024`). */
attrs->cpufactor = 1.0;
attrs->maxcpus = 4;
attrs->maxmem = 64 * 1024 * 1024;
attrs->maxswap = 1024;
attrs->maxtmp = 2048;
attrs->ndisks = 1;
attrs->hostmodel = "Intel64";
attrs->hosttype = "LINUX64";
optind = 1;
while ((opt = getopt(argc, argv, "f:c:m:s:t:d:")) != -1) {
switch (opt) {
case 'f':
attrs->cpufactor = atof(optarg);
break;
case 'c':
attrs->maxcpus = atoi(optarg);
break;
case 'm':
attrs->maxmem = atoi(optarg);
break;
case 's':
attrs->maxswap = atoi(optarg);
break;
case 't':
attrs->maxtmp = atoi(optarg);
break;
case 'd':
attrs->ndisks = atoi(optarg);
break;
default:
printf(" join [-f cpufactor] [-c maxcpus] [-m maxmem] [-s maxswap] "
"[-t maxtmp] [-d ndisks]\n");
return -EINVAL;
}
}
return 0;
}

40
src/controller/testsuite/join_worker_args.h Normal file
View file

@ -0,0 +1,40 @@
#ifndef JOIN_WORKER_ARGS_H
#define JOIN_WORKER_ARGS_H
/* join_worker_args.h -- xce-test-cli-ux task 2.6 remediation (bug-1557 fix)
*
* Extracted from test.c's cmd_join_worker(): the pure argument-parsing/
* defaulting block that populates a hostattrs_t (xce/types.h) from argc/argv,
* including the bug-1557 maxmem default fix. This block runs entirely before
* cmd_join_worker's only network call (xcecli_join()) and touches no
* active_ctx, REPL state, or any global beyond the standard `optind` reset
* every other cmd_ handler in this file already performs -- the same
* isolation rationale as joblist_display.c's extraction (task 2.5) and
* resval_typeresolve.c's (resource-value-verification task 1.1).
*
* test.c's cmd_join_worker() calls this directly in place of its old inline
* parsing block, then proceeds to xcecli_join() exactly as before -- no
* behavior change to the network-call phase, extraction only.
*/
#include "xce/types.h" /* hostattrs_t */
/* Parse join's CLI flags ("-f cpufactor -c maxcpus -m maxmem -s maxswap
* -t maxtmp -d ndisks") into `*attrs`, first defaulting every field
* (including the bug-1557-corrected maxmem default of 64 * 1024 * 1024
* bytes -- see the in-body comment in join_worker_args.c for the full
* bytes-vs-MB history) and then overriding any field whose flag was given.
*
* Resets `optind` to 1 before parsing, matching this file's own convention
* for repeated in-process command invocations (see cmd_jobfinish's comment
* on the same convention).
*
* Returns 0 on success. Returns -EINVAL on an unrecognized flag or a NULL
* `attrs`, leaving `*attrs` fully defaulted (and possibly partially
* overridden by whichever flags were already parsed before the
* unrecognized one) -- callers must not use `*attrs` after a nonzero
* return. On a parse error, this function itself prints the same usage
* string cmd_join_worker printed before extraction. */
int join_worker_parse_attrs(int argc, char* argv[], hostattrs_t* attrs);
#endif /* JOIN_WORKER_ARGS_H */

49
src/controller/testsuite/test.c
View file

@ -35,6 +35,8 @@
#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 "join_worker_args.h" /* join_worker_parse_attrs (xce-test-cli-ux
* task 2.6 remediation, bug-1557) */
#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 */
@ -3363,46 +3365,19 @@ int cmd_join_worker(int argc, char* argv[])
{
int rc;
int role;
int opt;
hostattrs_t attrs = { .cpufactor = 1.0,
.maxcpus = 4,
.maxmem = 64,
.maxswap = 1024,
.maxtmp = 2048,
.ndisks = 1 };
hostattrs_t attrs;
role = HostRole_worker;
optind = 1;
attrs.hostmodel = "Intel64";
attrs.hosttype = "LINUX64";
while ((opt = getopt(argc, argv, "f:c:m:s:t:d:")) != -1) {
switch (opt) {
case 'f':
attrs.cpufactor = atof(optarg);
break;
case 'c':
attrs.maxcpus = atoi(optarg);
break;
case 'm':
attrs.maxmem = atoi(optarg);
break;
case 's':
attrs.maxswap = atoi(optarg);
break;
case 't':
attrs.maxtmp = atoi(optarg);
break;
case 'd':
attrs.ndisks = atoi(optarg);
break;
default:
printf(" join [-f cpufactor] [-c maxcpus] [-m maxmem] [-s maxswap] "
"[-t maxtmp] [-d ndisks]\n");
return -EINVAL;
}
/* xce-test-cli-ux task 2.6 remediation (bug-1557): the argument-parsing/
* defaulting block (including the maxmem default fix) is extracted into
* join_worker_args.c/.h so it can be unit-tested in isolation from this
* function's REPL/network state -- see join_worker_args.h for the full
* rationale. This is a structural extraction only; the parsing/defaulting
* behavior and everything from xcecli_join() onward are unchanged. */
rc = join_worker_parse_attrs(argc, argv, &attrs);
if (rc < 0) {
return rc;
}
rc = xcecli_join(active_ctx->ctx, role, &attrs);

129
src/controller/testsuite/test_join_worker_args.c Normal file
View file

@ -0,0 +1,129 @@
/* test_join_worker_args.c -- xce-test-cli-ux task 2.6 remediation
* (bug-1557 fix)
*
* Unity unit tests for join_worker_args.c's join_worker_parse_attrs(),
* extracted from test.c's cmd_join_worker() (see join_worker_args.h for the
* full extraction rationale). This is the permanent, automated regression
* guard for bug-1557 (join's stale maxmem default): review had confirmed
* the fix value/unit were already correct in cmd_join_worker, but rejected
* that task for lacking a permanent test given a genuine testable seam
* existed -- this file is that test.
*
* Standalone Unity main (this binary is its own test executable -- see
* meson.build's join_worker_args_unit_test), mirroring
* joblist_display_unit_test's/resval_typeresolve_unit_test's own pattern.
*/
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <unity/unity.h>
#include "join_worker_args.h"
#include "xce/types.h" /* hostattrs_t */
/* Requirement 12.1's actual regression guard: no -m flag given must default
* maxmem to 64 * 1024 * 1024 (64 MB expressed in the codebase's
* bytes-canonical unit) -- not the pre-bug-1557 nonsensical 64 bytes. */
static void test_join_worker_default_maxmem_is_64mb_in_bytes(void)
{
char* argv[] = { "join" };
hostattrs_t attrs;
int rc = join_worker_parse_attrs(1, argv, &attrs);
TEST_ASSERT_EQUAL_INT(0, rc);
/* This Unity build has double-precision assertions disabled -- use
* TEST_ASSERT_EQUAL_FLOAT (this file's own convention, matching
* test_listfmt_json.c/test_wal_job_rusage_fact.c) instead. 64 * 1024 *
* 1024 is an exact power of two, so the float round-trip is exact. */
TEST_ASSERT_EQUAL_FLOAT_MESSAGE(
(float)(64 * 1024 * 1024), (float)attrs.maxmem,
"join's default maxmem must be 64 * 1024 * 1024 bytes (bug-1557, "
"Requirement 12.1)");
}
/* Proves the flag-parsing itself still works after extraction, not just the
* default: -m <value> must override the default exactly. */
static void test_join_worker_dash_m_overrides_default(void)
{
char* argv[] = { "join", "-m", "12345" };
hostattrs_t attrs;
int rc = join_worker_parse_attrs(3, argv, &attrs);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_FLOAT_MESSAGE(
12345.0f, (float)attrs.maxmem,
"-m <value> must override join's default maxmem");
}
/* Fail-loud behavior (matching this file's error-handling convention
* elsewhere): an unrecognized flag must return the negative-errno error
* code, not silently succeed with a partially-parsed attrs. */
static void test_join_worker_unrecognized_flag_returns_einval(void)
{
char* argv[] = { "join", "-z", "bogus" };
hostattrs_t attrs;
int rc = join_worker_parse_attrs(3, argv, &attrs);
TEST_ASSERT_EQUAL_INT(-EINVAL, rc);
}
/* A NULL attrs pointer must also fail loud rather than crash. */
static void test_join_worker_null_attrs_returns_einval(void)
{
char* argv[] = { "join" };
int rc = join_worker_parse_attrs(1, argv, NULL);
TEST_ASSERT_EQUAL_INT(-EINVAL, rc);
}
/* Documents/locks in the CURRENT (still-buggy, unchanged-by-this-task)
* default values of maxcpus/maxswap/maxtmp -- the sibling same-class fields
* task 2.6's implementer correctly declined to touch (a separate,
* recommended follow-up bug covers them). If someone later fixes those
* fields, THIS test's assertions (specific to maxmem) must not be confused
* with assertions on those separate fields. */
static void test_join_worker_sibling_defaults_unchanged(void)
{
char* argv[] = { "join" };
hostattrs_t attrs;
int rc = join_worker_parse_attrs(1, argv, &attrs);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_INT_MESSAGE(
4, attrs.maxcpus,
"maxcpus default is unchanged by task 2.6 (bug-1557 scoped to "
"maxmem only)");
TEST_ASSERT_EQUAL_FLOAT_MESSAGE(
1024.0f, (float)attrs.maxswap,
"maxswap's still-buggy default is intentionally unchanged by task "
"2.6 -- separate follow-up bug covers it");
TEST_ASSERT_EQUAL_FLOAT_MESSAGE(
2048.0f, (float)attrs.maxtmp,
"maxtmp's still-buggy default is intentionally unchanged by task "
"2.6 -- separate follow-up bug covers it");
}
/* ------------------------------------------------------------------ */
/* Standalone Unity main (this binary is its own test executable -- see */
/* meson.build's join_worker_args_unit_test) */
/* ------------------------------------------------------------------ */
void setUp(void) { return; }
void tearDown(void) { return; }
int main(void)
{
printf("\n>> xce-test-cli-ux task 2.6 remediation: join_worker_args "
"join_worker_parse_attrs (bug-1557)\n\n");
UNITY_BEGIN();
RUN_TEST(test_join_worker_default_maxmem_is_64mb_in_bytes);
RUN_TEST(test_join_worker_dash_m_overrides_default);
RUN_TEST(test_join_worker_unrecognized_flag_returns_einval);
RUN_TEST(test_join_worker_null_attrs_returns_einval);
RUN_TEST(test_join_worker_sibling_defaults_unchanged);
return UNITY_END();
}