From 616c1fa753ddbc8a0b476caf3038f6914cf35414 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 24 Mar 2026 15:45:39 -0400 Subject: [PATCH] refactor: replace try/finally with beforeEach/afterEach + add CONTRIBUTING.md Test suite modernization: - Converted all try/finally cleanup patterns to beforeEach/afterEach hooks across 11 test files (core, copilot-install, config, workstream, milestone-summary, forensics, state, antigravity, profile-pipeline, workspace) - Consolidated 40 inline mkdtempSync calls to use centralized helpers - Added createTempDir() helper for bare temp directories - Added optional prefix parameter to createTempProject/createTempGitProject - Fixed config test HOME sandboxing (was reading global defaults.json) New CONTRIBUTING.md: - Test standards: hooks over try/finally, centralized helpers, HOME sandboxing - Node 22/24 compatibility requirements with Node 26 forward-compat - Code style, PR guidelines, security practices - File structure overview All 1382 tests pass, 0 failures. Co-Authored-By: Claude Opus 4.6 (1M context) --- CONTRIBUTING.md | 196 ++++++++++++++++++++ tests/antigravity-install.test.cjs | 5 +- tests/config.test.cjs | 78 ++++---- tests/copilot-install.test.cjs | 276 +++++++++++++---------------- tests/core.test.cjs | 238 ++++++++++++------------- tests/forensics.test.cjs | 87 ++++----- tests/helpers.cjs | 15 +- tests/milestone-summary.test.cjs | 131 ++++++-------- tests/profile-pipeline.test.cjs | 6 +- tests/state.test.cjs | 27 +-- tests/workspace.test.cjs | 12 +- tests/workstream.test.cjs | 21 ++- 12 files changed, 623 insertions(+), 469 deletions(-) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..1ffb4ace9 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,196 @@ +# Contributing to GSD + +## Getting Started + +```bash +# Clone the repo +git clone https://github.com/gsd-build/get-shit-done.git +cd get-shit-done + +# Install dependencies +npm install + +# Run tests +npm test +``` + +## Pull Request Guidelines + +- **One concern per PR** — bug fixes, features, and refactors should be separate PRs +- **No drive-by formatting** — don't reformat code unrelated to your change +- **Link issues** — use `Fixes #123` or `Closes #123` in PR body for auto-close +- **CI must pass** — all matrix jobs (Ubuntu, macOS, Windows × Node 22, 24) must be green + +## Testing Standards + +All tests use Node.js built-in test runner (`node:test`) and assertion library (`node:assert`). **Do not use Jest, Mocha, Chai, or any external test framework.** + +### Required Imports + +```javascript +const { describe, it, test, beforeEach, afterEach, before, after } = require('node:test'); +const assert = require('node:assert/strict'); +``` + +### Setup and Cleanup: Use Hooks, Not try/finally + +**Always use `beforeEach`/`afterEach` for setup and cleanup.** Do not use `try/finally` blocks for test cleanup — they are verbose, error-prone, and can mask test failures. + +```javascript +// GOOD — hooks handle setup/cleanup +describe('my feature', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('does the thing', () => { + // test body focuses only on the assertion + assert.strictEqual(result, expected); + }); +}); +``` + +```javascript +// BAD — try/finally is verbose and masks failures +test('does the thing', () => { + const tmpDir = createTempProject(); + try { + // test body + assert.strictEqual(result, expected); + } finally { + cleanup(tmpDir); + } +}); +``` + +### Use Centralized Test Helpers + +Import helpers from `tests/helpers.cjs` instead of inlining temp directory creation: + +```javascript +const { createTempProject, createTempGitProject, createTempDir, cleanup, runGsdTools } = require('./helpers.cjs'); +``` + +| Helper | Creates | Use When | +|--------|---------|----------| +| `createTempProject(prefix?)` | tmpDir with `.planning/phases/` | Testing GSD tools that need planning structure | +| `createTempGitProject(prefix?)` | Same + git init + initial commit | Testing git-dependent features | +| `createTempDir(prefix?)` | Bare temp directory | Testing features that don't need `.planning/` | +| `cleanup(tmpDir)` | Removes directory recursively | Always use in `afterEach` | +| `runGsdTools(args, cwd, env?)` | Executes gsd-tools.cjs | Testing CLI commands | + +### Test Structure + +```javascript +describe('featureName', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + // Additional setup specific to this suite + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('handles normal case', () => { + // Arrange + // Act + // Assert + }); + + test('handles edge case', () => { + // ... + }); + + describe('sub-feature', () => { + // Nested describes can have their own hooks + beforeEach(() => { + // Additional setup for sub-feature + }); + + test('sub-feature works', () => { + // ... + }); + }); +}); +``` + +### Node.js Version Compatibility + +Tests must pass on: +- **Node 22** (LTS) +- **Node 24** (Current) + +Forward-compatible with Node 26. Do not use: +- Deprecated APIs +- Version-specific features not available in Node 22 + +Safe to use: +- `node:test` — stable since Node 18, fully featured in 22+ +- `describe`/`it`/`test` — all supported +- `beforeEach`/`afterEach`/`before`/`after` — all supported +- `t.plan()` — available since Node 22.2 +- Snapshot testing — available since Node 22.3 + +### Assertions + +Use `node:assert/strict` for strict equality by default: + +```javascript +const assert = require('node:assert/strict'); + +assert.strictEqual(actual, expected); // === +assert.deepStrictEqual(actual, expected); // deep === +assert.ok(value); // truthy +assert.throws(() => { ... }, /pattern/); // throws +assert.rejects(async () => { ... }); // async throws +``` + +### Running Tests + +```bash +# Run all tests +npm test + +# Run a single test file +node --test tests/core.test.cjs + +# Run with coverage +npm run test:coverage +``` + +## Code Style + +- **CommonJS** (`.cjs`) — the project uses `require()`, not ESM `import` +- **No external dependencies in core** — `gsd-tools.cjs` and all lib files use only Node.js built-ins +- **Conventional commits** — `feat:`, `fix:`, `docs:`, `refactor:`, `test:`, `ci:` + +## File Structure + +``` +bin/install.js — Installer (multi-runtime) +get-shit-done/ + bin/lib/ — Core library modules (.cjs) + workflows/ — Workflow definitions (.md) + references/ — Reference documentation (.md) + templates/ — File templates +agents/ — Agent definitions (.md) +commands/gsd/ — Slash command definitions (.md) +tests/ — Test files (.test.cjs) + helpers.cjs — Shared test utilities +docs/ — User-facing documentation +``` + +## Security + +- **Path validation** — use `validatePath()` from `security.cjs` for any user-provided paths +- **No shell injection** — use `execFileSync` (array args) over `execSync` (string interpolation) +- **No `${{ }}` in GitHub Actions `run:` blocks** — bind to `env:` mappings first diff --git a/tests/antigravity-install.test.cjs b/tests/antigravity-install.test.cjs index baa04ed10..49ba3f135 100644 --- a/tests/antigravity-install.test.cjs +++ b/tests/antigravity-install.test.cjs @@ -12,6 +12,7 @@ const assert = require('node:assert'); const path = require('path'); const os = require('os'); const fs = require('fs'); +const { createTempDir, cleanup } = require('./helpers.cjs'); const { getDirName, @@ -287,7 +288,7 @@ describe('copyCommandsAsAntigravitySkills', () => { let skillsDir; beforeEach(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-ag-test-')); + tmpDir = createTempDir('gsd-ag-test-'); srcDir = path.join(tmpDir, 'commands', 'gsd'); skillsDir = path.join(tmpDir, 'skills'); fs.mkdirSync(srcDir, { recursive: true }); @@ -381,7 +382,7 @@ describe('writeManifest (Antigravity)', () => { let tmpDir; beforeEach(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-manifest-ag-')); + tmpDir = createTempDir('gsd-manifest-ag-'); // Create minimal structure const skillsDir = path.join(tmpDir, 'skills', 'gsd-help'); fs.mkdirSync(skillsDir, { recursive: true }); diff --git a/tests/config.test.cjs b/tests/config.test.cjs index 9671dbc0e..31a2da656 100644 --- a/tests/config.test.cjs +++ b/tests/config.test.cjs @@ -256,8 +256,8 @@ describe('config-get command', () => { beforeEach(() => { tmpDir = createTempProject(); - // Create config with known values - runGsdTools('config-ensure-section', tmpDir); + // Create config with known values — sandbox HOME to avoid global defaults + runGsdTools('config-ensure-section', tmpDir, { HOME: tmpDir, USERPROFILE: tmpDir }); }); afterEach(() => { @@ -265,7 +265,7 @@ describe('config-get command', () => { }); test('gets a top-level value', () => { - const result = runGsdTools('config-get model_profile', tmpDir); + const result = runGsdTools('config-get model_profile', tmpDir, { HOME: tmpDir, USERPROFILE: tmpDir }); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -298,18 +298,25 @@ describe('config-get command', () => { ); }); - test('errors when config.json does not exist', () => { - const emptyTmpDir = createTempProject(); - try { + describe('when config.json does not exist', () => { + let emptyTmpDir; + + beforeEach(() => { + emptyTmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(emptyTmpDir); + }); + + test('errors when config.json does not exist', () => { const result = runGsdTools('config-get model_profile', emptyTmpDir); assert.strictEqual(result.success, false); assert.ok( result.error.includes('No config.json'), `Expected "No config.json" in error: ${result.error}` ); - } finally { - cleanup(emptyTmpDir); - } + }); }); test('errors when no key path provided', () => { @@ -484,7 +491,7 @@ describe('config-set research_before_questions and discuss_mode', () => { beforeEach(() => { tmpDir = createTempProject(); - runGsdTools('config-ensure-section', tmpDir); + runGsdTools('config-ensure-section', tmpDir, { HOME: tmpDir, USERPROFILE: tmpDir }); }); afterEach(() => { @@ -599,7 +606,7 @@ describe('config-set-model-profile command', () => { beforeEach(() => { tmpDir = createTempProject(); - runGsdTools('config-ensure-section', tmpDir); + runGsdTools('config-ensure-section', tmpDir, { HOME: tmpDir, USERPROFILE: tmpDir }); }); afterEach(() => { @@ -620,7 +627,7 @@ describe('config-set-model-profile command', () => { }); test('reports previous profile in output', () => { - const result = runGsdTools('config-set-model-profile budget', tmpDir); + const result = runGsdTools('config-set-model-profile budget', tmpDir, { HOME: tmpDir, USERPROFILE: tmpDir }); assert.ok(result.success, `Command failed: ${result.error}`); const out = JSON.parse(result.output); @@ -661,17 +668,24 @@ describe('config-set-model-profile command', () => { assert.strictEqual(result.success, false); }); - test('creates config if missing before setting profile', () => { - const emptyDir = createTempProject(); - try { + describe('when config is missing', () => { + let emptyDir; + + beforeEach(() => { + emptyDir = createTempProject(); + }); + + afterEach(() => { + cleanup(emptyDir); + }); + + test('creates config if missing before setting profile', () => { const result = runGsdTools('config-set-model-profile budget', emptyDir); assert.ok(result.success, `Command failed: ${result.error}`); const config = readConfig(emptyDir); assert.strictEqual(config.model_profile, 'budget'); - } finally { - cleanup(emptyDir); - } + }); }); }); @@ -711,22 +725,26 @@ describe('config-set workflow.skip_discuss', () => { assert.strictEqual(config.workflow.skip_discuss, false); }); - test('skip_discuss is present in config-new-project output', () => { - const emptyDir = createTempProject(); - try { + describe('skip_discuss in config-new-project', () => { + let emptyDir; + + beforeEach(() => { + emptyDir = createTempProject(); + }); + + afterEach(() => { + cleanup(emptyDir); + }); + + test('skip_discuss is present in config-new-project output', () => { const result = runGsdTools(['config-new-project', '{}'], emptyDir, { HOME: emptyDir, USERPROFILE: emptyDir }); assert.ok(result.success, `Command failed: ${result.error}`); const config = readConfig(emptyDir); assert.strictEqual(config.workflow.skip_discuss, false, 'skip_discuss should default to false'); - } finally { - cleanup(emptyDir); - } - }); + }); - test('skip_discuss can be set via config-new-project choices', () => { - const emptyDir = createTempProject(); - try { + test('skip_discuss can be set via config-new-project choices', () => { const choices = JSON.stringify({ workflow: { skip_discuss: true }, }); @@ -735,9 +753,7 @@ describe('config-set workflow.skip_discuss', () => { const config = readConfig(emptyDir); assert.strictEqual(config.workflow.skip_discuss, true); - } finally { - cleanup(emptyDir); - } + }); }); test('config-get workflow.skip_discuss returns the set value', () => { diff --git a/tests/copilot-install.test.cjs b/tests/copilot-install.test.cjs index 46436d5f1..744643d01 100644 --- a/tests/copilot-install.test.cjs +++ b/tests/copilot-install.test.cjs @@ -51,54 +51,41 @@ describe('getDirName (Copilot)', () => { // ─── getGlobalDir ─────────────────────────────────────────────────────────────── describe('getGlobalDir (Copilot)', () => { - test('returns ~/.copilot with no env var or explicit dir', () => { - const original = process.env.COPILOT_CONFIG_DIR; - try { + let originalCopilotConfigDir; + + beforeEach(() => { + originalCopilotConfigDir = process.env.COPILOT_CONFIG_DIR; + }); + + afterEach(() => { + if (originalCopilotConfigDir !== undefined) { + process.env.COPILOT_CONFIG_DIR = originalCopilotConfigDir; + } else { delete process.env.COPILOT_CONFIG_DIR; - const result = getGlobalDir('copilot'); - assert.strictEqual(result, path.join(os.homedir(), '.copilot')); - } finally { - if (original !== undefined) { - process.env.COPILOT_CONFIG_DIR = original; - } else { - delete process.env.COPILOT_CONFIG_DIR; - } } }); + test('returns ~/.copilot with no env var or explicit dir', () => { + delete process.env.COPILOT_CONFIG_DIR; + const result = getGlobalDir('copilot'); + assert.strictEqual(result, path.join(os.homedir(), '.copilot')); + }); + test('returns explicit dir when provided', () => { const result = getGlobalDir('copilot', '/custom/path'); assert.strictEqual(result, '/custom/path'); }); test('respects COPILOT_CONFIG_DIR env var', () => { - const original = process.env.COPILOT_CONFIG_DIR; - try { - process.env.COPILOT_CONFIG_DIR = '~/custom-copilot'; - const result = getGlobalDir('copilot'); - assert.strictEqual(result, path.join(os.homedir(), 'custom-copilot')); - } finally { - if (original !== undefined) { - process.env.COPILOT_CONFIG_DIR = original; - } else { - delete process.env.COPILOT_CONFIG_DIR; - } - } + process.env.COPILOT_CONFIG_DIR = '~/custom-copilot'; + const result = getGlobalDir('copilot'); + assert.strictEqual(result, path.join(os.homedir(), 'custom-copilot')); }); test('explicit dir takes priority over COPILOT_CONFIG_DIR', () => { - const original = process.env.COPILOT_CONFIG_DIR; - try { - process.env.COPILOT_CONFIG_DIR = '~/env-path'; - const result = getGlobalDir('copilot', '/explicit/path'); - assert.strictEqual(result, '/explicit/path'); - } finally { - if (original !== undefined) { - process.env.COPILOT_CONFIG_DIR = original; - } else { - delete process.env.COPILOT_CONFIG_DIR; - } - } + process.env.COPILOT_CONFIG_DIR = '~/env-path'; + const result = getGlobalDir('copilot', '/explicit/path'); + assert.strictEqual(result, '/explicit/path'); }); test('does not break existing runtimes', () => { @@ -605,46 +592,45 @@ Check ~/.claude/settings and run gsd:health.`; describe('copyCommandsAsCopilotSkills', () => { const srcDir = path.join(__dirname, '..', 'commands', 'gsd'); + let tempDir; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-copilot-skills-')); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); test('creates skill folders from source commands', () => { - const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-copilot-skills-')); - try { - copyCommandsAsCopilotSkills(srcDir, tempDir, 'gsd'); + copyCommandsAsCopilotSkills(srcDir, tempDir, 'gsd'); - // Check specific folders exist - assert.ok(fs.existsSync(path.join(tempDir, 'gsd-health')), 'gsd-health folder exists'); - assert.ok(fs.existsSync(path.join(tempDir, 'gsd-health', 'SKILL.md')), 'gsd-health/SKILL.md exists'); - assert.ok(fs.existsSync(path.join(tempDir, 'gsd-help')), 'gsd-help folder exists'); - assert.ok(fs.existsSync(path.join(tempDir, 'gsd-progress')), 'gsd-progress folder exists'); + // Check specific folders exist + assert.ok(fs.existsSync(path.join(tempDir, 'gsd-health')), 'gsd-health folder exists'); + assert.ok(fs.existsSync(path.join(tempDir, 'gsd-health', 'SKILL.md')), 'gsd-health/SKILL.md exists'); + assert.ok(fs.existsSync(path.join(tempDir, 'gsd-help')), 'gsd-help folder exists'); + assert.ok(fs.existsSync(path.join(tempDir, 'gsd-progress')), 'gsd-progress folder exists'); - // Count gsd-* directories — should match number of source command files - const dirs = fs.readdirSync(tempDir, { withFileTypes: true }) - .filter(e => e.isDirectory() && e.name.startsWith('gsd-')); - const expectedSkillCount = fs.readdirSync(path.join(__dirname, '..', 'commands', 'gsd')) - .filter(f => f.endsWith('.md')).length; - assert.strictEqual(dirs.length, expectedSkillCount, `expected ${expectedSkillCount} skill folders, got ${dirs.length}`); - } finally { - fs.rmSync(tempDir, { recursive: true }); - } + // Count gsd-* directories — should match number of source command files + const dirs = fs.readdirSync(tempDir, { withFileTypes: true }) + .filter(e => e.isDirectory() && e.name.startsWith('gsd-')); + const expectedSkillCount = fs.readdirSync(path.join(__dirname, '..', 'commands', 'gsd')) + .filter(f => f.endsWith('.md')).length; + assert.strictEqual(dirs.length, expectedSkillCount, `expected ${expectedSkillCount} skill folders, got ${dirs.length}`); }); test('skill content has Copilot frontmatter format', () => { - const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-copilot-skills-')); - try { - copyCommandsAsCopilotSkills(srcDir, tempDir, 'gsd'); + copyCommandsAsCopilotSkills(srcDir, tempDir, 'gsd'); - const skillContent = fs.readFileSync(path.join(tempDir, 'gsd-health', 'SKILL.md'), 'utf8'); - // Frontmatter format checks - assert.ok(skillContent.startsWith('---\nname: gsd-health\n'), 'starts with name: gsd-health'); - assert.ok(skillContent.includes('allowed-tools: Read, Bash, Write, AskUserQuestion'), - 'allowed-tools is comma-separated'); - assert.ok(!skillContent.includes('allowed-tools:\n -'), 'NOT YAML multiline format'); - // CONV-06/07 applied - assert.ok(!skillContent.includes('~/.claude/'), 'no ~/.claude/ references'); - assert.ok(!skillContent.match(/gsd:[a-z]/), 'no gsd: command references'); - } finally { - fs.rmSync(tempDir, { recursive: true }); - } + const skillContent = fs.readFileSync(path.join(tempDir, 'gsd-health', 'SKILL.md'), 'utf8'); + // Frontmatter format checks + assert.ok(skillContent.startsWith('---\nname: gsd-health\n'), 'starts with name: gsd-health'); + assert.ok(skillContent.includes('allowed-tools: Read, Bash, Write, AskUserQuestion'), + 'allowed-tools is comma-separated'); + assert.ok(!skillContent.includes('allowed-tools:\n -'), 'NOT YAML multiline format'); + // CONV-06/07 applied + assert.ok(!skillContent.includes('~/.claude/'), 'no ~/.claude/ references'); + assert.ok(!skillContent.match(/gsd:[a-z]/), 'no gsd: command references'); }); test('generates gsd-autonomous skill from autonomous.md command', () => { @@ -652,31 +638,26 @@ describe('copyCommandsAsCopilotSkills', () => { const srcFile = path.join(srcDir, 'autonomous.md'); assert.ok(fs.existsSync(srcFile), 'commands/gsd/autonomous.md must exist as source'); - const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-copilot-skills-')); - try { - copyCommandsAsCopilotSkills(srcDir, tempDir, 'gsd'); + copyCommandsAsCopilotSkills(srcDir, tempDir, 'gsd'); - // Skill folder and file created - assert.ok(fs.existsSync(path.join(tempDir, 'gsd-autonomous')), 'gsd-autonomous folder exists'); - assert.ok(fs.existsSync(path.join(tempDir, 'gsd-autonomous', 'SKILL.md')), 'gsd-autonomous/SKILL.md exists'); + // Skill folder and file created + assert.ok(fs.existsSync(path.join(tempDir, 'gsd-autonomous')), 'gsd-autonomous folder exists'); + assert.ok(fs.existsSync(path.join(tempDir, 'gsd-autonomous', 'SKILL.md')), 'gsd-autonomous/SKILL.md exists'); - const skillContent = fs.readFileSync(path.join(tempDir, 'gsd-autonomous', 'SKILL.md'), 'utf8'); + const skillContent = fs.readFileSync(path.join(tempDir, 'gsd-autonomous', 'SKILL.md'), 'utf8'); - // Frontmatter: name converted from gsd:autonomous to gsd-autonomous - assert.ok(skillContent.startsWith('---\nname: gsd-autonomous\n'), 'name is gsd-autonomous'); - assert.ok(skillContent.includes('description: Run all remaining phases autonomously'), - 'description preserved'); - // argument-hint present and double-quoted - assert.ok(skillContent.includes('argument-hint: "[--from N]"'), 'argument-hint present and quoted'); - // allowed-tools comma-separated - assert.ok(skillContent.includes('allowed-tools: Read, Write, Bash, Glob, Grep, AskUserQuestion, Task'), - 'allowed-tools is comma-separated'); - // No Claude-format remnants - assert.ok(!skillContent.includes('allowed-tools:\n -'), 'NOT YAML multiline format'); - assert.ok(!skillContent.includes('~/.claude/'), 'no ~/.claude/ references in body'); - } finally { - fs.rmSync(tempDir, { recursive: true }); - } + // Frontmatter: name converted from gsd:autonomous to gsd-autonomous + assert.ok(skillContent.startsWith('---\nname: gsd-autonomous\n'), 'name is gsd-autonomous'); + assert.ok(skillContent.includes('description: Run all remaining phases autonomously'), + 'description preserved'); + // argument-hint present and double-quoted + assert.ok(skillContent.includes('argument-hint: "[--from N]"'), 'argument-hint present and quoted'); + // allowed-tools comma-separated + assert.ok(skillContent.includes('allowed-tools: Read, Write, Bash, Glob, Grep, AskUserQuestion, Task'), + 'allowed-tools is comma-separated'); + // No Claude-format remnants + assert.ok(!skillContent.includes('allowed-tools:\n -'), 'NOT YAML multiline format'); + assert.ok(!skillContent.includes('~/.claude/'), 'no ~/.claude/ references in body'); }); test('autonomous skill body converts gsd: to gsd- (CONV-07)', () => { @@ -697,21 +678,16 @@ describe('copyCommandsAsCopilotSkills', () => { }); test('cleans up old skill directories on re-run', () => { - const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-copilot-skills-')); - try { - // Create a fake old directory - fs.mkdirSync(path.join(tempDir, 'gsd-fake-old'), { recursive: true }); - fs.writeFileSync(path.join(tempDir, 'gsd-fake-old', 'SKILL.md'), 'old'); - assert.ok(fs.existsSync(path.join(tempDir, 'gsd-fake-old')), 'fake old dir exists before'); + // Create a fake old directory + fs.mkdirSync(path.join(tempDir, 'gsd-fake-old'), { recursive: true }); + fs.writeFileSync(path.join(tempDir, 'gsd-fake-old', 'SKILL.md'), 'old'); + assert.ok(fs.existsSync(path.join(tempDir, 'gsd-fake-old')), 'fake old dir exists before'); - // Run copy — should clean up old dirs - copyCommandsAsCopilotSkills(srcDir, tempDir, 'gsd'); + // Run copy — should clean up old dirs + copyCommandsAsCopilotSkills(srcDir, tempDir, 'gsd'); - assert.ok(!fs.existsSync(path.join(tempDir, 'gsd-fake-old')), 'fake old dir removed'); - assert.ok(fs.existsSync(path.join(tempDir, 'gsd-health')), 'real dirs still exist'); - } finally { - fs.rmSync(tempDir, { recursive: true }); - } + assert.ok(!fs.existsSync(path.join(tempDir, 'gsd-fake-old')), 'fake old dir removed'); + assert.ok(fs.existsSync(path.join(tempDir, 'gsd-health')), 'real dirs still exist'); }); }); @@ -1058,55 +1034,52 @@ describe('Copilot manifest and patches fixes', () => { assert.ok(data.files[skillKey].length === 64, 'hash is SHA-256 (64 hex chars)'); }); - test('reportLocalPatches shows /gsd-reapply-patches for Copilot', () => { - // Create patches directory with metadata - const patchesDir = path.join(tmpDir, 'gsd-local-patches'); - fs.mkdirSync(patchesDir, { recursive: true }); - fs.writeFileSync(path.join(patchesDir, 'backup-meta.json'), JSON.stringify({ - from_version: '1.0', - files: ['skills/gsd-test/SKILL.md'] - })); + describe('reportLocalPatches', () => { + let originalLog; + let logs; - // Capture console output - const logs = []; - const originalLog = console.log; - console.log = (...args) => logs.push(args.join(' ')); + beforeEach(() => { + originalLog = console.log; + logs = []; + console.log = (...args) => logs.push(args.join(' ')); + }); + + afterEach(() => { + console.log = originalLog; + }); + + test('reportLocalPatches shows /gsd-reapply-patches for Copilot', () => { + // Create patches directory with metadata + const patchesDir = path.join(tmpDir, 'gsd-local-patches'); + fs.mkdirSync(patchesDir, { recursive: true }); + fs.writeFileSync(path.join(patchesDir, 'backup-meta.json'), JSON.stringify({ + from_version: '1.0', + files: ['skills/gsd-test/SKILL.md'] + })); - try { const result = reportLocalPatches(tmpDir, 'copilot'); assert.ok(result.length > 0, 'returns patched files list'); const output = logs.join('\n'); assert.ok(output.includes('/gsd-reapply-patches'), 'uses dash format for Copilot'); assert.ok(!output.includes('/gsd:reapply-patches'), 'does not use colon format'); - } finally { - console.log = originalLog; - } - }); + }); - test('reportLocalPatches shows /gsd:reapply-patches for Claude (unchanged)', () => { - // Create patches directory with metadata - const patchesDir = path.join(tmpDir, 'gsd-local-patches'); - fs.mkdirSync(patchesDir, { recursive: true }); - fs.writeFileSync(path.join(patchesDir, 'backup-meta.json'), JSON.stringify({ - from_version: '1.0', - files: ['get-shit-done/bin/verify.cjs'] - })); + test('reportLocalPatches shows /gsd:reapply-patches for Claude (unchanged)', () => { + // Create patches directory with metadata + const patchesDir = path.join(tmpDir, 'gsd-local-patches'); + fs.mkdirSync(patchesDir, { recursive: true }); + fs.writeFileSync(path.join(patchesDir, 'backup-meta.json'), JSON.stringify({ + from_version: '1.0', + files: ['get-shit-done/bin/verify.cjs'] + })); - // Capture console output - const logs = []; - const originalLog = console.log; - console.log = (...args) => logs.push(args.join(' ')); - - try { const result = reportLocalPatches(tmpDir, 'claude'); assert.ok(result.length > 0, 'returns patched files list'); const output = logs.join('\n'); assert.ok(output.includes('/gsd:reapply-patches'), 'uses colon format for Claude'); - } finally { - console.log = originalLog; - } + }); }); }); @@ -1329,11 +1302,19 @@ describe('E2E: Copilot uninstall verification', () => { } }); - test('preserves non-GSD content in skills directory', () => { - // Standalone lifecycle: install → add custom content → uninstall → verify - const td = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-e2e-preserve-skill-')); - try { + describe('preserves non-GSD content', () => { + let td; + + beforeEach(() => { + td = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-e2e-preserve-')); runCopilotInstall(td); + }); + + afterEach(() => { + fs.rmSync(td, { recursive: true, force: true }); + }); + + test('preserves non-GSD content in skills directory', () => { // Add non-GSD custom skill const customSkillDir = path.join(td, '.github', 'skills', 'my-custom-skill'); fs.mkdirSync(customSkillDir, { recursive: true }); @@ -1343,16 +1324,9 @@ describe('E2E: Copilot uninstall verification', () => { // Verify custom content preserved assert.ok(fs.existsSync(path.join(customSkillDir, 'SKILL.md')), 'Non-GSD skill directory and SKILL.md should be preserved after uninstall'); - } finally { - fs.rmSync(td, { recursive: true, force: true }); - } - }); + }); - test('preserves non-GSD content in agents directory', () => { - // Standalone lifecycle: install → add custom content → uninstall → verify - const td = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-e2e-preserve-agent-')); - try { - runCopilotInstall(td); + test('preserves non-GSD content in agents directory', () => { // Add non-GSD custom agent const customAgentPath = path.join(td, '.github', 'agents', 'my-agent.md'); fs.writeFileSync(customAgentPath, '# My Custom Agent\n'); @@ -1361,8 +1335,6 @@ describe('E2E: Copilot uninstall verification', () => { // Verify custom content preserved assert.ok(fs.existsSync(customAgentPath), 'Non-GSD agent file should be preserved after uninstall'); - } finally { - fs.rmSync(td, { recursive: true, force: true }); - } + }); }); }); diff --git a/tests/core.test.cjs b/tests/core.test.cjs index c4a234a9b..c9b747318 100644 --- a/tests/core.test.cjs +++ b/tests/core.test.cjs @@ -1017,25 +1017,24 @@ describe('stale hook path', () => { describe('resolveWorktreeRoot', () => { const { resolveWorktreeRoot } = require('../get-shit-done/bin/lib/core.cjs'); + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); test('returns cwd when not in a git repo', () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-wt-test-')); - try { - assert.strictEqual(resolveWorktreeRoot(tmpDir), tmpDir); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } + assert.strictEqual(resolveWorktreeRoot(tmpDir), tmpDir); }); test('returns cwd in a normal git repo (not a worktree)', () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-wt-test-')); - try { - const { execSync } = require('child_process'); - execSync('git init', { cwd: tmpDir, stdio: 'pipe' }); - assert.strictEqual(resolveWorktreeRoot(tmpDir), tmpDir); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } + const { execSync: execSyncLocal } = require('child_process'); + execSyncLocal('git init', { cwd: tmpDir, stdio: 'pipe' }); + assert.strictEqual(resolveWorktreeRoot(tmpDir), tmpDir); }); }); @@ -1043,78 +1042,70 @@ describe('resolveWorktreeRoot', () => { describe('resolveWorktreeRoot with linked worktree .planning/', () => { const { resolveWorktreeRoot } = require('../get-shit-done/bin/lib/core.cjs'); - const { execSync } = require('child_process'); + const { execSync: execSyncLocal } = require('child_process'); // On Windows CI, os.tmpdir() may return 8.3 short paths (RUNNER~1) while // git returns long paths (runneradmin). realpathSync.native resolves both. const normalizePath = (p) => { try { return fs.realpathSync.native(p); } catch { return fs.realpathSync(p); } }; - test('returns linked worktree cwd when it has its own .planning/', () => { - const mainDir = normalizePath(fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-wt-main-'))); - let worktreeDir; - try { - // Set up main repo with a commit - execSync('git init', { cwd: mainDir, stdio: 'pipe' }); - execSync('git config user.email "test@test.com"', { cwd: mainDir, stdio: 'pipe' }); - execSync('git config user.name "Test"', { cwd: mainDir, stdio: 'pipe' }); - execSync('git config commit.gpgsign false', { cwd: mainDir, stdio: 'pipe' }); - fs.mkdirSync(path.join(mainDir, '.planning'), { recursive: true }); - fs.writeFileSync(path.join(mainDir, 'README.md'), '# Main'); - execSync('git add -A', { cwd: mainDir, stdio: 'pipe' }); - execSync('git commit -m "initial"', { cwd: mainDir, stdio: 'pipe' }); + let mainDir; + let worktreeDir; - // Create a linked worktree - worktreeDir = normalizePath(fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-wt-linked-'))); - fs.rmSync(worktreeDir, { recursive: true, force: true }); - execSync(`git worktree add "${worktreeDir}" -b test-linked`, { cwd: mainDir, stdio: 'pipe' }); + function initBareGitRepo() { + const dir = normalizePath(fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-wt-main-'))); + execSyncLocal('git init', { cwd: dir, stdio: 'pipe' }); + execSyncLocal('git config user.email "test@test.com"', { cwd: dir, stdio: 'pipe' }); + execSyncLocal('git config user.name "Test"', { cwd: dir, stdio: 'pipe' }); + execSyncLocal('git config commit.gpgsign false', { cwd: dir, stdio: 'pipe' }); + fs.writeFileSync(path.join(dir, 'README.md'), '# Main'); + execSyncLocal('git add -A', { cwd: dir, stdio: 'pipe' }); + execSyncLocal('git commit -m "initial"', { cwd: dir, stdio: 'pipe' }); + return dir; + } - // Give the linked worktree its own .planning/ - fs.mkdirSync(path.join(worktreeDir, '.planning'), { recursive: true }); + beforeEach(() => { + mainDir = initBareGitRepo(); + worktreeDir = null; + }); - // resolveWorktreeRoot should return the linked worktree dir, not the main repo - const result = normalizePath(resolveWorktreeRoot(worktreeDir)); - assert.strictEqual(result, worktreeDir, - 'linked worktree with .planning/ should resolve to itself, not the main repo'); - } finally { - if (worktreeDir) { - try { execSync(`git worktree remove "${worktreeDir}" --force`, { cwd: mainDir, stdio: 'pipe' }); } catch { /* ok */ } - try { fs.rmSync(worktreeDir, { recursive: true, force: true }); } catch { /* ok */ } - } - fs.rmSync(mainDir, { recursive: true, force: true }); + afterEach(() => { + if (worktreeDir) { + try { execSyncLocal(`git worktree remove "${worktreeDir}" --force`, { cwd: mainDir, stdio: 'pipe' }); } catch { /* ok */ } + try { fs.rmSync(worktreeDir, { recursive: true, force: true }); } catch { /* ok */ } } + cleanup(mainDir); + }); + + test('returns linked worktree cwd when it has its own .planning/', () => { + // Add .planning/ to main repo + fs.mkdirSync(path.join(mainDir, '.planning'), { recursive: true }); + + // Create a linked worktree + worktreeDir = normalizePath(fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-wt-linked-'))); + fs.rmSync(worktreeDir, { recursive: true, force: true }); + execSyncLocal(`git worktree add "${worktreeDir}" -b test-linked`, { cwd: mainDir, stdio: 'pipe' }); + + // Give the linked worktree its own .planning/ + fs.mkdirSync(path.join(worktreeDir, '.planning'), { recursive: true }); + + // resolveWorktreeRoot should return the linked worktree dir, not the main repo + const result = normalizePath(resolveWorktreeRoot(worktreeDir)); + assert.strictEqual(result, worktreeDir, + 'linked worktree with .planning/ should resolve to itself, not the main repo'); }); test('returns main repo root when linked worktree has no .planning/', () => { - const mainDir = normalizePath(fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-wt-main-'))); - let worktreeDir; - try { - // Set up main repo with a commit - execSync('git init', { cwd: mainDir, stdio: 'pipe' }); - execSync('git config user.email "test@test.com"', { cwd: mainDir, stdio: 'pipe' }); - execSync('git config user.name "Test"', { cwd: mainDir, stdio: 'pipe' }); - execSync('git config commit.gpgsign false', { cwd: mainDir, stdio: 'pipe' }); - fs.writeFileSync(path.join(mainDir, 'README.md'), '# Main'); - execSync('git add -A', { cwd: mainDir, stdio: 'pipe' }); - execSync('git commit -m "initial"', { cwd: mainDir, stdio: 'pipe' }); + // Create a linked worktree (no .planning/ in main or worktree) + worktreeDir = normalizePath(fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-wt-linked-'))); + fs.rmSync(worktreeDir, { recursive: true, force: true }); + execSyncLocal(`git worktree add "${worktreeDir}" -b test-linked-no-plan`, { cwd: mainDir, stdio: 'pipe' }); - // Create a linked worktree (no .planning/) - worktreeDir = normalizePath(fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-wt-linked-'))); - fs.rmSync(worktreeDir, { recursive: true, force: true }); - execSync(`git worktree add "${worktreeDir}" -b test-linked-no-plan`, { cwd: mainDir, stdio: 'pipe' }); - - // resolveWorktreeRoot should return the main repo root - const result = normalizePath(resolveWorktreeRoot(worktreeDir)); - const expected = normalizePath(mainDir); - assert.strictEqual(result, expected, - 'linked worktree without .planning/ should resolve to main repo root'); - } finally { - if (worktreeDir) { - try { execSync(`git worktree remove "${worktreeDir}" --force`, { cwd: mainDir, stdio: 'pipe' }); } catch { /* ok */ } - try { fs.rmSync(worktreeDir, { recursive: true, force: true }); } catch { /* ok */ } - } - fs.rmSync(mainDir, { recursive: true, force: true }); - } + // resolveWorktreeRoot should return the main repo root + const result = normalizePath(resolveWorktreeRoot(worktreeDir)); + const expected = normalizePath(mainDir); + assert.strictEqual(result, expected, + 'linked worktree without .planning/ should resolve to main repo root'); }); }); @@ -1122,37 +1113,36 @@ describe('resolveWorktreeRoot with linked worktree .planning/', () => { describe('monorepo worktree CWD preservation', () => { const { resolveWorktreeRoot } = require('../get-shit-done/bin/lib/core.cjs'); + let tmpDir; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-monorepo-wt-')); + }); + + afterEach(() => { + cleanup(tmpDir); + }); test('CWD with .planning/ skips worktree resolution (monorepo subdirectory)', () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-monorepo-wt-')); const subDir = path.join(tmpDir, 'service-alpha'); fs.mkdirSync(path.join(subDir, '.planning'), { recursive: true }); - try { - let cwd = subDir; - if (!fs.existsSync(path.join(cwd, '.planning'))) { - const worktreeRoot = resolveWorktreeRoot(cwd); - if (worktreeRoot !== cwd) cwd = worktreeRoot; - } - assert.strictEqual(cwd, subDir, 'CWD with .planning/ must not be overridden by worktree resolution'); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + let cwd = subDir; + if (!fs.existsSync(path.join(cwd, '.planning'))) { + const worktreeRoot = resolveWorktreeRoot(cwd); + if (worktreeRoot !== cwd) cwd = worktreeRoot; } + assert.strictEqual(cwd, subDir, 'CWD with .planning/ must not be overridden by worktree resolution'); }); test('CWD without .planning/ still goes through worktree resolution', () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-monorepo-wt-')); - try { - let cwd = tmpDir; - let worktreeResolutionCalled = false; - if (!fs.existsSync(path.join(cwd, '.planning'))) { - worktreeResolutionCalled = true; - const worktreeRoot = resolveWorktreeRoot(cwd); - if (worktreeRoot !== cwd) cwd = worktreeRoot; - } - assert.ok(worktreeResolutionCalled, 'worktree resolution must be called when .planning/ is absent'); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + let cwd = tmpDir; + let worktreeResolutionCalled = false; + if (!fs.existsSync(path.join(cwd, '.planning'))) { + worktreeResolutionCalled = true; + const worktreeRoot = resolveWorktreeRoot(cwd); + if (worktreeRoot !== cwd) cwd = worktreeRoot; } + assert.ok(worktreeResolutionCalled, 'worktree resolution must be called when .planning/ is absent'); }); }); @@ -1160,50 +1150,40 @@ describe('monorepo worktree CWD preservation', () => { describe('withPlanningLock', () => { const { withPlanningLock, planningDir } = require('../get-shit-done/bin/lib/core.cjs'); + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); test('executes function and returns result', () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-lock-test-')); - fs.mkdirSync(path.join(tmpDir, '.planning'), { recursive: true }); - try { - const result = withPlanningLock(tmpDir, () => 42); - assert.strictEqual(result, 42); - // Lock file should be cleaned up - assert.ok(!fs.existsSync(path.join(planningDir(tmpDir), '.lock'))); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } + const result = withPlanningLock(tmpDir, () => 42); + assert.strictEqual(result, 42); + // Lock file should be cleaned up + assert.ok(!fs.existsSync(path.join(planningDir(tmpDir), '.lock'))); }); test('cleans up lock file even on error', () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-lock-test-')); - fs.mkdirSync(path.join(tmpDir, '.planning'), { recursive: true }); - try { - assert.throws(() => { - withPlanningLock(tmpDir, () => { throw new Error('test'); }); - }, /test/); - assert.ok(!fs.existsSync(path.join(planningDir(tmpDir), '.lock'))); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } + assert.throws(() => { + withPlanningLock(tmpDir, () => { throw new Error('test'); }); + }, /test/); + assert.ok(!fs.existsSync(path.join(planningDir(tmpDir), '.lock'))); }); test('recovers from stale lock (>30s old)', () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-lock-test-')); - const planDir = path.join(tmpDir, '.planning'); - fs.mkdirSync(planDir, { recursive: true }); - const lockPath = path.join(planDir, '.lock'); - try { - // Create a stale lock - fs.writeFileSync(lockPath, '{"pid":99999}'); - // Backdate the lock file by 31 seconds - const staleTime = new Date(Date.now() - 31000); - fs.utimesSync(lockPath, staleTime, staleTime); + const lockPath = path.join(tmpDir, '.planning', '.lock'); + // Create a stale lock + fs.writeFileSync(lockPath, '{"pid":99999}'); + // Backdate the lock file by 31 seconds + const staleTime = new Date(Date.now() - 31000); + fs.utimesSync(lockPath, staleTime, staleTime); - const result = withPlanningLock(tmpDir, () => 'recovered'); - assert.strictEqual(result, 'recovered'); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } + const result = withPlanningLock(tmpDir, () => 'recovered'); + assert.strictEqual(result, 'recovered'); }); }); diff --git a/tests/forensics.test.cjs b/tests/forensics.test.cjs index 4cbeb22e7..cf7a780bc 100644 --- a/tests/forensics.test.cjs +++ b/tests/forensics.test.cjs @@ -5,7 +5,7 @@ * follow expected patterns, and cover all anomaly detection types. */ -const { test, describe } = require('node:test'); +const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert'); const fs = require('fs'); const path = require('path'); @@ -188,71 +188,56 @@ describe('forensics report structure', () => { describe('forensics fixture-based tests', () => { let tmpDir; - function setup() { + beforeEach(() => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-forensics-test-')); - } + }); - function teardown() { + afterEach(() => { if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); - } + }); test('detects missing artifacts in phase structure', () => { - setup(); - try { - // Phase 1: complete - const phase1 = path.join(tmpDir, '.planning', 'phases', '01-setup'); - fs.mkdirSync(phase1, { recursive: true }); - fs.writeFileSync(path.join(phase1, '01-PLAN-A.md'), 'plan'); - fs.writeFileSync(path.join(phase1, '01-SUMMARY.md'), 'summary'); - fs.writeFileSync(path.join(phase1, '01-VERIFICATION.md'), 'verification'); + // Phase 1: complete + const phase1 = path.join(tmpDir, '.planning', 'phases', '01-setup'); + fs.mkdirSync(phase1, { recursive: true }); + fs.writeFileSync(path.join(phase1, '01-PLAN-A.md'), 'plan'); + fs.writeFileSync(path.join(phase1, '01-SUMMARY.md'), 'summary'); + fs.writeFileSync(path.join(phase1, '01-VERIFICATION.md'), 'verification'); - // Phase 2: missing SUMMARY and VERIFICATION (anomaly) - const phase2 = path.join(tmpDir, '.planning', 'phases', '02-core'); - fs.mkdirSync(phase2, { recursive: true }); - fs.writeFileSync(path.join(phase2, '02-PLAN-A.md'), 'plan'); + // Phase 2: missing SUMMARY and VERIFICATION (anomaly) + const phase2 = path.join(tmpDir, '.planning', 'phases', '02-core'); + fs.mkdirSync(phase2, { recursive: true }); + fs.writeFileSync(path.join(phase2, '02-PLAN-A.md'), 'plan'); - // Verify detection - const p1Files = fs.readdirSync(phase1); - const p2Files = fs.readdirSync(phase2); + // Verify detection + const p1Files = fs.readdirSync(phase1); + const p2Files = fs.readdirSync(phase2); - assert.ok(p1Files.some(f => f.includes('SUMMARY')), 'phase 1 has SUMMARY'); - assert.ok(p1Files.some(f => f.includes('VERIFICATION')), 'phase 1 has VERIFICATION'); - assert.ok(!p2Files.some(f => f.includes('SUMMARY')), 'phase 2 missing SUMMARY (anomaly)'); - assert.ok(!p2Files.some(f => f.includes('VERIFICATION')), 'phase 2 missing VERIFICATION (anomaly)'); - } finally { - teardown(); - } + assert.ok(p1Files.some(f => f.includes('SUMMARY')), 'phase 1 has SUMMARY'); + assert.ok(p1Files.some(f => f.includes('VERIFICATION')), 'phase 1 has VERIFICATION'); + assert.ok(!p2Files.some(f => f.includes('SUMMARY')), 'phase 2 missing SUMMARY (anomaly)'); + assert.ok(!p2Files.some(f => f.includes('VERIFICATION')), 'phase 2 missing VERIFICATION (anomaly)'); }); test('forensics report directory can be created', () => { - setup(); - try { - const forensicsDir = path.join(tmpDir, '.planning', 'forensics'); - fs.mkdirSync(forensicsDir, { recursive: true }); - const reportPath = path.join(forensicsDir, 'report-20260321-150000.md'); - fs.writeFileSync(reportPath, '# Forensic Report\n'); + const forensicsDir = path.join(tmpDir, '.planning', 'forensics'); + fs.mkdirSync(forensicsDir, { recursive: true }); + const reportPath = path.join(forensicsDir, 'report-20260321-150000.md'); + fs.writeFileSync(reportPath, '# Forensic Report\n'); - assert.ok(fs.existsSync(reportPath), 'report file should be created'); - const content = fs.readFileSync(reportPath, 'utf-8'); - assert.ok(content.includes('Forensic Report'), 'report should have header'); - } finally { - teardown(); - } + assert.ok(fs.existsSync(reportPath), 'report file should be created'); + const content = fs.readFileSync(reportPath, 'utf-8'); + assert.ok(content.includes('Forensic Report'), 'report should have header'); }); test('handles project with no .planning directory', () => { - setup(); - try { - // No .planning/ at all - const planningExists = fs.existsSync(path.join(tmpDir, '.planning')); - assert.strictEqual(planningExists, false, 'no .planning/ should exist'); + // No .planning/ at all + const planningExists = fs.existsSync(path.join(tmpDir, '.planning')); + assert.strictEqual(planningExists, false, 'no .planning/ should exist'); - // Forensics should still work with git data - const forensicsDir = path.join(tmpDir, '.planning', 'forensics'); - fs.mkdirSync(forensicsDir, { recursive: true }); - assert.ok(fs.existsSync(forensicsDir), 'forensics dir created on demand'); - } finally { - teardown(); - } + // Forensics should still work with git data + const forensicsDir = path.join(tmpDir, '.planning', 'forensics'); + fs.mkdirSync(forensicsDir, { recursive: true }); + assert.ok(fs.existsSync(forensicsDir), 'forensics dir created on demand'); }); }); diff --git a/tests/helpers.cjs b/tests/helpers.cjs index 91ed3169f..a61ec1653 100644 --- a/tests/helpers.cjs +++ b/tests/helpers.cjs @@ -47,16 +47,21 @@ function runGsdTools(args, cwd = process.cwd(), env = {}) { } } +// Create a bare temp directory (no .planning/ structure) +function createTempDir(prefix = 'gsd-test-') { + return fs.mkdtempSync(path.join(require('os').tmpdir(), prefix)); +} + // Create temp directory structure -function createTempProject() { - const tmpDir = fs.mkdtempSync(path.join(require('os').tmpdir(), 'gsd-test-')); +function createTempProject(prefix = 'gsd-test-') { + const tmpDir = fs.mkdtempSync(path.join(require('os').tmpdir(), prefix)); fs.mkdirSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true }); return tmpDir; } // Create temp directory with initialized git repo and at least one commit -function createTempGitProject() { - const tmpDir = fs.mkdtempSync(path.join(require('os').tmpdir(), 'gsd-test-')); +function createTempGitProject(prefix = 'gsd-test-') { + const tmpDir = fs.mkdtempSync(path.join(require('os').tmpdir(), prefix)); fs.mkdirSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true }); execSync('git init', { cwd: tmpDir, stdio: 'pipe' }); @@ -79,4 +84,4 @@ function cleanup(tmpDir) { fs.rmSync(tmpDir, { recursive: true, force: true }); } -module.exports = { runGsdTools, createTempProject, createTempGitProject, cleanup, TOOLS_PATH }; +module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, TOOLS_PATH }; diff --git a/tests/milestone-summary.test.cjs b/tests/milestone-summary.test.cjs index 327f1023c..fd4508ecf 100644 --- a/tests/milestone-summary.test.cjs +++ b/tests/milestone-summary.test.cjs @@ -5,7 +5,7 @@ * and follow expected patterns. Tests artifact discovery logic. */ -const { test, describe } = require('node:test'); +const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert'); const fs = require('fs'); const path = require('path'); @@ -195,100 +195,85 @@ describe('milestone-summary fixture-based artifact discovery', () => { const os = require('os'); let tmpDir; - function setup() { + beforeEach(() => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-ms-test-')); - } + }); - function teardown() { + afterEach(() => { if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); - } + }); test('discovers artifacts in archived milestone structure', () => { - setup(); - try { - // Create archived milestone structure - const milestonesDir = path.join(tmpDir, '.planning', 'milestones'); - fs.mkdirSync(milestonesDir, { recursive: true }); - fs.writeFileSync(path.join(milestonesDir, 'v1.0-ROADMAP.md'), '# Roadmap v1.0'); - fs.writeFileSync(path.join(milestonesDir, 'v1.0-REQUIREMENTS.md'), '# Reqs v1.0'); - fs.writeFileSync(path.join(milestonesDir, 'v1.0-MILESTONE-AUDIT.md'), '# Audit v1.0'); + // Create archived milestone structure + const milestonesDir = path.join(tmpDir, '.planning', 'milestones'); + fs.mkdirSync(milestonesDir, { recursive: true }); + fs.writeFileSync(path.join(milestonesDir, 'v1.0-ROADMAP.md'), '# Roadmap v1.0'); + fs.writeFileSync(path.join(milestonesDir, 'v1.0-REQUIREMENTS.md'), '# Reqs v1.0'); + fs.writeFileSync(path.join(milestonesDir, 'v1.0-MILESTONE-AUDIT.md'), '# Audit v1.0'); - // Verify all 3 archived files are discoverable - const files = fs.readdirSync(milestonesDir); - assert.ok(files.includes('v1.0-ROADMAP.md'), 'archived ROADMAP should exist'); - assert.ok(files.includes('v1.0-REQUIREMENTS.md'), 'archived REQUIREMENTS should exist'); - assert.ok(files.includes('v1.0-MILESTONE-AUDIT.md'), 'archived AUDIT should exist'); - } finally { - teardown(); - } + // Verify all 3 archived files are discoverable + const files = fs.readdirSync(milestonesDir); + assert.ok(files.includes('v1.0-ROADMAP.md'), 'archived ROADMAP should exist'); + assert.ok(files.includes('v1.0-REQUIREMENTS.md'), 'archived REQUIREMENTS should exist'); + assert.ok(files.includes('v1.0-MILESTONE-AUDIT.md'), 'archived AUDIT should exist'); }); test('discovers phase artifacts across multiple phases', () => { - setup(); - try { - // Create phase structure with varying artifact completeness - const phase1 = path.join(tmpDir, '.planning', 'phases', '01-setup'); - const phase2 = path.join(tmpDir, '.planning', 'phases', '02-core'); - const phase3 = path.join(tmpDir, '.planning', 'phases', '03-ui'); - fs.mkdirSync(phase1, { recursive: true }); - fs.mkdirSync(phase2, { recursive: true }); - fs.mkdirSync(phase3, { recursive: true }); + // Create phase structure with varying artifact completeness + const phase1 = path.join(tmpDir, '.planning', 'phases', '01-setup'); + const phase2 = path.join(tmpDir, '.planning', 'phases', '02-core'); + const phase3 = path.join(tmpDir, '.planning', 'phases', '03-ui'); + fs.mkdirSync(phase1, { recursive: true }); + fs.mkdirSync(phase2, { recursive: true }); + fs.mkdirSync(phase3, { recursive: true }); - // Phase 1: all artifacts - fs.writeFileSync(path.join(phase1, '01-SUMMARY.md'), 'one_liner: Setup'); - fs.writeFileSync(path.join(phase1, '01-CONTEXT.md'), 'D-01'); - fs.writeFileSync(path.join(phase1, '01-VERIFICATION.md'), 'status: passed'); - fs.writeFileSync(path.join(phase1, '01-RESEARCH.md'), '# Research'); + // Phase 1: all artifacts + fs.writeFileSync(path.join(phase1, '01-SUMMARY.md'), 'one_liner: Setup'); + fs.writeFileSync(path.join(phase1, '01-CONTEXT.md'), 'D-01'); + fs.writeFileSync(path.join(phase1, '01-VERIFICATION.md'), 'status: passed'); + fs.writeFileSync(path.join(phase1, '01-RESEARCH.md'), '# Research'); - // Phase 2: partial artifacts (no RESEARCH, no VERIFICATION) - fs.writeFileSync(path.join(phase2, '02-SUMMARY.md'), 'one_liner: Core'); - fs.writeFileSync(path.join(phase2, '02-CONTEXT.md'), 'D-02'); + // Phase 2: partial artifacts (no RESEARCH, no VERIFICATION) + fs.writeFileSync(path.join(phase2, '02-SUMMARY.md'), 'one_liner: Core'); + fs.writeFileSync(path.join(phase2, '02-CONTEXT.md'), 'D-02'); - // Phase 3: only SUMMARY - fs.writeFileSync(path.join(phase3, '03-SUMMARY.md'), 'one_liner: UI'); + // Phase 3: only SUMMARY + fs.writeFileSync(path.join(phase3, '03-SUMMARY.md'), 'one_liner: UI'); - // Verify discovery - const phasesDir = path.join(tmpDir, '.planning', 'phases'); - const phaseDirs = fs.readdirSync(phasesDir, { withFileTypes: true }) - .filter(e => e.isDirectory()) - .map(e => e.name); - assert.strictEqual(phaseDirs.length, 3, 'should find 3 phase directories'); + // Verify discovery + const phasesDir = path.join(tmpDir, '.planning', 'phases'); + const phaseDirs = fs.readdirSync(phasesDir, { withFileTypes: true }) + .filter(e => e.isDirectory()) + .map(e => e.name); + assert.strictEqual(phaseDirs.length, 3, 'should find 3 phase directories'); - // Phase 1 has all 4 artifact types - const p1Files = fs.readdirSync(phase1); - assert.strictEqual(p1Files.length, 4, 'phase 1 should have 4 artifacts'); + // Phase 1 has all 4 artifact types + const p1Files = fs.readdirSync(phase1); + assert.strictEqual(p1Files.length, 4, 'phase 1 should have 4 artifacts'); - // Phase 2 has 2 artifact types - const p2Files = fs.readdirSync(phase2); - assert.strictEqual(p2Files.length, 2, 'phase 2 should have 2 artifacts'); + // Phase 2 has 2 artifact types + const p2Files = fs.readdirSync(phase2); + assert.strictEqual(p2Files.length, 2, 'phase 2 should have 2 artifacts'); - // Phase 3 has 1 artifact type - const p3Files = fs.readdirSync(phase3); - assert.strictEqual(p3Files.length, 1, 'phase 3 should have 1 artifact'); - } finally { - teardown(); - } + // Phase 3 has 1 artifact type + const p3Files = fs.readdirSync(phase3); + assert.strictEqual(p3Files.length, 1, 'phase 3 should have 1 artifact'); }); test('handles empty .planning directory without error', () => { - setup(); - try { - const planningDir = path.join(tmpDir, '.planning'); - fs.mkdirSync(planningDir, { recursive: true }); + const planningDir = path.join(tmpDir, '.planning'); + fs.mkdirSync(planningDir, { recursive: true }); - // No milestones, no phases — just empty .planning/ - const contents = fs.readdirSync(planningDir); - assert.strictEqual(contents.length, 0, 'empty .planning/ should have no contents'); + // No milestones, no phases — just empty .planning/ + const contents = fs.readdirSync(planningDir); + assert.strictEqual(contents.length, 0, 'empty .planning/ should have no contents'); - // Should not throw when checking for milestones dir - const milestonesExists = fs.existsSync(path.join(planningDir, 'milestones')); - assert.strictEqual(milestonesExists, false, 'milestones/ should not exist'); + // Should not throw when checking for milestones dir + const milestonesExists = fs.existsSync(path.join(planningDir, 'milestones')); + assert.strictEqual(milestonesExists, false, 'milestones/ should not exist'); - const phasesExists = fs.existsSync(path.join(planningDir, 'phases')); - assert.strictEqual(phasesExists, false, 'phases/ should not exist'); - } finally { - teardown(); - } + const phasesExists = fs.existsSync(path.join(planningDir, 'phases')); + assert.strictEqual(phasesExists, false, 'phases/ should not exist'); }); test('output path pattern produces valid filenames', () => { diff --git a/tests/profile-pipeline.test.cjs b/tests/profile-pipeline.test.cjs index e6e783412..4dab8ac56 100644 --- a/tests/profile-pipeline.test.cjs +++ b/tests/profile-pipeline.test.cjs @@ -10,7 +10,7 @@ const assert = require('node:assert'); const fs = require('fs'); const path = require('path'); const os = require('os'); -const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); +const { runGsdTools, createTempDir, createTempProject, cleanup } = require('./helpers.cjs'); // ─── scan-sessions ──────────────────────────────────────────────────────────── @@ -18,7 +18,7 @@ describe('scan-sessions command', () => { let tmpDir; beforeEach(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-profile-test-')); + tmpDir = createTempDir('gsd-profile-test-'); }); afterEach(() => { @@ -79,7 +79,7 @@ describe('extract-messages command', () => { let tmpDir; beforeEach(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-profile-test-')); + tmpDir = createTempDir('gsd-profile-test-'); }); afterEach(() => { diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 82023c347..721de2871 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -149,27 +149,34 @@ describe('state-snapshot command', () => { assert.strictEqual(output.paused_at, 'Phase 3, Plan 1, Task 2 - mid-implementation', 'paused_at extracted'); }); - test('supports --cwd override when command runs outside project root', () => { - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - `# Session State + describe('--cwd override', () => { + let outsideDir; + + beforeEach(() => { + outsideDir = fs.mkdtempSync(path.join(require('os').tmpdir(), 'gsd-test-outside-')); + }); + + afterEach(() => { + cleanup(outsideDir); + }); + + test('supports --cwd override when command runs outside project root', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# Session State **Current Phase:** 03 **Status:** Ready to plan ` - ); - const outsideDir = fs.mkdtempSync(path.join(require('os').tmpdir(), 'gsd-test-outside-')); + ); - try { const result = runGsdTools(`state-snapshot --cwd "${tmpDir}"`, outsideDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); assert.strictEqual(output.current_phase, '03', 'should read STATE.md from overridden cwd'); assert.strictEqual(output.status, 'Ready to plan', 'should parse status from overridden cwd'); - } finally { - cleanup(outsideDir); - } + }); }); test('returns error for invalid --cwd path', () => { diff --git a/tests/workspace.test.cjs b/tests/workspace.test.cjs index 48ccfe572..aa6180182 100644 --- a/tests/workspace.test.cjs +++ b/tests/workspace.test.cjs @@ -11,7 +11,7 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); const { execSync } = require('child_process'); -const { runGsdTools, cleanup } = require('./helpers.cjs'); +const { runGsdTools, createTempDir, cleanup } = require('./helpers.cjs'); const { detectChildRepos } = require('../get-shit-done/bin/lib/init.cjs'); // ─── detectChildRepos ──────────────────────────────────────────────────────── @@ -20,7 +20,7 @@ describe('detectChildRepos', () => { let tmpDir; beforeEach(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-ws-test-')); + tmpDir = createTempDir('gsd-ws-test-'); }); afterEach(() => { @@ -81,7 +81,7 @@ describe('init new-workspace', () => { let tmpDir; beforeEach(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-ws-test-')); + tmpDir = createTempDir('gsd-ws-test-'); }); afterEach(() => { @@ -125,7 +125,7 @@ describe('init list-workspaces', () => { let tmpDir; beforeEach(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-ws-test-')); + tmpDir = createTempDir('gsd-ws-test-'); }); afterEach(() => { @@ -172,7 +172,7 @@ describe('init remove-workspace', () => { let tmpDir; beforeEach(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-ws-test-')); + tmpDir = createTempDir('gsd-ws-test-'); }); afterEach(() => { @@ -224,7 +224,7 @@ describe('workspace worktree integration', () => { let sourceRepo; beforeEach(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-ws-integ-')); + tmpDir = createTempDir('gsd-ws-integ-'); // Create a source git repo with a commit sourceRepo = path.join(tmpDir, 'source-repo'); fs.mkdirSync(sourceRepo); diff --git a/tests/workstream.test.cjs b/tests/workstream.test.cjs index ea27f5ef3..33c562514 100644 --- a/tests/workstream.test.cjs +++ b/tests/workstream.test.cjs @@ -2,7 +2,7 @@ * Workstream Tests — CRUD, env-var routing, collision detection */ -const { describe, test, before, after } = require('node:test'); +const { describe, test, before, after, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); @@ -162,16 +162,23 @@ describe('workstream list', () => { assert.deepStrictEqual(names, ['alpha', 'beta']); }); - test('reports flat mode when no workstreams exist', () => { - const flatDir = createTempProject(); - try { + describe('flat mode', () => { + let flatDir; + + beforeEach(() => { + flatDir = createTempProject(); + }); + + afterEach(() => { + cleanup(flatDir); + }); + + test('reports flat mode when no workstreams exist', () => { const result = runGsdTools(['workstream', 'list', '--raw'], flatDir); assert.ok(result.success); const data = JSON.parse(result.output); assert.strictEqual(data.mode, 'flat'); - } finally { - cleanup(flatDir); - } + }); }); });