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

feat(scheduler-sim-discrete-clock): retire --sim-speed as a warn-once no-op, end-to-end CLI tests (task 2.3)

Browse source 
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-29 05:06:06 +00:00
parent 05320e777b
commit eb5f339541
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
3 changed files with 311 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

4
.kiro/specs/scheduler-sim-discrete-clock/tasks.md
View file

@ -11,7 +11,7 @@ Test-first throughout (red → green → refactor). Every sub-task leaves the bu
- _Requirements: 2.3, 2.4, 2.5_
- _Boundary: CliCompat, scheduler test build_
- [ ] 2. Core: pin the simulator clock, remove the REPL speed parameter, retire the CLI option
- [x] 2. Core: pin the simulator clock, remove the REPL speed parameter, retire the CLI option
- [x] 2.1 (P) Pin the simulation-mode clock so it moves only at recorded timestamps
- Write the tests first in the new test binary, each passing a non-zero speed through the still-present simulator argument so they fail on the current code: after start the clock equals the chosen snapshot's timestamp and stays equal after a real sleep, with the core holding the frozen speed setting; with a stand-in allocator that sleeps and a recorded session start that is not a whole second, the session start seen inside the session equals the recorded one exactly, and simulation output has that session start, a zero session duration and every decision timestamp equal to it, with at least one decision per session; a session start earlier than the preceding event, and events earlier than the start snapshot, set the clock to exactly those values and the replay completes; a mid-replay snapshot followed by the end marker leaves the clock at the last event's timestamp after the run and after a sleep, with the start timestamp pinned so the real skip branch runs; the same recording replayed twice gives identical logical timestamps and decisions in both outputs, ignoring only the wall-clock header field (design.md, Testing Strategy: Unit (SimClock), Integration (replay loop)).
- Make simulator startup always select the frozen clock setting and drop the speed parameter from the start-clock function. The start-clock function keeps writing the shadow copy, now always the frozen setting, because `info config` and the startup test still read it; the simulator's speed argument is no longer read. Both are removed in 3.1.
@ -27,7 +27,7 @@ Test-first throughout (red → green → refactor). Every sub-task leaves the bu
- Done when the REPL tests pass and no REPL output or help text offers a speed setting.
- _Requirements: 1.5, 3.1, 3.2, 3.3, 3.4, 3.5_
- _Boundary: ReplSet, ReplHelp, ReplInfo_
- [ ] 2.3 Retire `--sim-speed` on the command line and prove the default command line end to end
- [x] 2.3 Retire `--sim-speed` on the command line and prove the default command line end to end
- Write the tests first in the new test binary, running the built scheduler binary as a child process on a fixture the test writes (session start not a whole second, a pending job before each session), draining its stdout before collecting the exit status: without the option it exits 0, its output has exact session starts, zero durations and decision timestamps equal to the session start, and its log has no warning; with each of `7`, `abc`, `-1` and an empty value (fresh output and log files per run) it exits 0, its output matches the default run except the wall-clock header field, and its log has exactly one warning naming the value; a repeated option logs one warning naming the last value; a trailing option with no value exits non-zero; an interactive run at end of input logs exactly one warning; `--help` describes the option as deprecated and mentions no real-time or as-fast-as-possible mode.
- Stop defaulting, validating and passing a simulation speed; call the warning helper once right after the logger starts and, when the option was given, drain and restart the logger there while still single-threaded; update the option's help text and the comment that cites its numeric validation; the flag-meaning comments that describe the simulator speed argument are left to 3.1 (design.md, Modified Files: `main.c`; Error Handling).
- Done when the end-to-end and help tests pass; the runs with `abc`, `-1`, an empty value and `7`, the repeated-option run, the interactive run and the help test fail against the pre-change command-line handling, while the run without the option is a regression guard that 2.1 already makes pass.

42
src/scheduler/src/main.c
View file

@ -6,6 +6,7 @@
#include <string.h>
#include <unistd.h>
#include "cli_compat.h"
#include "client.h"
#include "controller.capnp.h"
#include "xce/cluster.h"
@ -89,9 +90,12 @@ static struct argp_option options[] = {
{ "sim-start-ts", OPT_SIM_START_TS, "TIMESTAMP", 0,
"Simulation start timestamp (ns since epoch); default: the replay "
"file's most recent snapshot" },
/* scheduler-sim-discrete-clock Req 2.4/2.6: kept, still taking a
* value (a trailing --sim-speed stays an argp error), but never
* parsed; sched_cli_warn_deprecated_options() logs its one warning. */
{ "sim-speed", OPT_SIM_SPEED, "N", 0,
"Simulation speed: 0=as-fast-as-possible, 1=real-time (default), "
"N>1=N x real-time" },
"Deprecated, has no effect: the simulator clock moves only at "
"recorded timestamps. Will be removed in a future release." },
{ "sim-verify", OPT_SIM_VERIFY, 0, 0,
"Compare simulated decisions against the replay file's recorded ones" },
{ "sim-alt-events", OPT_SIM_ALT_EVENTS, "FILE", 0,
@ -293,7 +297,7 @@ int main(int argc, char* argv[])
/* Req 5 AC 11 (scheduler-core spec, task 9.3 follow-up, bug-1585):
* --record-snapshot-interval is the ONLY way record_snapshot_interval
* is ever provisioned -- same "argv is the sole source" contract as
* record_dir above. Numeric validation mirrors --sim-speed's own
* record_dir above. Numeric validation mirrors --sim-start-ts's own
* strtoull() pattern elsewhere in this function (below, in the --sim
* branch) -- rejects a non-numeric or partially-numeric argument with a
* clear error instead of silently truncating via atoi(). */
@ -324,6 +328,22 @@ int main(int argc, char* argv[])
mylog_init(logfile, loglevel, logsize, logcount);
/* scheduler-sim-discrete-clock Req 2.1-2.5: --sim-speed is accepted in
* both modes with any value and has no effect beyond this one startup
* warning. When it was given, drain the logger right away (tlog_exit()
* joins its writer thread only after every buffered line is on disk)
* and restart it, so the warning survives every later exit path: the
* --sim returns below, the REPL's exit(0) and normal-mode shutdown.
* Safe only here, while still single-threaded: no picker, allocator or
* driver thread exists before sched_sim_driver_run()/sched_init(), and
* tlog_exit() must not race another logging thread -- which is also why
* it is not called at the --sim returns or in the REPL's quit. */
sched_cli_warn_deprecated_options(arguments.sim_speed);
if (arguments.sim_speed != NULL) {
mylog_fini();
mylog_init(logfile, loglevel, logsize, logcount);
}
/* Req 5 AC 9 "Mode selection" / Req 5 AC 13 "Simulation mode startup"
* (scheduler-core spec, task 11.1): --sim selects the simulation driver
* (SimDriver) instead of the normal, controller-attached one. Checked
@ -336,7 +356,6 @@ int main(int argc, char* argv[])
sched_sim_args_t sim_args;
xce_sched_sim_driver_t sim;
uint64_t sim_start_ts = SCHED_SIM_START_TS_LATEST;
uint64_t sim_speed = 1; /* default: real-time */
int rc;
if (arguments.sim_start_ts != NULL) {
@ -353,24 +372,9 @@ int main(int argc, char* argv[])
}
}
if (arguments.sim_speed != NULL) {
char* endp = NULL;
sim_speed = strtoull(arguments.sim_speed, &endp, 10);
if ((endp == NULL) || (*endp != '\0')
|| (endp == arguments.sim_speed)) {
fprintf(stderr,
"xce_scheduler: --sim-speed expects a numeric "
"value, got \"%s\"\n",
arguments.sim_speed);
return 1;
}
}
memset(&sim_args, 0, sizeof(sim_args));
sim_args.replay_path = arguments.sim_file;
sim_args.sim_start_ts = sim_start_ts;
sim_args.sim_speed = sim_speed;
sim_args.verify = (arguments.sim_verify != 0);
sim_args.alt_events_path = arguments.sim_alt_events;
sim_args.output_path = arguments.sim_output;

289
src/scheduler/testsuite/test_sim_discrete_clock.c
View file

@ -29,9 +29,17 @@
* early-print path writes WARN lines to stdout, which the test-local
* capture_stdout_around() below captures.
*
* XCE_SCHEDULER_BIN (compile-time, from the scheduler executable's
* full_path() in testsuite/meson.build) is used by the end-to-end tests
* later tasks add to this file.
* Task 2.3 (Requirements 1.4, 1.7, 1.8, 2.1-2.6; design.md, Testing
* Strategy: CLI help and CLI end-to-end (a)-(e)): XCE_SCHEDULER_BIN
* (compile-time, from the scheduler executable's full_path() in
* testsuite/meson.build) runs as a child process on a fixture this file
* writes. Without --sim-speed the default command line produces exact
* session starts, zero durations and decision timestamps equal to the
* session start, with no warning; with any value (valid or not) the output
* is unchanged except recordingStartWall and the log holds exactly one
* warning naming the (last) value; a trailing --sim-speed is still an argp
* error; the REPL exit path keeps the warning; --help calls the option
* deprecated.
*/
#include <errno.h>
@ -43,6 +51,7 @@
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#include <unity/unity.h>
@ -971,6 +980,274 @@ static void test_two_replays_produce_identical_output(void)
}
}
/* ---------------------------------------------------------------------- */
/* CLI end to end and --help (task 2.3; Requirements 1.4, 1.7, 1.8, */
/* 2.1-2.6). The built scheduler binary runs as a child process on the */
/* two-session fixture; its stdout (decision lines) goes to /dev/null and */
/* the exit status comes from WEXITSTATUS(pclose()). Every run gets a */
/* fresh --sim-output and --logfile. */
/* ---------------------------------------------------------------------- */
/* Runs `cmd` through the shell, drains its stdout to EOF and returns its
* exit status (-1 if it did not exit normally). */
static int run_child(const char* cmd)
{
FILE* p = popen(cmd, "r");
char sink[512];
int st;
TEST_ASSERT_NOT_NULL_MESSAGE(p, "popen of the scheduler must succeed");
while (fread(sink, 1, sizeof(sink), p) > 0) {
}
st = pclose(p);
TEST_ASSERT_TRUE_MESSAGE(st != -1, "pclose must succeed");
return WIFEXITED(st) ? WEXITSTATUS(st) : -1;
}
/* Runs the scheduler in batch simulation mode on `fixture` with `extra`
* appended to the command line; writes to fresh output and log files whose
* paths are returned. */
static int run_sim_cli(const char* fixture, const char* extra, char* out,
char* log)
{
char cmd[4 * PATH_MAX];
make_tmp_file(out, PATH_MAX, "cliout");
make_tmp_file(log, PATH_MAX, "clilog");
snprintf(cmd, sizeof(cmd),
XCE_SCHEDULER_BIN " --sim %s --sim-output %s --logfile %s %s "
"> /dev/null",
fixture, out, log, extra);
return run_child(cmd);
}
/* Counts the log lines naming --sim-speed; *named is set to how many of
* them also contain `quoted`. An absent log counts as zero lines. */
static int count_speed_log_lines(const char* log, const char* quoted,
int* named)
{
FILE* f = fopen(log, "r");
char line[4096];
int n = 0;
*named = 0;
if (f == NULL) {
return 0;
}
while (fgets(line, sizeof(line), f) != NULL) {
if (strstr(line, "--sim-speed") != NULL) {
n++;
if ((quoted != NULL) && (strstr(line, quoted) != NULL)) {
(*named)++;
}
}
}
fclose(f);
return n;
}
static void assert_one_speed_warning(const char* log, const char* quoted)
{
int named = 0;
TEST_ASSERT_EQUAL_INT_MESSAGE(1, count_speed_log_lines(log, quoted,
&named),
"the log must hold exactly one "
"--sim-speed warning");
TEST_ASSERT_EQUAL_INT_MESSAGE(1, named, "the warning must name the "
"quoted value");
}
/* Output identical to the reference except ReplayHeader.recordingStartWall:
* header decoded and compared field by field, the rest byte for byte. */
static void assert_same_output(const char* ref, const char* got)
{
replay_header_t hdr[2];
long len[2];
long hdr_end[2];
char* buf[2];
int r;
buf[0] = slurp_output(ref, &len[0], &hdr_end[0], &hdr[0]);
buf[1] = slurp_output(got, &len[1], &hdr_end[1], &hdr[1]);
TEST_ASSERT_EQUAL_UINT64(hdr[0].recording_start_ts,
hdr[1].recording_start_ts);
TEST_ASSERT_EQUAL_STRING(hdr[0].recorder_version, hdr[1].recorder_version);
TEST_ASSERT_EQUAL_STRING(hdr[0].sched_policy, hdr[1].sched_policy);
TEST_ASSERT_EQUAL_PTR(NULL, hdr[0].cluster);
TEST_ASSERT_EQUAL_PTR(NULL, hdr[1].cluster);
TEST_ASSERT_EQUAL_INT64(len[0] - hdr_end[0], len[1] - hdr_end[1]);
TEST_ASSERT_EQUAL_MEMORY_MESSAGE(buf[0] + hdr_end[0], buf[1] + hdr_end[1],
(size_t)(len[0] - hdr_end[0]),
"--sim-speed must not change the "
"output after the header");
for (r = 0; r < 2; r++) {
free_ReplayHeader(&hdr[r]);
free(buf[r]);
}
}
/* (a) no --sim-speed: exit 0, exact session starts, zero durations,
* decision ts == beginRound (at least one decision per session), and no
* warning in the log. Returns the output path in `out` for (b). */
static void run_default_cli(const char* fixture, char* out)
{
const uint64_t begin[2] = { OUT_B1, OUT_B2 };
char log[PATH_MAX];
out_session_t sessions[4];
int named = 0;
int n;
int k;
int i;
TEST_ASSERT_EQUAL_INT_MESSAGE(0, run_sim_cli(fixture, "", out, log),
"the default command line must exit 0");
n = read_output_sessions(out, sessions, 4);
TEST_ASSERT_EQUAL_INT(2, n);
for (k = 0; k < n; k++) {
TEST_ASSERT_EQUAL_UINT64(begin[k], sessions[k].begin_round);
TEST_ASSERT_EQUAL_UINT64(0ULL, sessions[k].duration_ns);
TEST_ASSERT_TRUE_MESSAGE(sessions[k].n_decisions >= 1,
"each session must decide at least once");
for (i = 0; i < sessions[k].n_decisions; i++) {
TEST_ASSERT_EQUAL_UINT64(begin[k], sessions[k].decision_ts[i]);
}
}
TEST_ASSERT_EQUAL_INT_MESSAGE(0, count_speed_log_lines(log, NULL, &named),
"no --sim-speed warning without the "
"option");
unlink(log);
}
/* (a) 1.4, 1.7, 1.8, 2.5 on the default command line. */
static void test_cli_default_exact_times_no_warning(void)
{
char fixture[PATH_MAX];
char out[PATH_MAX];
write_two_session_fixture(fixture, sizeof(fixture));
run_default_cli(fixture, out);
unlink(out);
}
/* (b) 2.1-2.4: any value, valid or not, exits 0 with output identical to
* the default run and exactly one warning naming the value. */
static void test_cli_sim_speed_any_value_is_noop_with_one_warning(void)
{
static const char* const extra[] = { "--sim-speed 7", "--sim-speed abc",
"--sim-speed -1", "--sim-speed=" };
static const char* const quoted[] = { "\"7\"", "\"abc\"", "\"-1\"",
"\"\"" };
char fixture[PATH_MAX];
char ref[PATH_MAX];
char out[PATH_MAX];
char log[PATH_MAX];
size_t i;
write_two_session_fixture(fixture, sizeof(fixture));
run_default_cli(fixture, ref);
for (i = 0; i < sizeof(extra) / sizeof(extra[0]); i++) {
TEST_ASSERT_EQUAL_INT_MESSAGE(0, run_sim_cli(fixture, extra[i], out,
log),
extra[i]);
assert_same_output(ref, out);
assert_one_speed_warning(log, quoted[i]);
unlink(out);
unlink(log);
}
unlink(ref);
}
/* (c) 2.3: a repeated option logs one warning, naming the last value. */
static void test_cli_repeated_sim_speed_names_last_value(void)
{
char fixture[PATH_MAX];
char out[PATH_MAX];
char log[PATH_MAX];
write_two_session_fixture(fixture, sizeof(fixture));
TEST_ASSERT_EQUAL_INT(0, run_sim_cli(fixture,
"--sim-speed 1 --sim-speed 9", out,
log));
assert_one_speed_warning(log, "\"9\"");
unlink(out);
unlink(log);
}
/* (d) 2.4: a trailing --sim-speed with no value is still an argp error. */
static void test_cli_trailing_sim_speed_without_value_fails(void)
{
char fixture[PATH_MAX];
char cmd[2 * PATH_MAX];
write_two_session_fixture(fixture, sizeof(fixture));
snprintf(cmd, sizeof(cmd),
XCE_SCHEDULER_BIN " --sim %s --sim-speed > /dev/null 2>&1",
fixture);
TEST_ASSERT_NOT_EQUAL_INT_MESSAGE(0, run_child(cmd),
"--sim-speed without a value must be "
"a command-line error");
}
/* (e) 2.3 on the REPL exit path: end of input -> quit -> exit(0), and the
* log holds exactly one warning. */
static void test_cli_interactive_sim_speed_logs_one_warning(void)
{
char fixture[PATH_MAX];
char log[PATH_MAX];
char cmd[3 * PATH_MAX];
write_two_session_fixture(fixture, sizeof(fixture));
make_tmp_file(log, sizeof(log), "clilogi");
snprintf(cmd, sizeof(cmd),
XCE_SCHEDULER_BIN " --sim %s -i --sim-speed 3 --logfile %s "
"< /dev/null > /dev/null",
fixture, log);
TEST_ASSERT_EQUAL_INT(0, run_child(cmd));
assert_one_speed_warning(log, "\"3\"");
unlink(log);
}
/* 2.6: --help describes --sim-speed as deprecated and offers no speed or
* real-time mode. */
static void test_cli_help_describes_sim_speed_as_deprecated(void)
{
FILE* p = popen(XCE_SCHEDULER_BIN " --help", "r");
char* text;
size_t cap = 65536;
size_t len = 0;
size_t got;
char* entry;
char* eol;
TEST_ASSERT_NOT_NULL(p);
text = (char*)calloc(1, cap);
TEST_ASSERT_NOT_NULL(text);
while ((got = fread(text + len, 1, cap - 1 - len, p)) > 0) {
len += got;
}
TEST_ASSERT_EQUAL_INT(0, WEXITSTATUS(pclose(p)));
entry = strstr(text, "--sim-speed");
TEST_ASSERT_NOT_NULL_MESSAGE(entry, "--help must list --sim-speed");
eol = strchr(entry, '\n');
if (eol != NULL) {
*eol = '\0';
}
TEST_ASSERT_NOT_NULL_MESSAGE(strstr(entry, "Deprecated"),
"the --sim-speed entry must say "
"Deprecated");
if (eol != NULL) {
*eol = '\n';
}
TEST_ASSERT_NULL_MESSAGE(strstr(text, "real-time"),
"--help must not mention real-time");
TEST_ASSERT_NULL_MESSAGE(strstr(text, "as-fast-as-possible"),
"--help must not mention as-fast-as-possible");
free(text);
}
int main(void)
{
UNITY_BEGIN();
@ -985,5 +1262,11 @@ int main(void)
RUN_TEST(test_mid_replay_snapshot_and_end_leave_clock_at_last_event);
RUN_TEST(test_session_start_and_output_times_equal_begin_round);
RUN_TEST(test_two_replays_produce_identical_output);
RUN_TEST(test_cli_default_exact_times_no_warning);
RUN_TEST(test_cli_sim_speed_any_value_is_noop_with_one_warning);
RUN_TEST(test_cli_repeated_sim_speed_names_last_value);
RUN_TEST(test_cli_trailing_sim_speed_without_value_fails);
RUN_TEST(test_cli_interactive_sim_speed_logs_one_warning);
RUN_TEST(test_cli_help_describes_sim_speed_as_deprecated);
return UNITY_END();
}