fix(test): Docker skip guards + flaky test stabilization (#677)

test/aspire-integration.test.ts

@@ -15,6 +15,7 @@ import { trace, metrics } from '@opentelemetry/api';
 import { NodeSDK, resources, metrics as sdkMetrics } from '@opentelemetry/sdk-node';
 import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-grpc';
 import { OTLPMetricExporter } from '@opentelemetry/exporter-metrics-otlp-grpc';
+import { dockerSkipReason } from './helpers/skip-guards.js';
 
 const { Resource } = resources;
 const { PeriodicExportingMetricReader } = sdkMetrics;
@@ -23,20 +24,7 @@ const { PeriodicExportingMetricReader } = sdkMetrics;
 // Skip guard — bail early if Docker is unavailable or tests disabled
 // ============================================================================
 
-function dockerAvailable(): boolean {
-  try {
-    execSync('docker --version', { stdio: 'ignore' });
-    return true;
-  } catch {
-    return false;
-  }
-}
-
-const SKIP_REASON = process.env['SKIP_DOCKER_TESTS'] === '1'
-  ? 'SKIP_DOCKER_TESTS=1'
-  : !dockerAvailable()
-    ? 'Docker not available'
-    : null;
+const SKIP_REASON = dockerSkipReason();
 
 const CONTAINER_NAME = 'squad-aspire-dashboard';
 const DASHBOARD_URL = 'http://localhost:18888';
@@ -71,14 +59,17 @@ function removeContainer(): void {
 }
 
 // Best-effort cleanup on unexpected exit (Ctrl+C, uncaught exception, etc.)
-// This prevents orphaned containers when the test runner is interrupted.
-for (const signal of ['SIGINT', 'SIGTERM'] as const) {
-  process.once(signal, () => {
-    removeContainer();
-    process.exit(128 + (signal === 'SIGINT' ? 2 : 15));
-  });
+// Only register handlers when Docker tests will actually run — avoids
+// Docker side effects (and stale removeContainer calls) in skip mode.
+if (SKIP_REASON === null) {
+  for (const signal of ['SIGINT', 'SIGTERM'] as const) {
+    process.once(signal, () => {
+      removeContainer();
+      process.exit(128 + (signal === 'SIGINT' ? 2 : 15));
+    });
+  }
+  process.once('exit', () => removeContainer());
 }
-process.once('exit', () => removeContainer());
 
 // ============================================================================
 // OTel setup — NodeSDK with gRPC exporters targeting the Aspire dashboard

test/cli/aspire.test.ts

@@ -18,6 +18,13 @@ import { describe, it, expect, vi, afterEach } from 'vitest';
 import { execSync } from 'child_process';
 import { existsSync } from 'fs';
 import { join } from 'path';
+import { dockerSkipReason } from '../helpers/skip-guards.js';
+
+// ===========================================================================
+// Skip guard — bail early if Docker is unavailable or tests disabled
+// ===========================================================================
+
+const SKIP_REASON = dockerSkipReason();
 
 // ===========================================================================
 // Docker availability helpers (mockable)
@@ -63,29 +70,39 @@ function buildAspireStopCommands(name = 'squad-aspire-dashboard'): string[][] {
 }
 
 // ===========================================================================
-// Docker availability
+// Docker availability — requires real Docker
 // ===========================================================================
 
-describe('CLI: squad aspire — Docker availability', { timeout: 30_000 }, () => {
+describe.skipIf(SKIP_REASON !== null)(
+  `CLI: squad aspire — Docker availability (${SKIP_REASON ?? 'enabled'})`,
+  { timeout: 30_000 },
+  () => {
   it('checkDockerAvailability returns version string when Docker is present', () => {
     const result = checkDockerAvailability();
-    if (result === null) {
-      console.warn('[ENV] Docker not available — skipping Docker detection test');
-      return;
-    }
+    expect(result).not.toBeNull();
     expect(result).toMatch(/docker/i);
   });
+});
+
+// ===========================================================================
+// Docker availability — mocked (always runs, no Docker required)
+// ===========================================================================
 
-  it('checkDockerAvailability returns null when Docker is absent', () => {
+describe('CLI: squad aspire — Docker detection (mocked)', () => {
+  it('checkDockerAvailability returns null when Docker CLI is absent', () => {
     const mockExecSync = vi.fn(() => { throw new Error('docker not found'); });
     let result: string | null;
     try {
-      mockExecSync();
+      mockExecSync('docker --version', { encoding: 'utf-8', timeout: 5000 });
       result = 'unexpected';
     } catch {
       result = null;
     }
     expect(result).toBeNull();
+    expect(mockExecSync).toHaveBeenCalledWith(
+      'docker --version',
+      expect.objectContaining({ encoding: 'utf-8' }),
+    );
   });
 });
 

test/helpers/skip-guards.ts

@@ -0,0 +1,35 @@
+/**
+ * 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.
+ */
+
+import { execSync } from 'node:child_process';
+
+/**
+ * Check if Docker is usable on this machine.
+ * Returns true if `docker info` succeeds within 5 seconds.
+ */
+export function isDockerAvailable(): boolean {
+  try {
+    execSync('docker info', { stdio: 'ignore', timeout: 5000 });
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+/**
+ * Determine skip reason for Docker-dependent tests.
+ * Returns null if tests should run, or a string reason to skip.
+ */
+export function dockerSkipReason(): string | null {
+  if (process.env['SKIP_DOCKER_TESTS'] === '1') {
+    return 'SKIP_DOCKER_TESTS=1';
+  }
+  if (!isDockerAvailable()) {
+    return 'Docker not available';
+  }
+  return null;
+}

test/template-sync.test.ts

@@ -153,7 +153,7 @@ describe('sync-templates.mjs script execution', () => {
     const output = execSync('node scripts/sync-templates.mjs', {
       cwd: ROOT,
       encoding: 'utf-8',
-      timeout: 30_000,
+      timeout: 60_000,
     });
     expect(output).toContain('Synced');
   });