From e7eac663eede5b72a49995e576f2d49f58a52cc5 Mon Sep 17 00:00:00 2001 From: Tabish Bidiwale Date: Fri, 9 Jan 2026 16:04:12 -0800 Subject: [PATCH] fix: skip Windows-specific permission tests that rely on chmod() fs.chmod() on directories doesn't restrict write access on Windows since Windows uses ACLs that Node.js doesn't control. Additionally, admin users and CI runners can bypass read-only attributes. Skip these tests on Windows and use platform-specific invalid paths in cross-platform tests. Fixes #401 (bash/pwsh completion commit breaking Windows e2e tests). --- .../installers/bash-installer.test.ts | 12 ++++++++++-- .../installers/fish-installer.test.ts | 4 +++- .../installers/powershell-installer.test.ts | 16 ++++++++++++---- test/utils/file-system.test.ts | 4 +++- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/test/core/completions/installers/bash-installer.test.ts b/test/core/completions/installers/bash-installer.test.ts index 7b72f5c30..948a36221 100644 --- a/test/core/completions/installers/bash-installer.test.ts +++ b/test/core/completions/installers/bash-installer.test.ts @@ -140,7 +140,11 @@ describe('BashInstaller', () => { it('should handle installation errors gracefully', async () => { // Create installer with non-existent/invalid home directory - const invalidInstaller = new BashInstaller('/root/invalid/nonexistent/path'); + // Use a path that will fail on both Unix and Windows + const invalidPath = process.platform === 'win32' + ? 'Z:\\nonexistent\\invalid\\path' // Non-existent drive letter on Windows + : '/root/invalid/nonexistent/path'; // Permission-denied path on Unix + const invalidInstaller = new BashInstaller(invalidPath); const result = await invalidInstaller.install(testScript); @@ -374,7 +378,11 @@ describe('BashInstaller', () => { it('should handle write permission errors gracefully', async () => { // Create installer with path that can't be written - const invalidInstaller = new BashInstaller('/root/invalid/path'); + // Use a path that will fail on both Unix and Windows + const invalidPath = process.platform === 'win32' + ? 'Z:\\nonexistent\\invalid\\path' // Non-existent drive letter on Windows + : '/root/invalid/path'; // Permission-denied path on Unix + const invalidInstaller = new BashInstaller(invalidPath); const result = await invalidInstaller.configureBashrc(completionsDir); diff --git a/test/core/completions/installers/fish-installer.test.ts b/test/core/completions/installers/fish-installer.test.ts index d5608ca24..db54f8b0b 100644 --- a/test/core/completions/installers/fish-installer.test.ts +++ b/test/core/completions/installers/fish-installer.test.ts @@ -192,7 +192,9 @@ complete -c openspec -a 'validate' -d 'Validate specs' await fs.rm(spacedHomeDir, { recursive: true, force: true }); }); - it('should return failure on permission error', async () => { + // Skip on Windows: fs.chmod() on directories doesn't restrict write access on Windows + // Windows uses ACLs which Node.js chmod doesn't control + it.skipIf(process.platform === 'win32')('should return failure on permission error', async () => { // Create a read-only directory to simulate permission error const restrictedDir = path.join(testHomeDir, '.config', 'fish', 'completions'); await fs.mkdir(restrictedDir, { recursive: true }); diff --git a/test/core/completions/installers/powershell-installer.test.ts b/test/core/completions/installers/powershell-installer.test.ts index 315f2a458..780f63103 100644 --- a/test/core/completions/installers/powershell-installer.test.ts +++ b/test/core/completions/installers/powershell-installer.test.ts @@ -237,7 +237,9 @@ describe('PowerShellInstaller', () => { expect(content).toContain('# OPENSPEC:END'); }); - it('should return false on write permission error', async () => { + // Skip on Windows: fs.chmod() doesn't reliably restrict write access on Windows + // (admin users can bypass read-only attribute, and CI runners often have elevated privileges) + it.skipIf(process.platform === 'win32')('should return false on write permission error', async () => { delete process.env.OPENSPEC_NO_AUTO_CONFIG; const profilePath = installer.getProfilePath(); await fs.mkdir(path.dirname(profilePath), { recursive: true }); @@ -451,7 +453,9 @@ Register-ArgumentCompleter -CommandName openspec -ScriptBlock $openspecCompleter // Note: OPENSPEC_NO_AUTO_CONFIG support was removed from PowerShell installer // Profile is now always auto-configured if possible - it('should provide instructions when profile cannot be configured', async () => { + // Skip on Windows: fs.chmod() doesn't reliably restrict write access on Windows + // (admin users can bypass read-only attribute, and CI runners often have elevated privileges) + it.skipIf(process.platform === 'win32')('should provide instructions when profile cannot be configured', async () => { delete process.env.OPENSPEC_NO_AUTO_CONFIG; // Make profile directory read-only to prevent configuration const profilePath = installer.getProfilePath(); @@ -496,7 +500,9 @@ Register-ArgumentCompleter -CommandName openspec -ScriptBlock $openspecCompleter await fs.rm(spacedHomeDir, { recursive: true, force: true }); }); - it('should return failure on permission error', async () => { + // Skip on Windows: fs.chmod() on directories doesn't restrict write access on Windows + // Windows uses ACLs which Node.js chmod doesn't control + it.skipIf(process.platform === 'win32')('should return failure on permission error', async () => { const targetPath = installer.getInstallationPath(); const targetDir = path.dirname(targetPath); await fs.mkdir(targetDir, { recursive: true }); @@ -614,7 +620,9 @@ Register-ArgumentCompleter -CommandName openspec -ScriptBlock $openspecCompleter expect(profileContentAfter).not.toContain('# OPENSPEC:START'); }); - it('should return failure on permission error', async () => { + // Skip on Windows: fs.chmod() on directories doesn't restrict write access on Windows + // Windows uses ACLs which Node.js chmod doesn't control + it.skipIf(process.platform === 'win32')('should return failure on permission error', async () => { delete process.env.OPENSPEC_NO_AUTO_CONFIG; await installer.install(mockCompletionScript); const targetPath = installer.getInstallationPath(); diff --git a/test/utils/file-system.test.ts b/test/utils/file-system.test.ts index 2f427a531..ffa888cae 100644 --- a/test/utils/file-system.test.ts +++ b/test/utils/file-system.test.ts @@ -196,7 +196,9 @@ describe('FileSystemUtils', () => { expect(canWrite).toBe(true); }); - it('should return false for non-existent file in read-only directory', async () => { + // Skip on Windows: fs.chmod() on directories doesn't restrict write access on Windows + // Windows uses ACLs which Node.js chmod doesn't control + it.skipIf(process.platform === 'win32')('should return false for non-existent file in read-only directory', async () => { const readOnlyDir = path.join(testDir, 'readonly-dir'); await fs.mkdir(readOnlyDir); await fs.chmod(readOnlyDir, 0o555); // Read-only + execute