Repository navigation
Consider asserting state-file mtime is unchanged (not just content) to catch no-op rewrites of .windows-autoscale-state. #122
Description
What
Tighten the autoscale state-file test so it asserts the state file is not rewritten at all, not just that its content is unchanged.
The review notes that the current test captures the exact state line in $idleSince and asserts -contains $idleSince after a refused shrink and after a GitHub outage. This catches a regression that resets the clock, but it does not catch a regression that rewrites .windows-autoscale-state with the same value (a no-op rewrite). A no-op rewrite would still pass the content assertion.
Change the test in windows_pools_test.ps1 (the test that exercises Invoke-AutoscalePass and reads $asState) to also capture the state file's LastWriteTime before the pass and assert it is unchanged after the pass, in both the refused-shrink and GitHub-outage scenarios.
Why
- Correctness risk: A no-op rewrite of the state file is a real behavioral regression — it touches the file on every pass even when nothing changed, which can mask bugs (e.g. an unconditional
Set-Contentthat should be guarded) and defeats the purpose of the "state unchanged" assertion. - Test robustness: The current content-only assertion is weaker than it appears. Asserting mtime catches a class of regressions the content check cannot.
- Non-blocking follow-up: Raised during AI review of PR Pin the exact idle clock in Windows autoscale refusal tests #120 as a minor gap, explicitly out of scope for that PR.
How
- In the relevant test in
windows_pools_test.ps1, before invoking the autoscale pass, capture the state file's last-write time:$before = (Get-Item $asState).LastWriteTimeUtc
- After the pass (in both the refused-shrink and GitHub-outage scenarios), assert:
(Get-Item $asState).LastWriteTimeUtc | Should -Be $before
- Keep the existing
-contains $idleSincecontent assertion — the mtime check is additive, not a replacement. - Add a brief comment noting the mtime assertion is intentional and catches no-op rewrites, so future refactors don't drop it.
- If filesystem timestamp resolution is a concern on the CI platform, consider a short sleep before the pass, or compare against a captured timestamp with a tolerance — but prefer exact equality if the platform supports it.
Files Affected
windows_pools_test.ps1- Unknown (any shared test helper that reads/writes
.windows-autoscale-state, if one exists)
Constraints
- Must not weaken or remove the existing
-contains $idleSincecontent assertions. - Must not change production behavior in
Invoke-AutoscalePassor the state serializer — this is a test-only change. - Must not introduce new dependencies or new GitHub API calls.
- Out of scope: the separate suggested follow-up to add a comment explaining the exact-line assertion is intentionally format-sensitive (track separately if desired).
- Out of scope: any refactor of the state serializer.
LLM tier
Sonnet — small, focused test change, but requires judgment about timestamp resolution, where to place the assertion, and how to keep it robust across platforms.
Success looks like
- The test captures the state file's
LastWriteTimeUtcbefore the pass and asserts it is unchanged after the pass, in both the refused-shrink and GitHub-outage scenarios. - The existing
-contains $idleSinceassertions remain in place. - A regression that rewrites
.windows-autoscale-statewith the same content now fails the test. - The test suite passes on the supported CI platform(s).
Failure looks like
- The mtime assertion is flaky due to filesystem timestamp resolution (false failures on unchanged files).
- The content assertions were removed or weakened in the process.
- Production code was modified to make the test pass.
- The assertion only covers one of the two scenarios (refused shrink, GitHub outage).
Follow-up from AI review of PR #120.
Activity