From 63f6424d1b2b683b921f6087ccb03eb05516ff3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diego=20Mari=C3=B1o?= Date: Wed, 18 Mar 2026 15:27:46 +0100 Subject: [PATCH] fix(tests): sandbox HOME in runGsdTools to prevent flaky assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildNewProjectConfig() merges ~/.gsd/defaults.json when present, so tests asserting concrete config values (model_profile, commit_docs, brave_search) would fail on machines with a personal defaults file. - Pass HOME=cwd as env override in runGsdTools — child process resolves os.homedir() to the temp directory, which has no .gsd/ subtree - Update three tests that previously wrote to the real ~/.gsd/ using fragile save/restore logic; they now write to tmpDir/.gsd/ instead, which is cleaned up automatically by afterEach - Remove now-unused `os` import from config.test.cjs --- tests/config.test.cjs | 141 +++++++++++------------------------------- tests/helpers.cjs | 5 ++ 2 files changed, 42 insertions(+), 104 deletions(-) diff --git a/tests/config.test.cjs b/tests/config.test.cjs index 99dacd402..c6fe2590f 100644 --- a/tests/config.test.cjs +++ b/tests/config.test.cjs @@ -11,7 +11,6 @@ const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert'); const fs = require('fs'); const path = require('path'); -const os = require('os'); const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); // ─── helpers ────────────────────────────────────────────────────────────────── @@ -77,122 +76,56 @@ describe('config-ensure-section command', () => { assert.strictEqual(secondOutput.reason, 'already_exists'); }); - // NOTE: This test touches ~/.gsd/ on the real filesystem. It uses save/restore - // try/finally and skips if the file already exists to avoid corrupting user config. test('detects Brave Search from file-based key', () => { - const homedir = os.homedir(); - const gsdDir = path.join(homedir, '.gsd'); - const braveKeyFile = path.join(gsdDir, 'brave_api_key'); + // runGsdTools sandboxes HOME=tmpDir, so brave_api_key is written there — + // no real filesystem side effects, cleanup happens via afterEach. + const gsdDir = path.join(tmpDir, '.gsd'); + fs.mkdirSync(gsdDir, { recursive: true }); + fs.writeFileSync(path.join(gsdDir, 'brave_api_key'), 'test-key', 'utf-8'); - // Skip if file already exists (don't mess with user's real config) - if (fs.existsSync(braveKeyFile)) { - return; - } + const result = runGsdTools('config-ensure-section', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); - // Create .gsd dir and brave_api_key file - const gsdDirExisted = fs.existsSync(gsdDir); - try { - if (!gsdDirExisted) { - fs.mkdirSync(gsdDir, { recursive: true }); - } - fs.writeFileSync(braveKeyFile, 'test-key', 'utf-8'); - - const result = runGsdTools('config-ensure-section', tmpDir); - assert.ok(result.success, `Command failed: ${result.error}`); - - const config = readConfig(tmpDir); - assert.strictEqual(config.brave_search, true); - } finally { - // Clean up - try { fs.unlinkSync(braveKeyFile); } catch { /* ignore */ } - if (!gsdDirExisted) { - try { fs.rmdirSync(gsdDir); } catch { /* ignore if not empty */ } - } - } + const config = readConfig(tmpDir); + assert.strictEqual(config.brave_search, true); }); - // NOTE: This test touches ~/.gsd/ on the real filesystem. It uses save/restore - // try/finally and skips if the file already exists to avoid corrupting user config. test('merges user defaults from defaults.json', () => { - const homedir = os.homedir(); - const gsdDir = path.join(homedir, '.gsd'); - const defaultsFile = path.join(gsdDir, 'defaults.json'); + // runGsdTools sandboxes HOME=tmpDir, so defaults.json is written there — + // no real filesystem side effects, cleanup happens via afterEach. + const gsdDir = path.join(tmpDir, '.gsd'); + fs.mkdirSync(gsdDir, { recursive: true }); + fs.writeFileSync(path.join(gsdDir, 'defaults.json'), JSON.stringify({ + model_profile: 'quality', + commit_docs: false, + }), 'utf-8'); - // Save existing defaults if present - let existingDefaults = null; - const gsdDirExisted = fs.existsSync(gsdDir); - if (fs.existsSync(defaultsFile)) { - existingDefaults = fs.readFileSync(defaultsFile, 'utf-8'); - } + const result = runGsdTools('config-ensure-section', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); - try { - if (!gsdDirExisted) { - fs.mkdirSync(gsdDir, { recursive: true }); - } - fs.writeFileSync(defaultsFile, JSON.stringify({ - model_profile: 'quality', - commit_docs: false, - }), 'utf-8'); - - const result = runGsdTools('config-ensure-section', tmpDir); - assert.ok(result.success, `Command failed: ${result.error}`); - - const config = readConfig(tmpDir); - assert.strictEqual(config.model_profile, 'quality', 'model_profile should be overridden'); - assert.strictEqual(config.commit_docs, false, 'commit_docs should be overridden'); - assert.ok(config.git && typeof config.git === 'object', 'git should be an object'); - assert.strictEqual(typeof config.git.branching_strategy, 'string', 'git.branching_strategy should be a string'); - } finally { - // Restore - if (existingDefaults !== null) { - fs.writeFileSync(defaultsFile, existingDefaults, 'utf-8'); - } else { - try { fs.unlinkSync(defaultsFile); } catch { /* ignore */ } - } - if (!gsdDirExisted) { - try { fs.rmdirSync(gsdDir); } catch { /* ignore */ } - } - } + const config = readConfig(tmpDir); + assert.strictEqual(config.model_profile, 'quality', 'model_profile should be overridden'); + assert.strictEqual(config.commit_docs, false, 'commit_docs should be overridden'); + assert.ok(config.git && typeof config.git === 'object', 'git should be an object'); + assert.strictEqual(typeof config.git.branching_strategy, 'string', 'git.branching_strategy should be a string'); }); - // NOTE: This test touches ~/.gsd/ on the real filesystem. It uses save/restore - // try/finally and skips if the file already exists to avoid corrupting user config. test('merges nested workflow keys from defaults.json preserving unset keys', () => { - const homedir = os.homedir(); - const gsdDir = path.join(homedir, '.gsd'); - const defaultsFile = path.join(gsdDir, 'defaults.json'); + // runGsdTools sandboxes HOME=tmpDir, so defaults.json is written there — + // no real filesystem side effects, cleanup happens via afterEach. + const gsdDir = path.join(tmpDir, '.gsd'); + fs.mkdirSync(gsdDir, { recursive: true }); + fs.writeFileSync(path.join(gsdDir, 'defaults.json'), JSON.stringify({ + workflow: { research: false }, + }), 'utf-8'); - let existingDefaults = null; - const gsdDirExisted = fs.existsSync(gsdDir); - if (fs.existsSync(defaultsFile)) { - existingDefaults = fs.readFileSync(defaultsFile, 'utf-8'); - } + const result = runGsdTools('config-ensure-section', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); - try { - if (!gsdDirExisted) { - fs.mkdirSync(gsdDir, { recursive: true }); - } - fs.writeFileSync(defaultsFile, JSON.stringify({ - workflow: { research: false }, - }), 'utf-8'); - - const result = runGsdTools('config-ensure-section', tmpDir); - assert.ok(result.success, `Command failed: ${result.error}`); - - const config = readConfig(tmpDir); - assert.strictEqual(config.workflow.research, false, 'research should be overridden'); - assert.strictEqual(typeof config.workflow.plan_check, 'boolean', 'plan_check should be a boolean'); - assert.strictEqual(typeof config.workflow.verifier, 'boolean', 'verifier should be a boolean'); - } finally { - if (existingDefaults !== null) { - fs.writeFileSync(defaultsFile, existingDefaults, 'utf-8'); - } else { - try { fs.unlinkSync(defaultsFile); } catch { /* ignore */ } - } - if (!gsdDirExisted) { - try { fs.rmdirSync(gsdDir); } catch { /* ignore */ } - } - } + const config = readConfig(tmpDir); + assert.strictEqual(config.workflow.research, false, 'research should be overridden'); + assert.strictEqual(typeof config.workflow.plan_check, 'boolean', 'plan_check should be a boolean'); + assert.strictEqual(typeof config.workflow.verifier, 'boolean', 'verifier should be a boolean'); }); }); diff --git a/tests/helpers.cjs b/tests/helpers.cjs index 4dddcf461..06abd6cc3 100644 --- a/tests/helpers.cjs +++ b/tests/helpers.cjs @@ -18,17 +18,22 @@ const TOOLS_PATH = path.join(__dirname, '..', 'get-shit-done', 'bin', 'gsd-tools function runGsdTools(args, cwd = process.cwd()) { try { let result; + // Override HOME so buildNewProjectConfig() doesn't pick up ~/.gsd/defaults.json + // from the developer's machine, which would cause flaky value assertions. + const env = { ...process.env, HOME: cwd }; if (Array.isArray(args)) { result = execFileSync(process.execPath, [TOOLS_PATH, ...args], { cwd, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], + env, }); } else { result = execSync(`node "${TOOLS_PATH}" ${args}`, { cwd, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], + env, }); } return { success: true, output: result.trim() };