Repository navigation
Description
What
Add test coverage in tests/pools_test.sh for two currently untested behaviours in pools.sh:
- The
--intervalloop. All existing tests exercise--once. Thewhile :; do ... sleep "$interval"; doneloop (and its subshell isolation) has no test, so a regression that breaks iteration or leaks state between ticks would go undetected. in_progressruns with queued jobs. The stub used by the tests returns nothing forin_progressruns, so the code path where a run is alreadyin_progressbut still hasqueuedjobs (which should still contribute to demand) is never exercised. This is a stated feature of the autoscaler.
Why
- The
--intervalloop is the primary production entry point; only the one-shot path is verified. A broken loop or a subshell-isolation bug would silently stop autoscaling. - The
in_progress+ queued-jobs path is a documented behaviour with no regression protection. Ifqueued_jobsor the demand calculation is refactored (e.g. the pagination orfilter=latesthandling flagged in the review), this case could break without any test failing. - Both gaps were called out as non-blocking follow-ups in the AI review of PR Add pools.sh autoscale: resize pools to their queued jobs #113.
How
-
--intervalloop test. Add a test that runspools.sh autoscale --interval <small>(e.g.1) against the existing stub, lets it tick at least twice, then terminates it (sendSIGTERM/SIGINTor run undertimeout). Assert:- The stub was invoked more than once (e.g. count invocations of the
ghstub, or count distinct state-file writes). - The process exits cleanly on the signal rather than hanging.
- State written on tick N does not corrupt tick N+1 (subshell isolation).
- The stub was invoked more than once (e.g. count invocations of the
-
in_progress+ queued jobs test. Extend theghstub so that for a chosen run it returnsstatus: "in_progress"on the run object while the/jobsendpoint returns one or more jobs withstatus: "queued"and labels matching a pool. Assert that the pool is brought up (or demand is counted) exactly as it would be for a fully queued run. Add a companion assertion that anin_progressrun with no queued jobs does not trigger a scale-up, to pin the boundary. -
Keep both tests in the existing style of
tests/pools_test.sh(same stub helpers, same assertion helpers such ascheck_absent).
Files Affected
tests/pools_test.shpools.sh(only if the stub or signal handling needs a small hook to make the loop testable; prefer not to change it)
Constraints
- Must not change production behaviour in
pools.shbeyond what is strictly needed to make the loop testable; the--oncepath and all existing tests must continue to pass unchanged. - Must not introduce new runtime dependencies (no new tools beyond what the test suite already uses).
- Out of scope: the other follow-ups from the review (per-run 404 handling in
queued_jobs,host_label.envparsing, documenting.autoscale-stateCWD-relativity), and the--dry-runstate-file assertion. - The
--intervaltest must not be flaky: use a short interval and a bounded wait, and fail loudly on timeout rather than hanging CI.
LLM tier
Sonnet. This requires designing new test cases against an existing stub harness, including signal handling and stub extension — moderate design judgment, not a mechanical change.
Success looks like
tests/pools_test.shcontains a test that runs the autoscaler with--intervaland asserts it ticks more than once and exits cleanly on termination.tests/pools_test.shcontains a test where anin_progressrun has queued jobs and the pool is scaled up accordingly, plus a negative case where anin_progressrun with no queued jobs does not scale up.- The full test suite passes locally and in CI, including all pre-existing tests.
- No production behaviour changes beyond what is required for testability.
Failure looks like
- The
--intervaltest hangs CI or is skipped/disabled to avoid flakiness. - The
in_progresstest only asserts on the stub's output rather than on the autoscaler's resulting action (e.g. the composeupcall or state file), so it would pass even if the demand calculation ignoredin_progress
Activity