From 40673a89fa14f1a07608a0a4acfc35f00b575550 Mon Sep 17 00:00:00 2001 From: Johnson K C Date: Sun, 7 Jun 2026 01:25:43 -0400 Subject: [PATCH] test: guard broken-symlink tests so the suite passes on Windows (#2176) * test: guard broken-symlink tests so the suite passes on Windows Four test cases create a dangling symlink with fs.symlinkSync() to exercise statSync catch branches, but did not guard for platforms where symlink creation is not permitted. On Windows without Developer Mode / admin rights, fs.symlinkSync throws EPERM, so these tests fail and `npm test` is red: - tests/ci/validators.test.js (Round 73, validate-commands skill entry) - tests/lib/session-manager.test.js (Round 83, getAllSessions) - tests/lib/session-manager.test.js (Round 84, getSessionById) - tests/lib/utils.test.js (Round 84, findFiles) Wrap each symlinkSync in try/catch and skip cleanly on failure, mirroring the existing convention already used in this repo (validators.test.js Round 57 and hooks/config-protection.test.js). On Linux/macOS and admin Windows the symlink still succeeds and the tests run unchanged; only the unsupported-symlink path now skips instead of failing. Co-Authored-By: Claude Opus 4.8 (1M context) * test: only skip symlink tests on EPERM/EACCES, rethrow other errors Address CodeRabbit review: the catch blocks swallowed every error, which could mask a real test/setup failure as a false skip. Inspect err.code and only take the skip path for EPERM/EACCES (symlink creation blocked, e.g. Windows without Developer Mode); rethrow anything else so genuine failures still surface. Per the repo coding guideline: never silently swallow errors. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- tests/ci/validators.test.js | 16 +++++++++++++++- tests/lib/session-manager.test.js | 28 ++++++++++++++++++++++++++-- tests/lib/utils.test.js | 14 +++++++++++++- 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/tests/ci/validators.test.js b/tests/ci/validators.test.js index 0e681084..0a06fc8f 100644 --- a/tests/ci/validators.test.js +++ b/tests/ci/validators.test.js @@ -2577,7 +2577,21 @@ function runTests() { fs.mkdirSync(validSkill, { recursive: true }); // Broken symlink: target does not exist — statSync will throw ENOENT const brokenLink = path.join(skillsDir, 'broken-skill'); - fs.symlinkSync('/nonexistent/target/path', brokenLink); + try { + fs.symlinkSync('/nonexistent/target/path', brokenLink); + } catch (err) { + // Skip only where symlink creation is blocked (e.g. Windows without + // Developer Mode / admin rights → EPERM/EACCES); rethrow anything else + // so real failures aren't masked. + if (err && (err.code === 'EPERM' || err.code === 'EACCES')) { + console.log(' (skipped — symlinks not supported)'); + cleanupTestDir(testDir); + cleanupTestDir(agentsDir); + fs.rmSync(skillsDir, { recursive: true, force: true }); + return; + } + throw err; + } // Command that references the valid skill (should resolve) fs.writeFileSync(path.join(testDir, 'cmd.md'), diff --git a/tests/lib/session-manager.test.js b/tests/lib/session-manager.test.js index e39b0a87..a34447d7 100644 --- a/tests/lib/session-manager.test.js +++ b/tests/lib/session-manager.test.js @@ -1384,7 +1384,19 @@ src/main.ts // Create a broken symlink that matches the session filename pattern const brokenSymlink = '2026-02-10-deadbeef-session.tmp'; - fs.symlinkSync('/nonexistent/path/that/does/not/exist', path.join(sessionsDir, brokenSymlink)); + try { + fs.symlinkSync('/nonexistent/path/that/does/not/exist', path.join(sessionsDir, brokenSymlink)); + } catch (err) { + // Skip only where symlink creation is blocked (e.g. Windows without + // Developer Mode / admin rights → EPERM/EACCES); rethrow anything else + // so real failures aren't masked. + if (err && (err.code === 'EPERM' || err.code === 'EACCES')) { + console.log(' (skipped — symlinks not supported)'); + fs.rmSync(isoHome, { recursive: true, force: true }); + return; + } + throw err; + } const origHome = process.env.HOME; const origUserProfile = process.env.USERPROFILE; @@ -1421,7 +1433,19 @@ src/main.ts // Create a broken symlink that matches a session ID pattern const brokenFile = '2026-02-11-deadbeef-session.tmp'; - fs.symlinkSync('/nonexistent/target/that/does/not/exist', path.join(sessionsDir, brokenFile)); + try { + fs.symlinkSync('/nonexistent/target/that/does/not/exist', path.join(sessionsDir, brokenFile)); + } catch (err) { + // Skip only where symlink creation is blocked (e.g. Windows without + // Developer Mode / admin rights → EPERM/EACCES); rethrow anything else + // so real failures aren't masked. + if (err && (err.code === 'EPERM' || err.code === 'EACCES')) { + console.log(' (skipped — symlinks not supported)'); + fs.rmSync(isoHome, { recursive: true, force: true }); + return; + } + throw err; + } const origHome = process.env.HOME; const origUserProfile = process.env.USERPROFILE; diff --git a/tests/lib/utils.test.js b/tests/lib/utils.test.js index a85758e4..3217567a 100644 --- a/tests/lib/utils.test.js +++ b/tests/lib/utils.test.js @@ -1415,7 +1415,19 @@ function runTests() { const realFile = path.join(tmpDir, 'real.txt'); fs.writeFileSync(realFile, 'content'); const brokenLink = path.join(tmpDir, 'broken.txt'); - fs.symlinkSync('/nonexistent/path/does/not/exist', brokenLink); + try { + fs.symlinkSync('/nonexistent/path/does/not/exist', brokenLink); + } catch (err) { + // Skip only where symlink creation is blocked (e.g. Windows without + // Developer Mode / admin rights → EPERM/EACCES); rethrow anything else + // so real failures aren't masked. + if (err && (err.code === 'EPERM' || err.code === 'EACCES')) { + console.log(' (skipped — symlinks not supported)'); + fs.rmSync(tmpDir, { recursive: true, force: true }); + return; + } + throw err; + } try { const results = utils.findFiles(tmpDir, '*.txt');