From bd8b9eaf9d90f93deee6c9a46380354e3b4cca3c Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 26 May 2026 16:14:15 -0400 Subject: [PATCH] fix(#344): harden windows test assertions and pin windows-2025 labels (#349) --- .github/workflows/test-skip.yml | 2 +- .github/workflows/test.yml | 13 +++---------- tests/commands.test.cjs | 2 +- tests/workflow-shell-pinning.test.cjs | 16 ++++++++-------- 4 files changed, 13 insertions(+), 20 deletions(-) diff --git a/.github/workflows/test-skip.yml b/.github/workflows/test-skip.yml index 0b2eef070..75ad05a40 100644 --- a/.github/workflows/test-skip.yml +++ b/.github/workflows/test-skip.yml @@ -41,7 +41,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest, windows-latest] + os: [ubuntu-latest, macos-latest, windows-2025] node-version: [22, 24] steps: - run: "echo 'doc-only PR: skipping test on node ${{ matrix.node-version }} / ${{ matrix.os }}'" diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fd046111c..6d057da33 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -68,22 +68,15 @@ jobs: # makes OS-specific vs version-specific regressions easier to distinguish. fail-fast: false matrix: - os: [ubuntu-latest, macos-latest, windows-latest] + os: [ubuntu-latest, macos-latest, windows-2025] node-version: [22, 24] # Windows path/separator coverage is also reinforced by hardcoded-paths.test.cjs # and windows-robustness.test.cjs (static analysis). # A dedicated windows-compat workflow runs on a weekly schedule. steps: - # actions/checkout@v6 uses includeIf.gitdir: to inject auth on Windows. - # On Windows git 2.54, the gitdir path comparison (forward-slash key vs - # backslash-resolved gitdir) is unreliable, so the conditional include - # intermittently fails to fire and the fetch proceeds unauthenticated. - # - # Fix: use actions/checkout@v4 on Windows only. v4 writes the auth token - # directly to .git/config (http.extraheader) instead of using includeIf, - # which is reliable across all git versions and platforms. - # Linux/macOS continue using v6 (includeIf works correctly there). + # Windows lane stays on checkout v4 for reliable auth/header behavior. + # (v6's includeIf.gitdir flow has shown intermittent Windows auth drift.) - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 (Windows) if: runner.os == 'Windows' with: diff --git a/tests/commands.test.cjs b/tests/commands.test.cjs index 0413889f3..8ba596a34 100644 --- a/tests/commands.test.cjs +++ b/tests/commands.test.cjs @@ -892,7 +892,7 @@ describe('current-timestamp command', () => { test('dispatches directly to CJS handler (no SDK bridge) to avoid Windows native crash path', () => { const sourcePath = path.join(__dirname, '..', 'get-shit-done', 'bin', 'gsd-tools.cjs'); const source = fs.readFileSync(sourcePath, 'utf8'); - const match = source.match(/case 'current-timestamp':\s*\{[\s\S]*?\n\s*break;\n\s*\}/); + const match = source.match(/case 'current-timestamp':\s*\{[\s\S]*?\r?\n\s*break;\r?\n\s*\}/); assert.ok(match, 'current-timestamp case block must exist in gsd-tools.cjs'); diff --git a/tests/workflow-shell-pinning.test.cjs b/tests/workflow-shell-pinning.test.cjs index 1d0721440..381417b6f 100644 --- a/tests/workflow-shell-pinning.test.cjs +++ b/tests/workflow-shell-pinning.test.cjs @@ -10,11 +10,11 @@ process.env.GSD_TEST_MODE = '1'; * step.shell ?? job.defaults.run.shell ?? workflow.defaults.run.shell * * Without an effective shell, GitHub Actions defaults to pwsh on - * windows-latest. The npm.cmd → node.exe → npm-cli.js child-process chain + * windows-latest / windows-2025. The npm.cmd → node.exe → npm-cli.js child-process chain * under pwsh can swallow stderr, making `npm ci` / `npm run` failures * invisible in CI logs. * - * Scope: only workflow files that reference windows-latest (directly as a + * Scope: only workflow files that reference a Windows hosted runner label * literal `runs-on:` value, or as a member of a `strategy.matrix.os` list). * Steps in jobs that cannot run on Windows cannot trigger the class of failure * described above, but the file must still be scanned once any job in it @@ -35,8 +35,8 @@ const WORKFLOWS_DIR = path.join(REPO_ROOT, '.github', 'workflows'); /** * Collect all .yml / .yaml files under the workflows directory. * - * Only files that reference "windows-latest" (as a literal runs-on value or - * inside a matrix.os list) are returned — the pwsh-stderr-swallow failure + * Only files that reference a Windows hosted label (`windows-latest` or + * `windows-2025`, as a literal runs-on value or inside a matrix.os list) are * class can only manifest in workflows that target Windows runners. */ function listWorkflowFiles() { @@ -45,17 +45,17 @@ function listWorkflowFiles() { .filter((e) => /\.ya?ml$/.test(e)) .map((e) => path.join(WORKFLOWS_DIR, e)); - // Filter to files that have at least one windows-latest reference. + // Filter to files that have at least one Windows hosted-runner reference. // allow-test-rule: file-scope prefilter, not a test assertion — we need to // detect whether a workflow file targets Windows runners at all. The pwsh // stderr-swallow class is windows-only, so files that never mention - // windows-latest are out of scope. Exposing a typed IR from production + // windows-hosted labels are out of scope. Exposing a typed IR from production // code is not appropriate here because the source-of-truth is the YAML // itself; the actual test assertions below ARE structural (parse runs-on, // strategy.matrix.os, defaults.run.shell, etc.). return all.filter((f) => { const raw = fs.readFileSync(f, 'utf8'); - return raw.indexOf('windows-latest') !== -1; + return raw.includes('windows-latest') || raw.includes('windows-2025'); }); } @@ -332,7 +332,7 @@ describe('GitHub Actions workflow shell pinning', () => { ).join('\n'); assert.fail( `${allViolations.length} npm run/ci step(s) are missing an explicit shell: directive.\n` + - `On windows-latest, steps without shell: default to pwsh, which can swallow npm stderr.\n` + + `On Windows hosted runners, steps without shell: default to pwsh, which can swallow npm stderr.\n` + `Add shell: bash (or another explicit shell) to each listed step:\n\n` + details, );