Skip to content

Pin the exact idle clock in Windows autoscale refusal tests - #120

Merged
leonarduk merged 1 commit into
mainfrom
test/windows-autoscale-idle-clock
Sep 18, 2026
Merged

leonarduk merged 1 commit into
mainfrom
test/windows-autoscale-idle-clock

Conversation

@leonarduk

Copy link
Copy Markdown
Owner

Closes #119

Summary

  • tests\windows_pools_test.ps1: after a refused shrink and after a GitHub outage, assert the exact .windows-autoscale-state line is unchanged, not just that it matches ^as \d+$. That pattern would also pass a clock reset to now. The code was already correct; DeepSeek's claim on Add windows-pools.ps1 autoscale for the native Windows fleet #118 that it resets the clock was a misread, since since is loaded from the state before the timeout comparison.
  • windows/README.md "Autoscaling": API cost per pass, and how to see the real error behind "could not ask GitHub".

Test plan

  • tests\windows_pools_test.ps1 under pwsh and Windows PowerShell 5.1: all checks passed on both

🤖 Generated with Claude Code

Matching '^as \d+$' also passes if a refused shrink or an outage reset the
clock to now, which would silently restart every pool's idle countdown.

Closes #119

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: e27aba71-e0b9-4472-9b41-61c4a4057f28


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

GPT AI Code Review

Skipped: API key not configured

The OPENAI_API_KEY repository secret is not set.

To enable GPT reviews, add OPENAI_API_KEY as a repository secret:

  1. Go to Settings > Secrets and variables > Actions in this repo.
  2. Click New repository secret.
  3. Name: OPENAI_API_KEY
  4. Value: your GPT API key.

Once the secret is configured, re-run this workflow or push a new commit to trigger a fresh review.

REVIEW SKIPPED - API KEY NOT CONFIGURED


Reviewed by GPT via gpt-pr-review.yml. Advisory only.

@github-actions

Copy link
Copy Markdown

DeepSeek AI Code Review

1. Acceptance criteria

Both ACs are met:

  • The test now captures the exact state line in $idleSince and asserts -contains $idleSince after both the refused shrink and the GitHub outage, replacing the weak -match '^as \d+$' check. A regression that reset the clock to "now" would now fail, since the new value would differ from the captured $idleSince.
  • windows/README.md gains a note on the per-pass API cost (one call per in-flight run per repo, plus runners) and points at gh api repos/<owner>/<repo>/actions/runs to see the real error behind "could not ask GitHub".

2. Bugs and logic errors

One subtle correctness concern in the test change:

Get-Content $asState returns an array of lines. -contains on an array checks membership of an element, which is what you want here — but only if the file has exactly one line and no trailing whitespace/CRLF mismatch. Set-Content on Windows writes CRLF line endings by default, and Get-Content strips them, so the round-trip is consistent. That's fine.

However, -contains is case-insensitive and does an exact string comparison of the element, which is stricter than -match and is the intended tightening. No bug there.

The real risk: if Invoke-AutoscalePass ever rewrites the state file with a normalized/trimmed value (e.g. re-serializing since through a different code path), the exact-string assertion could become brittle and fail for a benign reason. Given the issue states the code is correct today and loads since from saved state before comparing, this is acceptable — but it does couple the test to the exact serialization format. Worth a comment in the test noting that the assertion is intentionally format-sensitive.

3. CLI and API safety

No CLI signature changes, no new dependencies, no new GitHub API calls introduced by this PR. The README note about API cost is accurate and does not misstate token scope or rate-limit behavior.

4. Test coverage

The tightened assertions cover both AC scenarios (refused shrink, GitHub outage). One gap worth noting: the test does not assert that the state file is not rewritten at all (e.g. mtime unchanged), only that its content matches. A regression that rewrote the file with the same value would pass. That's a minor gap and probably out of scope for this issue.

5. Suggested follow-up issues

  • Add a comment in windows_pools_test.ps1 explaining that the exact-line assertion is intentionally format-sensitive, so future refactors of the state serializer don't silently weaken it.
  • Consider asserting state-file mtime is unchanged (not just content) to catch no-op rewrites of .windows-autoscale-state.

APPROVE — both acceptance criteria are satisfied; the only observations are non-blocking test-robustness notes.


Reviewed by DeepSeek via deepseek-pr-review.yml. Advisory only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows autoscale: idle-clock test can't tell a preserved clock from a reset one

1 participant