From 51255366577eecce7153d019d689e620ca02782f Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sun, 20 Sep 2026 07:35:11 +0000 Subject: [PATCH] test(scheduler-sim-diagnostics): strengthen coincidentally-passing assertions (task 10.1) Six pre-existing assertions in test_repl_limit_table.c (across 4 test functions) used bare strstr() substring checks against whole-number fixture values on "mem" -- e.g. strstr(captured, "accounted=4.00"). Task 9's fix changed the actual output to "accounted=4.000000 (4.00 B)", but the old substring is a literal prefix of the new one, so these assertions kept passing by coincidence of formatting, providing zero actual regression protection for the fix. Strengthened each to assert the full corrected " ()" string. Proved via TDD: reverting task 9's fix now makes all 6 strengthened assertions (plus task 9's own 2 genuine tests) fail; restoring the fix returns the suite to 85/85 green. Confirmed via full-file scan that no other loose assertion of this shape remains, and that info worker/info resource's own regression suites were never vulnerable to this class of bug (they build expected strings dynamically rather than hardcoding literal substrings). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC --- .../specs/scheduler-sim-diagnostics/tasks.md | 2 +- .../testsuite/test_repl_limit_table.c | 45 +++++++++++++------ 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/.kiro/specs/scheduler-sim-diagnostics/tasks.md b/.kiro/specs/scheduler-sim-diagnostics/tasks.md index 5d773657..fa175de4 100644 --- a/.kiro/specs/scheduler-sim-diagnostics/tasks.md +++ b/.kiro/specs/scheduler-sim-diagnostics/tasks.md @@ -100,7 +100,7 @@ - _Boundary: REPL Limit Info Command_ - [ ] 10. Validation: Confirm correctness, consistency, and no regression -- [ ] 10.1 Re-verify existing test coverage against the corrected usage display +- [x] 10.1 Re-verify existing test coverage against the corrected usage display - Task 9 found that fixing the resource-classification defect did NOT break any pre-existing assertion outright — instead, several existing assertions on the live usage display's amounts turned out to be loose substring checks against whole-number fixture values, which happen to still match the corrected, human-augmented output by coincidence of formatting (the old expected text is a literal prefix of the new one) rather than by genuinely exercising the fix - Identify every such loose, coincidentally-still-passing assertion, and strengthen each one to assert the actual corrected output precisely enough that it would fail if the resource-classification fix were reverted - Observable: each strengthened assertion demonstrably fails when checked against the prior (defective) behavior and passes against the corrected behavior — not merely a new assertion added alongside an untouched, still-coincidentally-passing old one diff --git a/src/scheduler/testsuite/test_repl_limit_table.c b/src/scheduler/testsuite/test_repl_limit_table.c index 2376311b..ee244244 100644 --- a/src/scheduler/testsuite/test_repl_limit_table.c +++ b/src/scheduler/testsuite/test_repl_limit_table.c @@ -1352,12 +1352,18 @@ test_info_limit_named_shows_accounted_and_capacity_for_matched_and_charged_globa captured = capture_info_limit(&sim, "charged_limit"); TEST_ASSERT_NOT_NULL_MESSAGE( - strstr(captured, "accounted=4.00"), - "Requirement 2.1: a global limit matched and charged must show its " - "accounted usage"); + strstr(captured, "accounted=4.000000 (4.00 B)"), + "Requirement 2.1/6.1: a global limit matched and charged must show " + "its accounted usage, classified by the limit's OWN constrained " + "resource ('mem') and rendered human-readable -- a bare " + "\"accounted=4.00\" substring check would also match the pre-fix " + "vres->vres->name-misclassified plain \"accounted=4.00\" output, " + "so this asserts the full corrected \" ()\" string " + "instead"); TEST_ASSERT_NOT_NULL_MESSAGE( - strstr(captured, "capacity=10.00"), - "Requirement 2.1: ... alongside its configured capacity"); + strstr(captured, "capacity=10.000000 (10.00 B)"), + "Requirement 2.1/6.1: ... alongside its configured capacity, same " + "full-string rationale as the accounted check above"); TEST_ASSERT_NULL_MESSAGE( strstr(captured, "no active usage instance yet"), "a genuinely matched+charged instance must not report 'no active " @@ -1452,10 +1458,13 @@ test_info_limit_named_shows_accounted_zero_for_matched_but_never_charged_instanc captured = capture_info_limit(&sim, "uncharged_limit"); TEST_ASSERT_NOT_NULL_MESSAGE( - strstr(captured, "accounted=0.00"), + strstr(captured, "accounted=0.000000 (0.00 B)"), "the existence-vs-zero-usage distinction: a matched vres with a " "real capacity entry but an absent in-use entry must display a " - "genuine accounted=0, not the no-instance message"); + "genuine accounted=0, not the no-instance message -- full " + "human-readable string (Requirement 6.1), not a bare \"accounted=" + "0.00\" substring that would equally match the pre-fix " + "misclassified output"); TEST_ASSERT_NULL_MESSAGE( strstr(captured, "no active usage instance yet"), "must NOT report 'no active usage instance yet' -- the vres and " @@ -1534,12 +1543,17 @@ test_info_limit_named_shows_separate_per_host_usage_lines_for_touched_hosts_and_ "Observable: a third configured-but-untouched host must show no " "line at all, not a fabricated zero"); TEST_ASSERT_NOT_NULL_MESSAGE( - strstr(captured, "accounted=2.00"), - "Requirement 2.1: hostA's own accounted usage must be shown"); + strstr(captured, "accounted=2.000000 (2.00 B)"), + "Requirement 2.1/6.1/6.4: hostA's own accounted usage must be " + "shown, classified by the limit's OWN constrained resource ('mem') " + "and rendered human-readable -- a bare \"accounted=2.00\" " + "substring check would also match the pre-fix vres->vres->name-" + "misclassified plain output"); TEST_ASSERT_NOT_NULL_MESSAGE( - strstr(captured, "accounted=3.00"), - "Requirement 2.1: hostB's own accounted usage must be shown, " - "distinct from hostA's"); + strstr(captured, "accounted=3.000000 (3.00 B)"), + "Requirement 2.1/6.1/6.4: hostB's own accounted usage must be " + "shown, distinct from hostA's, same full-string rationale as the " + "hostA check above"); free(captured); @@ -1596,10 +1610,13 @@ test_info_limit_named_shows_accounted_zero_for_host_with_capacity_but_never_char "hostA has a genuinely populated capacity entry and must appear as " "a line, even though nothing has been charged against it yet"); TEST_ASSERT_NOT_NULL_MESSAGE( - strstr(captured, "accounted=0.00"), + strstr(captured, "accounted=0.000000 (0.00 B)"), "the existence-vs-zero-usage distinction applies per host too: a " "populated capacity entry with an absent in-use entry must display " - "a genuine accounted=0, not omit the host"); + "a genuine accounted=0, not omit the host -- full human-readable " + "string (Requirement 6.1/6.4), not a bare \"accounted=0.00\" " + "substring that would equally match the pre-fix misclassified " + "output"); free(captured);