fix(test): shared Docker skip guard + template-sync timeout (#677)#679
fix(test): shared Docker skip guard + template-sync timeout (#677)#679tamirdresher wants to merge 1 commit intodevfrom
Conversation
…nc timeout - Create test/helpers/skip-guards.ts with reusable isDockerAvailable() and dockerSkipReason() functions - Refactor aspire-integration.test.ts to use shared helper instead of inline Docker detection - Increase template-sync.test.ts script execution timeout from 30s to 60s to handle CI resource contention Refs #582, Closes #677 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Test-infrastructure stabilization aimed at reducing CI flakiness by centralizing Docker skip-guard logic and relaxing a known timeout in the template-sync execution test.
Changes:
- Added a shared Docker skip-guard helper (
isDockerAvailable,dockerSkipReason). - Refactored
aspire-integration.test.tsto use the shared skip-guard helper. - Increased
execSynctimeout intemplate-sync.test.tsfrom 30s to 60s.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/template-sync.test.ts | Extends script-execution timeout to reduce contention-related flake. |
| test/helpers/skip-guards.ts | Introduces shared helper functions for Docker-dependent test skipping. |
| test/aspire-integration.test.ts | Replaces inline Docker detection with the shared skip-guard helper. |
| * Shared test helpers for skip guards and environment detection. | ||
| * | ||
| * Provides reusable functions for detecting Docker availability, | ||
| * shell module availability, and other environment conditions | ||
| * that determine whether certain test suites should run. |
There was a problem hiding this comment.
The file header comment claims this helper also covers “shell module availability” and other conditions, but this module currently only implements Docker helpers. This makes the comment misleading—either remove that claim or add the corresponding helper(s).
| * Shared test helpers for skip guards and environment detection. | |
| * | |
| * Provides reusable functions for detecting Docker availability, | |
| * shell module availability, and other environment conditions | |
| * that determine whether certain test suites should run. | |
| * Shared test helpers for Docker-related skip guards and environment detection. | |
| * | |
| * Provides reusable functions for detecting Docker availability | |
| * and determining whether Docker-dependent test suites should run. |
| * Check if Docker is available on this machine. | ||
| * Returns true if `docker --version` succeeds within 5 seconds. | ||
| */ | ||
| export function isDockerAvailable(): boolean { | ||
| try { | ||
| execSync('docker --version', { stdio: 'ignore', timeout: 5000 }); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
isDockerAvailable() currently treats docker --version as proof Docker is “available”, but that succeeds even when the Docker daemon isn’t running (common cause of the reported failures). To make the skip guard actually prevent Docker API connection errors, check daemon connectivity instead (e.g., docker info / docker ps with a short timeout) and return false when the server isn’t reachable.
|
@bradygaster @diberry FYI - dev branch has a pre-existing TS build error: ensurePersonalSquadDir not resolved through subpath exports in personal.ts and init.ts. Function exists in resolution.ts:351 but tsc cannot find it. This blocks CI for all PRs including this one. Happy to help debug. |
What
Adds a shared Docker skip guard helper and increases the template-sync timeout to fix flaky tests under full-suite load.
Why
Issue #582 reports ~24 intermittent test failures. This PR addresses the survivable subset (tests not being removed by #675 REPL removal):
Note: speed-gates.test.ts, human-journeys.test.ts, and repl-ux-fixes.test.ts are also flaky but import from shell/ which is being removed by PR #675 — those are intentionally not touched here.
Closes #677, Refs #582
How
Testing
Docs
N/A — test infrastructure only (exempt per PR_REQUIREMENTS.md)
Exports
N/A
Breaking Changes
None
Waivers
None required — test-only changes are exempt from Documentation, Exports, and Samples requirements.