From 138ff738216e1dd29434bd20868328c96b791ac7 Mon Sep 17 00:00:00 2001 From: Tommaso Sciortino Date: Mon, 21 Jul 2025 22:21:37 -0700 Subject: [PATCH] Fix windows bugs + refactor tests. (#4634) --- .../core/src/utils/getFolderStructure.test.ts | 468 ++++++++---------- packages/core/src/utils/getFolderStructure.ts | 35 +- 2 files changed, 225 insertions(+), 278 deletions(-) diff --git a/packages/core/src/utils/getFolderStructure.test.ts b/packages/core/src/utils/getFolderStructure.test.ts index b2da4aab..8694fe20 100644 --- a/packages/core/src/utils/getFolderStructure.test.ts +++ b/packages/core/src/utils/getFolderStructure.test.ts @@ -4,384 +4,340 @@ * SPDX-License-Identifier: Apache-2.0 */ -/* eslint-disable @typescript-eslint/no-explicit-any */ -import { describe, it, expect, vi, beforeEach, afterEach, Mock } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import fsPromises from 'fs/promises'; -import * as fs from 'fs'; import * as nodePath from 'path'; +import * as os from 'os'; import { getFolderStructure } from './getFolderStructure.js'; import * as gitUtils from './gitUtils.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; - -vi.mock('path', async (importOriginal) => { - const original = (await importOriginal()) as typeof nodePath; - return { - ...original, - resolve: vi.fn((str) => str), - // Other path functions (basename, join, normalize, etc.) will use original implementation - }; -}); - -vi.mock('fs/promises'); -vi.mock('fs'); -vi.mock('./gitUtils.js'); - -// Import 'path' again here, it will be the mocked version import * as path from 'path'; -interface TestDirent { - name: string; - isFile: () => boolean; - isDirectory: () => boolean; - isBlockDevice: () => boolean; - isCharacterDevice: () => boolean; - isSymbolicLink: () => boolean; - isFIFO: () => boolean; - isSocket: () => boolean; - path: string; - parentPath: string; -} - -// Helper to create Dirent-like objects for mocking fs.readdir -const createDirent = (name: string, type: 'file' | 'dir'): TestDirent => ({ - name, - isFile: () => type === 'file', - isDirectory: () => type === 'dir', - isBlockDevice: () => false, - isCharacterDevice: () => false, - isSymbolicLink: () => false, - isFIFO: () => false, - isSocket: () => false, - path: '', - parentPath: '', -}); +vi.mock('./gitUtils.js'); describe('getFolderStructure', () => { - beforeEach(() => { + let testRootDir: string; + + const createEmptyDir = async (...pathSegments: string[]) => { + const fullPath = path.join(testRootDir, ...pathSegments); + await fsPromises.mkdir(fullPath, { recursive: true }); + }; + + const createTestFile = async (...pathSegments: string[]) => { + const fullPath = path.join(testRootDir, ...pathSegments); + await fsPromises.mkdir(path.dirname(fullPath), { recursive: true }); + await fsPromises.writeFile(fullPath, ''); + return fullPath; + }; + + beforeEach(async () => { + testRootDir = await fsPromises.mkdtemp( + path.join(os.tmpdir(), 'folder-structure-test-'), + ); vi.resetAllMocks(); + }); - // path.resolve is now a vi.fn() due to the top-level vi.mock. - // We ensure its implementation is set for each test (or rely on the one from vi.mock). - // vi.resetAllMocks() clears call history but not the implementation set by vi.fn() in vi.mock. - // If we needed to change it per test, we would do it here: - (path.resolve as Mock).mockImplementation((str: string) => str); + afterEach(async () => { + await fsPromises.rm(testRootDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); - // Re-apply/define the mock implementation for fsPromises.readdir for each test - (fsPromises.readdir as Mock).mockImplementation( - async (dirPath: string | Buffer | URL) => { - // path.normalize here will use the mocked path module. - // Since normalize is spread from original, it should be the real one. - const normalizedPath = path.normalize(dirPath.toString()); - if (mockFsStructure[normalizedPath]) { - return mockFsStructure[normalizedPath]; - } - throw Object.assign( - new Error( - `ENOENT: no such file or directory, scandir '${normalizedPath}'`, - ), - { code: 'ENOENT' }, - ); - }, + it('should return basic folder structure', async () => { + await createTestFile('fileA1.ts'); + await createTestFile('fileA2.js'); + await createTestFile('subfolderB', 'fileB1.md'); + + const structure = await getFolderStructure(testRootDir); + expect(structure.trim()).toBe( + ` +Showing up to 200 items (files + folders). + +${testRootDir}${path.sep} +├───fileA1.ts +├───fileA2.js +└───subfolderB${path.sep} + └───fileB1.md +`.trim(), ); }); - afterEach(() => { - vi.restoreAllMocks(); // Restores spies (like fsPromises.readdir) and resets vi.fn mocks (like path.resolve) - }); - - const mockFsStructure: Record = { - '/testroot': [ - createDirent('file1.txt', 'file'), - createDirent('subfolderA', 'dir'), - createDirent('emptyFolder', 'dir'), - createDirent('.hiddenfile', 'file'), - createDirent('node_modules', 'dir'), - ], - '/testroot/subfolderA': [ - createDirent('fileA1.ts', 'file'), - createDirent('fileA2.js', 'file'), - createDirent('subfolderB', 'dir'), - ], - '/testroot/subfolderA/subfolderB': [createDirent('fileB1.md', 'file')], - '/testroot/emptyFolder': [], - '/testroot/node_modules': [createDirent('somepackage', 'dir')], - '/testroot/manyFilesFolder': Array.from({ length: 10 }, (_, i) => - createDirent(`file-${i}.txt`, 'file'), - ), - '/testroot/manyFolders': Array.from({ length: 5 }, (_, i) => - createDirent(`folder-${i}`, 'dir'), - ), - ...Array.from({ length: 5 }, (_, i) => ({ - [`/testroot/manyFolders/folder-${i}`]: [ - createDirent('child.txt', 'file'), - ], - })).reduce((acc, val) => ({ ...acc, ...val }), {}), - '/testroot/deepFolders': [createDirent('level1', 'dir')], - '/testroot/deepFolders/level1': [createDirent('level2', 'dir')], - '/testroot/deepFolders/level1/level2': [createDirent('level3', 'dir')], - '/testroot/deepFolders/level1/level2/level3': [ - createDirent('file.txt', 'file'), - ], - }; - - it('should return basic folder structure', async () => { - const structure = await getFolderStructure('/testroot/subfolderA'); - const expected = ` -Showing up to 200 items (files + folders). - -/testroot/subfolderA/ -├───fileA1.ts -├───fileA2.js -└───subfolderB/ - └───fileB1.md -`.trim(); - expect(structure.trim()).toBe(expected); - }); - it('should handle an empty folder', async () => { - const structure = await getFolderStructure('/testroot/emptyFolder'); - const expected = ` + const structure = await getFolderStructure(testRootDir); + expect(structure.trim()).toBe( + ` Showing up to 200 items (files + folders). -/testroot/emptyFolder/ -`.trim(); - expect(structure.trim()).toBe(expected.trim()); +${testRootDir}${path.sep} +` + .trim() + .trim(), + ); }); it('should ignore folders specified in ignoredFolders (default)', async () => { - const structure = await getFolderStructure('/testroot'); - const expected = ` + await createTestFile('.hiddenfile'); + await createTestFile('file1.txt'); + await createEmptyDir('emptyFolder'); + await createTestFile('node_modules', 'somepackage', 'index.js'); + await createTestFile('subfolderA', 'fileA1.ts'); + await createTestFile('subfolderA', 'fileA2.js'); + await createTestFile('subfolderA', 'subfolderB', 'fileB1.md'); + + const structure = await getFolderStructure(testRootDir); + expect(structure.trim()).toBe( + ` Showing up to 200 items (files + folders). Folders or files indicated with ... contain more items not shown, were ignored, or the display limit (200 items) was reached. -/testroot/ +${testRootDir}${path.sep} ├───.hiddenfile ├───file1.txt -├───emptyFolder/ -├───node_modules/... -└───subfolderA/ +├───emptyFolder${path.sep} +├───node_modules${path.sep}... +└───subfolderA${path.sep} ├───fileA1.ts ├───fileA2.js - └───subfolderB/ + └───subfolderB${path.sep} └───fileB1.md -`.trim(); - expect(structure.trim()).toBe(expected); +`.trim(), + ); }); it('should ignore folders specified in custom ignoredFolders', async () => { - const structure = await getFolderStructure('/testroot', { + await createTestFile('.hiddenfile'); + await createTestFile('file1.txt'); + await createEmptyDir('emptyFolder'); + await createTestFile('node_modules', 'somepackage', 'index.js'); + await createTestFile('subfolderA', 'fileA1.ts'); + + const structure = await getFolderStructure(testRootDir, { ignoredFolders: new Set(['subfolderA', 'node_modules']), }); const expected = ` Showing up to 200 items (files + folders). Folders or files indicated with ... contain more items not shown, were ignored, or the display limit (200 items) was reached. -/testroot/ +${testRootDir}${path.sep} ├───.hiddenfile ├───file1.txt -├───emptyFolder/ -├───node_modules/... -└───subfolderA/... +├───emptyFolder${path.sep} +├───node_modules${path.sep}... +└───subfolderA${path.sep}... `.trim(); expect(structure.trim()).toBe(expected); }); it('should filter files by fileIncludePattern', async () => { - const structure = await getFolderStructure('/testroot/subfolderA', { + await createTestFile('fileA1.ts'); + await createTestFile('fileA2.js'); + await createTestFile('subfolderB', 'fileB1.md'); + + const structure = await getFolderStructure(testRootDir, { fileIncludePattern: /\.ts$/, }); const expected = ` Showing up to 200 items (files + folders). -/testroot/subfolderA/ +${testRootDir}${path.sep} ├───fileA1.ts -└───subfolderB/ +└───subfolderB${path.sep} `.trim(); expect(structure.trim()).toBe(expected); }); it('should handle maxItems truncation for files within a folder', async () => { - const structure = await getFolderStructure('/testroot/subfolderA', { + await createTestFile('fileA1.ts'); + await createTestFile('fileA2.js'); + await createTestFile('subfolderB', 'fileB1.md'); + + const structure = await getFolderStructure(testRootDir, { maxItems: 3, }); const expected = ` Showing up to 3 items (files + folders). -/testroot/subfolderA/ +${testRootDir}${path.sep} ├───fileA1.ts ├───fileA2.js -└───subfolderB/ +└───subfolderB${path.sep} `.trim(); expect(structure.trim()).toBe(expected); }); it('should handle maxItems truncation for subfolders', async () => { - const structure = await getFolderStructure('/testroot/manyFolders', { + for (let i = 0; i < 5; i++) { + await createTestFile(`folder-${i}`, 'child.txt'); + } + + const structure = await getFolderStructure(testRootDir, { maxItems: 4, }); const expectedRevised = ` Showing up to 4 items (files + folders). Folders or files indicated with ... contain more items not shown, were ignored, or the display limit (4 items) was reached. -/testroot/manyFolders/ -├───folder-0/ -├───folder-1/ -├───folder-2/ -├───folder-3/ +${testRootDir}${path.sep} +├───folder-0${path.sep} +├───folder-1${path.sep} +├───folder-2${path.sep} +├───folder-3${path.sep} └───... `.trim(); expect(structure.trim()).toBe(expectedRevised); }); it('should handle maxItems that only allows the root folder itself', async () => { - const structure = await getFolderStructure('/testroot/subfolderA', { + await createTestFile('fileA1.ts'); + await createTestFile('fileA2.ts'); + await createTestFile('subfolderB', 'fileB1.ts'); + + const structure = await getFolderStructure(testRootDir, { maxItems: 1, }); - const expectedRevisedMax1 = ` + const expected = ` Showing up to 1 items (files + folders). Folders or files indicated with ... contain more items not shown, were ignored, or the display limit (1 items) was reached. -/testroot/subfolderA/ +${testRootDir}${path.sep} ├───fileA1.ts ├───... └───... `.trim(); - expect(structure.trim()).toBe(expectedRevisedMax1); + expect(structure.trim()).toBe(expected); }); - it('should handle nonexistent directory', async () => { - // Temporarily make fsPromises.readdir throw ENOENT for this specific path - const originalReaddir = fsPromises.readdir; - (fsPromises.readdir as Mock).mockImplementation( - async (p: string | Buffer | URL) => { - if (p === '/nonexistent') { - throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); - } - return originalReaddir(p); - }, - ); - - const structure = await getFolderStructure('/nonexistent'); + it('should handle non-existent directory', async () => { + const nonExistentPath = path.join(testRootDir, 'non-existent'); + const structure = await getFolderStructure(nonExistentPath); expect(structure).toContain( - 'Error: Could not read directory "/nonexistent"', + `Error: Could not read directory "${nonExistentPath}". Check path and permissions.`, ); }); it('should handle deep folder structure within limits', async () => { - const structure = await getFolderStructure('/testroot/deepFolders', { + await createTestFile('level1', 'level2', 'level3', 'file.txt'); + + const structure = await getFolderStructure(testRootDir, { maxItems: 10, }); const expected = ` Showing up to 10 items (files + folders). -/testroot/deepFolders/ -└───level1/ - └───level2/ - └───level3/ +${testRootDir}${path.sep} +└───level1${path.sep} + └───level2${path.sep} + └───level3${path.sep} └───file.txt `.trim(); expect(structure.trim()).toBe(expected); }); it('should truncate deep folder structure if maxItems is small', async () => { - const structure = await getFolderStructure('/testroot/deepFolders', { + await createTestFile('level1', 'level2', 'level3', 'file.txt'); + + const structure = await getFolderStructure(testRootDir, { maxItems: 3, }); const expected = ` Showing up to 3 items (files + folders). -/testroot/deepFolders/ -└───level1/ - └───level2/ - └───level3/ +${testRootDir}${path.sep} +└───level1${path.sep} + └───level2${path.sep} + └───level3${path.sep} `.trim(); expect(structure.trim()).toBe(expected); }); -}); -describe('getFolderStructure gitignore', () => { - beforeEach(() => { - vi.resetAllMocks(); - (path.resolve as Mock).mockImplementation((str: string) => str); - - (fsPromises.readdir as Mock).mockImplementation(async (p) => { - const path = p.toString(); - if (path === '/test/project') { - return [ - createDirent('file1.txt', 'file'), - createDirent('node_modules', 'dir'), - createDirent('ignored.txt', 'file'), - createDirent('gem_ignored.txt', 'file'), - createDirent('.gemini', 'dir'), - ] as any; - } - if (path === '/test/project/node_modules') { - return [createDirent('some-package', 'dir')] as any; - } - if (path === '/test/project/.gemini') { - return [ - createDirent('config.yaml', 'file'), - createDirent('logs.json', 'file'), - ] as any; - } - return []; + describe('with gitignore', () => { + beforeEach(() => { + vi.mocked(gitUtils.isGitRepository).mockReturnValue(true); }); - (fs.readFileSync as Mock).mockImplementation((p) => { - const path = p.toString(); - if (path === '/test/project/.gitignore') { - return 'ignored.txt\nnode_modules/\n.gemini/\n!/.gemini/config.yaml'; - } - if (path === '/test/project/.geminiignore') { - return 'gem_ignored.txt\nnode_modules/\n.gemini/\n!/.gemini/config.yaml'; - } - return ''; + it('should ignore files and folders specified in .gitignore', async () => { + await fsPromises.writeFile( + nodePath.join(testRootDir, '.gitignore'), + 'ignored.txt\nnode_modules/\n.gemini/*\n!/.gemini/config.yaml', + ); + await createTestFile('file1.txt'); + await createTestFile('node_modules', 'some-package', 'index.js'); + await createTestFile('ignored.txt'); + await createTestFile('.gemini', 'config.yaml'); + await createTestFile('.gemini', 'logs.json'); + + const fileService = new FileDiscoveryService(testRootDir); + const structure = await getFolderStructure(testRootDir, { + fileService, + }); + + expect(structure).not.toContain('ignored.txt'); + expect(structure).toContain(`node_modules${path.sep}...`); + expect(structure).not.toContain('logs.json'); + expect(structure).toContain('config.yaml'); + expect(structure).toContain('file1.txt'); }); - vi.mocked(gitUtils.isGitRepository).mockReturnValue(true); + it('should not ignore files if respectGitIgnore is false', async () => { + await fsPromises.writeFile( + nodePath.join(testRootDir, '.gitignore'), + 'ignored.txt', + ); + await createTestFile('file1.txt'); + await createTestFile('ignored.txt'); + + const fileService = new FileDiscoveryService(testRootDir); + const structure = await getFolderStructure(testRootDir, { + fileService, + fileFilteringOptions: { + respectGeminiIgnore: false, + respectGitIgnore: false, + }, + }); + + expect(structure).toContain('ignored.txt'); + expect(structure).toContain('file1.txt'); + }); }); - it('should ignore files and folders specified in .gitignore', async () => { - const fileService = new FileDiscoveryService('/test/project'); - const structure = await getFolderStructure('/test/project', { - fileService, - }); - expect(structure).not.toContain('ignored.txt'); - expect(structure).toContain('node_modules/...'); - expect(structure).not.toContain('logs.json'); - }); + describe('with geminiignore', () => { + it('should ignore geminiignore files by default', async () => { + await fsPromises.writeFile( + nodePath.join(testRootDir, '.geminiignore'), + 'ignored.txt\nnode_modules/\n.gemini/\n!/.gemini/config.yaml', + ); + await createTestFile('file1.txt'); + await createTestFile('node_modules', 'some-package', 'index.js'); + await createTestFile('ignored.txt'); + await createTestFile('.gemini', 'config.yaml'); + await createTestFile('.gemini', 'logs.json'); - it('should not ignore files if respectGitIgnore is false', async () => { - const fileService = new FileDiscoveryService('/test/project'); - const structure = await getFolderStructure('/test/project', { - fileService, - fileFilteringOptions: { - respectGeminiIgnore: false, - respectGitIgnore: false, - }, + const fileService = new FileDiscoveryService(testRootDir); + const structure = await getFolderStructure(testRootDir, { + fileService, + }); + expect(structure).not.toContain('ignored.txt'); + expect(structure).toContain('node_modules/...'); + expect(structure).not.toContain('logs.json'); }); - expect(structure).toContain('ignored.txt'); - // node_modules is still ignored by default - expect(structure).toContain('node_modules/...'); - }); - it('should ignore files and folders specified in .geminiignore', async () => { - const fileService = new FileDiscoveryService('/test/project'); - const structure = await getFolderStructure('/test/project', { - fileService, - }); - expect(structure).not.toContain('gem_ignored.txt'); - expect(structure).toContain('node_modules/...'); - expect(structure).not.toContain('logs.json'); - }); + it('should not ignore files if respectGeminiIgnore is false', async () => { + await fsPromises.writeFile( + nodePath.join(testRootDir, '.geminiignore'), + 'ignored.txt\nnode_modules/\n.gemini/\n!/.gemini/config.yaml', + ); + await createTestFile('file1.txt'); + await createTestFile('node_modules', 'some-package', 'index.js'); + await createTestFile('ignored.txt'); + await createTestFile('.gemini', 'config.yaml'); + await createTestFile('.gemini', 'logs.json'); - it('should not ignore files if respectGeminiIgnore is false', async () => { - const fileService = new FileDiscoveryService('/test/project'); - const structure = await getFolderStructure('/test/project', { - fileService, - fileFilteringOptions: { - respectGeminiIgnore: false, - respectGitIgnore: true, // Explicitly disable gemini ignore only - }, + const fileService = new FileDiscoveryService(testRootDir); + const structure = await getFolderStructure(testRootDir, { + fileService, + fileFilteringOptions: { + respectGeminiIgnore: false, + respectGitIgnore: true, // Explicitly disable gemini ignore only + }, + }); + expect(structure).toContain('ignored.txt'); + // node_modules is still ignored by default + expect(structure).toContain('node_modules/...'); }); - expect(structure).toContain('gem_ignored.txt'); - // node_modules is still ignored by default - expect(structure).toContain('node_modules/...'); }); }); diff --git a/packages/core/src/utils/getFolderStructure.ts b/packages/core/src/utils/getFolderStructure.ts index 15588a4b..60c539b5 100644 --- a/packages/core/src/utils/getFolderStructure.ts +++ b/packages/core/src/utils/getFolderStructure.ts @@ -236,7 +236,7 @@ function formatStructure( // Ignored root nodes ARE printed with a connector. if (!isProcessingRootNode || node.isIgnored) { builder.push( - `${currentIndent}${connector}${node.name}/${node.isIgnored ? TRUNCATION_INDICATOR : ''}`, + `${currentIndent}${connector}${node.name}${path.sep}${node.isIgnored ? TRUNCATION_INDICATOR : ''}`, ); } @@ -322,34 +322,25 @@ export async function getFolderStructure( formatStructure(structureRoot, '', true, true, structureLines); // 3. Build the final output string - const displayPath = resolvedPath.replace(/\\/g, '/'); - - let disclaimer = ''; - // Check if truncation occurred anywhere or if ignored folders are present. - // A simple check: if any node indicates more files/subfolders, or is ignored. - let truncationOccurred = false; - function checkForTruncation(node: FullFolderInfo) { + function isTruncated(node: FullFolderInfo): boolean { if (node.hasMoreFiles || node.hasMoreSubfolders || node.isIgnored) { - truncationOccurred = true; + return true; } - if (!truncationOccurred) { - for (const sub of node.subFolders) { - checkForTruncation(sub); - if (truncationOccurred) break; + for (const sub of node.subFolders) { + if (isTruncated(sub)) { + return true; } } - } - checkForTruncation(structureRoot); - - if (truncationOccurred) { - disclaimer = `Folders or files indicated with ${TRUNCATION_INDICATOR} contain more items not shown, were ignored, or the display limit (${mergedOptions.maxItems} items) was reached.`; + return false; } - const summary = - `Showing up to ${mergedOptions.maxItems} items (files + folders). ${disclaimer}`.trim(); + let summary = `Showing up to ${mergedOptions.maxItems} items (files + folders).`; - const output = `${summary}\n\n${displayPath}/\n${structureLines.join('\n')}`; - return output; + if (isTruncated(structureRoot)) { + summary += ` Folders or files indicated with ${TRUNCATION_INDICATOR} contain more items not shown, were ignored, or the display limit (${mergedOptions.maxItems} items) was reached.`; + } + + return `${summary}\n\n${resolvedPath}${path.sep}\n${structureLines.join('\n')}`; } catch (error: unknown) { console.error(`Error getting folder structure for ${resolvedPath}:`, error); return `Error processing directory "${resolvedPath}": ${getErrorMessage(error)}`;