From 9d91d470b46b01881de886e20e679ded3fee9e6d Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Mon, 3 Aug 2026 09:11:55 +0000 Subject: [PATCH] test(scheduler-core): guard core scheduling loop against ordering-logic regressions (task 2.5) Task 2.4 already made sched_job_picker() delegate entirely to the picker interface with zero direct queue/job traversal, satisfying Req 1 AC 1/AC 7 as a byproduct. This adds a source-text regression test locking that property in place: asserts sched_job_picker()'s body contains none of the forbidden direct-traversal symbols and does contain the expected picker-interface call sites. --- .kiro/specs/scheduler-core/tasks.md | 2 +- src/scheduler/testsuite/meson.build | 24 ++ .../test_picker_core_loop_boundary.c | 240 ++++++++++++++++++ 3 files changed, 265 insertions(+), 1 deletion(-) create mode 100644 src/scheduler/testsuite/test_picker_core_loop_boundary.c diff --git a/.kiro/specs/scheduler-core/tasks.md b/.kiro/specs/scheduler-core/tasks.md index 8a9c2694..c352929f 100644 --- a/.kiro/specs/scheduler-core/tasks.md +++ b/.kiro/specs/scheduler-core/tasks.md @@ -60,7 +60,7 @@ - _Requirements: 1.3, 1.4, 1.6, 2.5, 3.1, 3.5_ - _Depends: 2.3, 1.3_ -- [ ] 2.5 Remove ordering logic from the core scheduling loop +- [x] 2.5 Remove ordering logic from the core scheduling loop - The core loop calls only `picker->next_job()` to obtain the next schedulable job — no project/queue/job iteration or ordering logic remains in the loop itself - Observable: a code-level check confirms the scheduling loop function contains no direct queue/job traversal — all selection reads happen through the picker interface - _Requirements: 1.1, 1.7_ diff --git a/src/scheduler/testsuite/meson.build b/src/scheduler/testsuite/meson.build index d5b6cfdf..d73c019e 100644 --- a/src/scheduler/testsuite/meson.build +++ b/src/scheduler/testsuite/meson.build @@ -134,6 +134,27 @@ sched_picker_wiring_test = executable('sched_picker_wiring_test', dependencies: [scheduler_test_deps], install: false) +# Regression guard for the core scheduling loop boundary (scheduler-core +# spec, task 2.5, Req 1 AC 1 / AC 7): "the core loop calls only +# picker->next_job() ... no project/queue/job iteration or ordering logic +# remains in the loop itself." Does NOT #include scheduler.c (that file's +# external dependency surface is why test_picker_wiring.c's own header +# comment already ruled out driving sched_job_picker() directly) -- instead +# it reads scheduler.c's real on-disk text at test run time and mechanically +# checks sched_job_picker()'s body for required picker-interface call sites +# and absent direct-traversal symbols. XCE_SCHEDULER_C_PATH bakes in the +# absolute path via meson.current_source_dir() so the check has no +# dependency on the test's cwd at run time. +sched_picker_core_loop_boundary_test = executable( + 'sched_picker_core_loop_boundary_test', + ['test_picker_core_loop_boundary.c'], + include_directories:[incdir], + dependencies: [scheduler_test_deps], + c_args: ['-DXCE_SCHEDULER_C_PATH="' + + (meson.current_source_dir() / '..' / 'src' / 'scheduler.c') + + '"'], + install: false) + # Register as meson tests so `meson test --suite scheduler` runs them all. # detect_leaks=0 matches the suite convention (fixtures are not torn down). test('scheduler test', scheduler_test, suite: 'scheduler', @@ -158,3 +179,6 @@ test('scheduler picker fcfs session test', sched_picker_fcfs_session_test, suite env: ['ASAN_OPTIONS=detect_leaks=0']) test('scheduler picker wiring test', sched_picker_wiring_test, suite: 'scheduler', env: ['ASAN_OPTIONS=detect_leaks=0']) +test('scheduler picker core loop boundary test', + sched_picker_core_loop_boundary_test, suite: 'scheduler', + env: ['ASAN_OPTIONS=detect_leaks=0']) diff --git a/src/scheduler/testsuite/test_picker_core_loop_boundary.c b/src/scheduler/testsuite/test_picker_core_loop_boundary.c new file mode 100644 index 00000000..92fe8a23 --- /dev/null +++ b/src/scheduler/testsuite/test_picker_core_loop_boundary.c @@ -0,0 +1,240 @@ +/* + * test_picker_core_loop_boundary.c - regression guard for the core + * scheduling loop boundary (scheduler-core spec, task 2.5, Req 1 AC 1 / + * AC 7): "The Scheduler core loop shall call picker->next_job(picker) ... + * and shall not contain any logic about project iteration, queue + * iteration, or job ordering -- those concerns are entirely encapsulated + * inside the picker plugin." + * + * Task 2.5's own verification pass (independently re-reading scheduler.c + * after task 2.4's rewrite) found sched_job_picker() already satisfies + * this property as a byproduct of task 2.4's work: it resolves the + * configured picker via sched_resolve_job_picker(), then either delegates + * wholesale to a legacy job_elect_fn plugin (self-contained, unchanged) or + * runs a generic loop that reads job order exclusively through + * picker_iface->begin_session()/next_job()/end_session() -- zero direct + * sched->queues/sched->jobs traversal. This task's Observable is therefore + * the *regression guard* on an already-true property, not a behavior + * change: a code-level check that keeps this true if a future change + * reintroduces ordering logic into the core loop. + * + * Why a source-text scan and not a linked white-box test: test_picker_wiring.c + * (task 2.4)'s own header comment already established that sched_job_picker() + * cannot be tractably driven directly -- it is `static` in scheduler.c, whose + * external dependency surface (xcecli_ctx_*, sel_register_alloc_thread, + * resource.c's allocation path, decision.c's decision constructors, ...) is + * far larger than any other white-box test in this directory stubs, and + * spinning the real picker/allocator threads to exercise it live is exactly + * what that file's task chose not to do. This file instead reads the + * actual, current on-disk source of sched_job_picker() at test run time (an + * absolute path baked in by meson.build via XCE_SCHEDULER_C_PATH, so no cwd + * assumption is needed -- see meson.build's comment on this test target) + * and mechanically checks the function body: it must contain the + * picker-interface call sites the Observable requires, and must NOT + * contain any of the direct-traversal symbols that would indicate + * project/queue/job ordering logic crept back into the loop. This is a + * grep-based assertion, same genre as test_upgrade_snapshot_apply.c's + * popen+grep structural check (controller testsuite) -- here done via + * fopen+strstr instead of popen since no shell/glob is needed. + * + * RED demonstration performed manually during development (not committed): + * temporarily inserting a `for (job = sched->jobs.head; ...)`-style + * fragment into sched_job_picker()'s real body made + * test_core_loop_has_no_forbidden_traversal_symbols fail immediately with + * the exact forbidden-symbol message, confirming the check exercises live + * source text rather than a frozen snapshot -- then reverted. + */ +#include +#include +#include + +#include + +#ifndef XCE_SCHEDULER_C_PATH +#error \ + "XCE_SCHEDULER_C_PATH must be defined by meson.build to the absolute path of scheduler.c" +#endif + +/* Anchor is the literal function signature text, unique to the real + * definition -- every comment reference to sched_job_picker elsewhere in + * this codebase omits the "(void* arg)" argument (verified via `grep -rn + * sched_job_picker src/scheduler`: only the definition itself carries this + * exact substring). If this signature is ever reformatted, this test fails + * loudly (extraction returns NULL) rather than silently passing on stale + * text. */ +static const char* const FUNC_ANCHOR + = "static void* sched_job_picker(void* arg)"; + +/* Symbols whose presence in sched_job_picker()'s body would mean project/ + * queue/job iteration or ordering logic leaked back into the core loop + * (Req 1 AC 1). None of these are legitimately needed by a function whose + * only jobs are: resolve the picker, delegate to job_elect_fn OR drive + * picker_iface's session hooks, and shuttle jobids across cbuf. */ +static const char* const FORBIDDEN_SYMBOLS[] = { + "sched->queues", "sched->jobs", "iter_make(", "iter_next(", + "ordq_get(", "ordq_put(", "ordq_peek(", "key_find(", +}; +#define NUM_FORBIDDEN (sizeof(FORBIDDEN_SYMBOLS) / sizeof(FORBIDDEN_SYMBOLS[0])) + +/* Picker-interface call sites the Observable requires: "all selection + * reads happen through the picker interface." Both the legacy-delegation + * branch and the native session-loop branch must be present -- task 2.5 + * did not collapse them (that would be a behavior change, out of scope; + * see this task's own "likely non-behavioral" framing). */ +static const char* const REQUIRED_SYMBOLS[] = { + "job_elect_fn(job_picker)", + "picker_iface->begin_session(job_picker)", + "picker_iface->next_job(job_picker)", + "picker_iface->end_session(job_picker)", +}; +#define NUM_REQUIRED (sizeof(REQUIRED_SYMBOLS) / sizeof(REQUIRED_SYMBOLS[0])) + +static char* read_whole_file(const char* path) +{ + FILE* f = fopen(path, "rb"); + if (f == NULL) { + return NULL; + } + + if (fseek(f, 0, SEEK_END) != 0) { + fclose(f); + return NULL; + } + + long len = ftell(f); + if (len < 0) { + fclose(f); + return NULL; + } + rewind(f); + + char* buf = (char*)malloc((size_t)len + 1); + if (buf == NULL) { + fclose(f); + return NULL; + } + + size_t nread = fread(buf, 1, (size_t)len, f); + fclose(f); + buf[nread] = '\0'; + + return buf; +} + +/* + * extract_function_body - returns a newly malloc'd, NUL-terminated copy of + * the {...} body of the function whose signature text is `anchor` within + * `text` (brace-depth counting from the first '{' following the anchor). + * Returns NULL if the anchor isn't found or braces never balance before + * EOF -- the caller must treat NULL as a hard test failure, not "skip". + */ +static char* extract_function_body(const char* text, const char* anchor) +{ + const char* sig = strstr(text, anchor); + if (sig == NULL) { + return NULL; + } + + const char* brace = strchr(sig, '{'); + if (brace == NULL) { + return NULL; + } + + int depth = 0; + const char* p = brace; + do { + if (*p == '{') { + depth++; + } else if (*p == '}') { + depth--; + } + p++; + } while ((depth > 0) && (*p != '\0')); + + if (depth != 0) { + return NULL; /* unbalanced -- EOF reached mid-function */ + } + + size_t len = (size_t)(p - brace); + char* body = (char*)malloc(len + 1); + if (body == NULL) { + return NULL; + } + memcpy(body, brace, len); + body[len] = '\0'; + + return body; +} + +/* Loads scheduler.c and extracts sched_job_picker()'s body fresh for each + * test (no shared/torn-down fixture state -- matches this directory's + * "fixtures aren't torn down" convention, but here each test cleans up its + * own two allocations at the end anyway since there's no ASAN leak check + * reason not to). */ +static char* load_core_loop_body(void) +{ + char* file_text = read_whole_file(XCE_SCHEDULER_C_PATH); + TEST_ASSERT_NOT_NULL_MESSAGE( + file_text, "could not read scheduler.c at XCE_SCHEDULER_C_PATH"); + + char* body = extract_function_body(file_text, FUNC_ANCHOR); + free(file_text); + + TEST_ASSERT_NOT_NULL_MESSAGE( + body, "could not locate/extract sched_job_picker()'s body -- has its " + "signature changed? update FUNC_ANCHOR in this test if so"); + + return body; +} + +/* Req 1 AC 1: "shall not contain any logic about project iteration, queue + * iteration, or job ordering". */ +static void test_core_loop_has_no_forbidden_traversal_symbols(void) +{ + char* body = load_core_loop_body(); + + for (size_t i = 0; i < NUM_FORBIDDEN; i++) { + char msg[256]; + snprintf(msg, sizeof(msg), + "sched_job_picker() body contains forbidden direct-" + "traversal symbol '%s' -- ordering logic has leaked back " + "into the core scheduling loop (Req 1 AC 1)", + FORBIDDEN_SYMBOLS[i]); + TEST_ASSERT_NULL_MESSAGE(strstr(body, FORBIDDEN_SYMBOLS[i]), msg); + } + + free(body); +} + +/* Req 1 AC 1 / Req 1 AC 4: "the core scheduling loop shall call + * picker->next_job(picker) ... all selection reads happen through the + * picker interface" -- the positive-side check, so a change that silently + * dropped a picker-interface call site (e.g. hardcoding a job again, or + * skipping begin_session/end_session) fails loud even though it wouldn't + * trip the forbidden-symbol list above. */ +static void test_core_loop_calls_only_through_picker_interface(void) +{ + char* body = load_core_loop_body(); + + for (size_t i = 0; i < NUM_REQUIRED; i++) { + char msg[256]; + snprintf(msg, sizeof(msg), + "sched_job_picker() body is missing required picker-" + "interface call site '%s'", + REQUIRED_SYMBOLS[i]); + TEST_ASSERT_NOT_NULL_MESSAGE(strstr(body, REQUIRED_SYMBOLS[i]), msg); + } + + free(body); +} + +void setUp(void) { return; } +void tearDown(void) { return; } + +int main(void) +{ + UNITY_BEGIN(); + RUN_TEST(test_core_loop_has_no_forbidden_traversal_symbols); + RUN_TEST(test_core_loop_calls_only_through_picker_interface); + return UNITY_END(); +}