Fix windows bugs + refactor tests. (#4634)

This commit is contained in:
Tommaso Sciortino 2025-07-21 22:21:37 -07:00 committed by GitHub
parent 9daead63dd
commit 138ff73821
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 225 additions and 278 deletions

View File

@ -4,342 +4,311 @@
* SPDX-License-Identifier: Apache-2.0 * SPDX-License-Identifier: Apache-2.0
*/ */
/* eslint-disable @typescript-eslint/no-explicit-any */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { describe, it, expect, vi, beforeEach, afterEach, Mock } from 'vitest';
import fsPromises from 'fs/promises'; import fsPromises from 'fs/promises';
import * as fs from 'fs';
import * as nodePath from 'path'; import * as nodePath from 'path';
import * as os from 'os';
import { getFolderStructure } from './getFolderStructure.js'; import { getFolderStructure } from './getFolderStructure.js';
import * as gitUtils from './gitUtils.js'; import * as gitUtils from './gitUtils.js';
import { FileDiscoveryService } from '../services/fileDiscoveryService.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'; import * as path from 'path';
interface TestDirent { vi.mock('./gitUtils.js');
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: '',
});
describe('getFolderStructure', () => { describe('getFolderStructure', () => {
beforeEach(() => { let testRootDir: string;
vi.resetAllMocks();
// path.resolve is now a vi.fn() due to the top-level vi.mock. const createEmptyDir = async (...pathSegments: string[]) => {
// We ensure its implementation is set for each test (or rely on the one from vi.mock). const fullPath = path.join(testRootDir, ...pathSegments);
// vi.resetAllMocks() clears call history but not the implementation set by vi.fn() in vi.mock. await fsPromises.mkdir(fullPath, { recursive: true });
// If we needed to change it per test, we would do it here:
(path.resolve as Mock).mockImplementation((str: string) => str);
// 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' },
);
},
);
});
afterEach(() => {
vi.restoreAllMocks(); // Restores spies (like fsPromises.readdir) and resets vi.fn mocks (like path.resolve)
});
const mockFsStructure: Record<string, TestDirent[]> = {
'/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'),
],
}; };
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();
});
afterEach(async () => {
await fsPromises.rm(testRootDir, { recursive: true, force: true });
vi.restoreAllMocks();
});
it('should return basic folder structure', async () => { it('should return basic folder structure', async () => {
const structure = await getFolderStructure('/testroot/subfolderA'); await createTestFile('fileA1.ts');
const expected = ` 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). Showing up to 200 items (files + folders).
/testroot/subfolderA/ ${testRootDir}${path.sep}
fileA1.ts fileA1.ts
fileA2.js fileA2.js
subfolderB/ subfolderB${path.sep}
fileB1.md fileB1.md
`.trim(); `.trim(),
expect(structure.trim()).toBe(expected); );
}); });
it('should handle an empty folder', async () => { it('should handle an empty folder', async () => {
const structure = await getFolderStructure('/testroot/emptyFolder'); const structure = await getFolderStructure(testRootDir);
const expected = ` expect(structure.trim()).toBe(
`
Showing up to 200 items (files + folders). Showing up to 200 items (files + folders).
/testroot/emptyFolder/ ${testRootDir}${path.sep}
`.trim(); `
expect(structure.trim()).toBe(expected.trim()); .trim()
.trim(),
);
}); });
it('should ignore folders specified in ignoredFolders (default)', async () => { it('should ignore folders specified in ignoredFolders (default)', async () => {
const structure = await getFolderStructure('/testroot'); await createTestFile('.hiddenfile');
const expected = ` 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. 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 .hiddenfile
file1.txt file1.txt
emptyFolder/ emptyFolder${path.sep}
node_modules/... node_modules${path.sep}...
subfolderA/ subfolderA${path.sep}
fileA1.ts fileA1.ts
fileA2.js fileA2.js
subfolderB/ subfolderB${path.sep}
fileB1.md fileB1.md
`.trim(); `.trim(),
expect(structure.trim()).toBe(expected); );
}); });
it('should ignore folders specified in custom ignoredFolders', async () => { 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']), ignoredFolders: new Set(['subfolderA', 'node_modules']),
}); });
const expected = ` 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. 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 .hiddenfile
file1.txt file1.txt
emptyFolder/ emptyFolder${path.sep}
node_modules/... node_modules${path.sep}...
subfolderA/... subfolderA${path.sep}...
`.trim(); `.trim();
expect(structure.trim()).toBe(expected); expect(structure.trim()).toBe(expected);
}); });
it('should filter files by fileIncludePattern', async () => { 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$/, fileIncludePattern: /\.ts$/,
}); });
const expected = ` const expected = `
Showing up to 200 items (files + folders). Showing up to 200 items (files + folders).
/testroot/subfolderA/ ${testRootDir}${path.sep}
fileA1.ts fileA1.ts
subfolderB/ subfolderB${path.sep}
`.trim(); `.trim();
expect(structure.trim()).toBe(expected); expect(structure.trim()).toBe(expected);
}); });
it('should handle maxItems truncation for files within a folder', async () => { 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, maxItems: 3,
}); });
const expected = ` const expected = `
Showing up to 3 items (files + folders). Showing up to 3 items (files + folders).
/testroot/subfolderA/ ${testRootDir}${path.sep}
fileA1.ts fileA1.ts
fileA2.js fileA2.js
subfolderB/ subfolderB${path.sep}
`.trim(); `.trim();
expect(structure.trim()).toBe(expected); expect(structure.trim()).toBe(expected);
}); });
it('should handle maxItems truncation for subfolders', async () => { 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, maxItems: 4,
}); });
const expectedRevised = ` 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. 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/ ${testRootDir}${path.sep}
folder-0/ folder-0${path.sep}
folder-1/ folder-1${path.sep}
folder-2/ folder-2${path.sep}
folder-3/ folder-3${path.sep}
... ...
`.trim(); `.trim();
expect(structure.trim()).toBe(expectedRevised); expect(structure.trim()).toBe(expectedRevised);
}); });
it('should handle maxItems that only allows the root folder itself', async () => { 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, 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. 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 fileA1.ts
... ...
... ...
`.trim(); `.trim();
expect(structure.trim()).toBe(expectedRevisedMax1); expect(structure.trim()).toBe(expected);
}); });
it('should handle nonexistent directory', async () => { it('should handle non-existent directory', async () => {
// Temporarily make fsPromises.readdir throw ENOENT for this specific path const nonExistentPath = path.join(testRootDir, 'non-existent');
const originalReaddir = fsPromises.readdir; const structure = await getFolderStructure(nonExistentPath);
(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');
expect(structure).toContain( 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 () => { 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, maxItems: 10,
}); });
const expected = ` const expected = `
Showing up to 10 items (files + folders). Showing up to 10 items (files + folders).
/testroot/deepFolders/ ${testRootDir}${path.sep}
level1/ level1${path.sep}
level2/ level2${path.sep}
level3/ level3${path.sep}
file.txt file.txt
`.trim(); `.trim();
expect(structure.trim()).toBe(expected); expect(structure.trim()).toBe(expected);
}); });
it('should truncate deep folder structure if maxItems is small', async () => { 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, maxItems: 3,
}); });
const expected = ` const expected = `
Showing up to 3 items (files + folders). Showing up to 3 items (files + folders).
/testroot/deepFolders/ ${testRootDir}${path.sep}
level1/ level1${path.sep}
level2/ level2${path.sep}
level3/ level3${path.sep}
`.trim(); `.trim();
expect(structure.trim()).toBe(expected); expect(structure.trim()).toBe(expected);
}); });
});
describe('getFolderStructure gitignore', () => { describe('with gitignore', () => {
beforeEach(() => { 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 [];
});
(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 '';
});
vi.mocked(gitUtils.isGitRepository).mockReturnValue(true); vi.mocked(gitUtils.isGitRepository).mockReturnValue(true);
}); });
it('should ignore files and folders specified in .gitignore', async () => { it('should ignore files and folders specified in .gitignore', async () => {
const fileService = new FileDiscoveryService('/test/project'); await fsPromises.writeFile(
const structure = await getFolderStructure('/test/project', { 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');
});
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');
});
});
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');
const fileService = new FileDiscoveryService(testRootDir);
const structure = await getFolderStructure(testRootDir, {
fileService, fileService,
}); });
expect(structure).not.toContain('ignored.txt'); expect(structure).not.toContain('ignored.txt');
@ -347,41 +316,28 @@ describe('getFolderStructure gitignore', () => {
expect(structure).not.toContain('logs.json'); expect(structure).not.toContain('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,
},
});
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 () => { it('should not ignore files if respectGeminiIgnore is false', async () => {
const fileService = new FileDiscoveryService('/test/project'); await fsPromises.writeFile(
const structure = await getFolderStructure('/test/project', { 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');
const fileService = new FileDiscoveryService(testRootDir);
const structure = await getFolderStructure(testRootDir, {
fileService, fileService,
fileFilteringOptions: { fileFilteringOptions: {
respectGeminiIgnore: false, respectGeminiIgnore: false,
respectGitIgnore: true, // Explicitly disable gemini ignore only respectGitIgnore: true, // Explicitly disable gemini ignore only
}, },
}); });
expect(structure).toContain('gem_ignored.txt'); expect(structure).toContain('ignored.txt');
// node_modules is still ignored by default // node_modules is still ignored by default
expect(structure).toContain('node_modules/...'); expect(structure).toContain('node_modules/...');
}); });
}); });
});

View File

@ -236,7 +236,7 @@ function formatStructure(
// Ignored root nodes ARE printed with a connector. // Ignored root nodes ARE printed with a connector.
if (!isProcessingRootNode || node.isIgnored) { if (!isProcessingRootNode || node.isIgnored) {
builder.push( 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); formatStructure(structureRoot, '', true, true, structureLines);
// 3. Build the final output string // 3. Build the final output string
const displayPath = resolvedPath.replace(/\\/g, '/'); function isTruncated(node: FullFolderInfo): boolean {
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) {
if (node.hasMoreFiles || node.hasMoreSubfolders || node.isIgnored) { if (node.hasMoreFiles || node.hasMoreSubfolders || node.isIgnored) {
truncationOccurred = true; return true;
} }
if (!truncationOccurred) {
for (const sub of node.subFolders) { for (const sub of node.subFolders) {
checkForTruncation(sub); if (isTruncated(sub)) {
if (truncationOccurred) break; return true;
} }
} }
} return false;
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.`;
} }
const summary = let summary = `Showing up to ${mergedOptions.maxItems} items (files + folders).`;
`Showing up to ${mergedOptions.maxItems} items (files + folders). ${disclaimer}`.trim();
const output = `${summary}\n\n${displayPath}/\n${structureLines.join('\n')}`; if (isTruncated(structureRoot)) {
return output; 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) { } catch (error: unknown) {
console.error(`Error getting folder structure for ${resolvedPath}:`, error); console.error(`Error getting folder structure for ${resolvedPath}:`, error);
return `Error processing directory "${resolvedPath}": ${getErrorMessage(error)}`; return `Error processing directory "${resolvedPath}": ${getErrorMessage(error)}`;