centralize file filtering in `FileDiscoveryService` (#1039)

1 files changed, 24 insertions, 118 deletions

diff --git a/packages/core/src/services/fileDiscoveryService.test.ts b/packages/core/src/services/fileDiscoveryService.test.ts
index 368f4d23..d7530cd6 100644
--- a/packages/core/src/services/fileDiscoveryService.test.ts
+++ b/packages/core/src/services/fileDiscoveryService.test.ts
@@ -8,15 +8,13 @@ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest';
 import type { Mocked } from 'vitest';
 import { FileDiscoveryService } from './fileDiscoveryService.js';
 import { GitIgnoreParser } from '../utils/gitIgnoreParser.js';
+import * as gitUtils from '../utils/gitUtils.js';
 
 // Mock the GitIgnoreParser
 vi.mock('../utils/gitIgnoreParser.js');
 
 // Mock gitUtils module
-vi.mock('../utils/gitUtils.js', () => ({
-  isGitRepository: vi.fn(() => true),
-  findGitRoot: vi.fn(() => '/test/project'),
-}));
+vi.mock('../utils/gitUtils.js');
 
 describe('FileDiscoveryService', () => {
   let service: FileDiscoveryService;
@@ -24,16 +22,16 @@ describe('FileDiscoveryService', () => {
   const mockProjectRoot = '/test/project';
 
   beforeEach(() => {
-    service = new FileDiscoveryService(mockProjectRoot);
-
     mockGitIgnoreParser = {
       initialize: vi.fn(),
       isIgnored: vi.fn(),
-      getIgnoredPatterns: vi.fn(() => ['.git/**', 'node_modules/**']),
-      parseGitIgnoreContent: vi.fn(),
+      loadPatterns: vi.fn(),
+      loadGitRepoPatterns: vi.fn(),
     } as unknown as Mocked<GitIgnoreParser>;
 
     vi.mocked(GitIgnoreParser).mockImplementation(() => mockGitIgnoreParser);
+    vi.mocked(gitUtils.isGitRepository).mockReturnValue(true);
+    vi.mocked(gitUtils.findGitRoot).mockReturnValue('/test/project');
     vi.clearAllMocks();
   });
 
@@ -42,36 +40,30 @@ describe('FileDiscoveryService', () => {
   });
 
   describe('initialization', () => {
-    it('should initialize git ignore parser by default', async () => {
-      await service.initialize();
-
+    it('should initialize git ignore parser by default', () => {
+      service = new FileDiscoveryService(mockProjectRoot);
       expect(GitIgnoreParser).toHaveBeenCalledWith(mockProjectRoot);
-      expect(mockGitIgnoreParser.initialize).toHaveBeenCalled();
-    });
-
-    it('should not initialize git ignore parser when respectGitIgnore is false', async () => {
-      await service.initialize({ respectGitIgnore: false });
-
-      expect(GitIgnoreParser).not.toHaveBeenCalled();
-      expect(mockGitIgnoreParser.initialize).not.toHaveBeenCalled();
+      expect(GitIgnoreParser).toHaveBeenCalledTimes(2);
+      expect(mockGitIgnoreParser.loadGitRepoPatterns).toHaveBeenCalled();
+      expect(mockGitIgnoreParser.loadPatterns).toHaveBeenCalled();
     });
 
-    it('should handle initialization errors gracefully', async () => {
-      mockGitIgnoreParser.initialize.mockRejectedValue(
-        new Error('Init failed'),
-      );
+    it('should not initialize git ignore parser when not a git repo', () => {
+      vi.mocked(gitUtils.isGitRepository).mockReturnValue(false);
+      service = new FileDiscoveryService(mockProjectRoot);
 
-      await expect(service.initialize()).rejects.toThrow('Init failed');
+      expect(GitIgnoreParser).toHaveBeenCalledOnce();
+      expect(mockGitIgnoreParser.loadGitRepoPatterns).not.toHaveBeenCalled();
     });
   });
 
   describe('filterFiles', () => {
-    beforeEach(async () => {
+    beforeEach(() => {
       mockGitIgnoreParser.isIgnored.mockImplementation(
         (path: string) =>
           path.includes('node_modules') || path.includes('.git'),
       );
-      await service.initialize();
+      service = new FileDiscoveryService(mockProjectRoot);
     });
 
     it('should filter out git-ignored files by default', () => {
@@ -106,102 +98,22 @@ describe('FileDiscoveryService', () => {
     });
   });
 
-  describe('shouldIgnoreFile', () => {
-    beforeEach(async () => {
+  describe('shouldGitIgnoreFile', () => {
+    beforeEach(() => {
       mockGitIgnoreParser.isIgnored.mockImplementation((path: string) =>
         path.includes('node_modules'),
       );
-      await service.initialize();
+      service = new FileDiscoveryService(mockProjectRoot);
     });
 
     it('should return true for git-ignored files', () => {
-      expect(service.shouldIgnoreFile('node_modules/package/index.js')).toBe(
+      expect(service.shouldGitIgnoreFile('node_modules/package/index.js')).toBe(
         true,
       );
     });
 
     it('should return false for non-ignored files', () => {
-      expect(service.shouldIgnoreFile('src/index.ts')).toBe(false);
-    });
-
-    it('should return false when respectGitIgnore is false', () => {
-      expect(
-        service.shouldIgnoreFile('node_modules/package/index.js', {
-          respectGitIgnore: false,
-        }),
-      ).toBe(false);
-    });
-
-    it('should return false when git ignore parser is not initialized', async () => {
-      const uninitializedService = new FileDiscoveryService(mockProjectRoot);
-      expect(
-        uninitializedService.shouldIgnoreFile('node_modules/package/index.js'),
-      ).toBe(false);
-    });
-  });
-
-  describe('isGitRepository', () => {
-    it('should return true when isGitRepo is explicitly set to true in options', () => {
-      const result = service.isGitRepository({ isGitRepo: true });
-      expect(result).toBe(true);
-    });
-
-    it('should return false when isGitRepo is explicitly set to false in options', () => {
-      const result = service.isGitRepository({ isGitRepo: false });
-      expect(result).toBe(false);
-    });
-
-    it('should use git utility function when isGitRepo is not specified', () => {
-      const result = service.isGitRepository();
-      expect(result).toBe(true); // mocked to return true
-    });
-
-    it('should use git utility function when options are undefined', () => {
-      const result = service.isGitRepository(undefined);
-      expect(result).toBe(true); // mocked to return true
-    });
-  });
-
-  describe('initialization with isGitRepo config', () => {
-    it('should initialize git ignore parser when isGitRepo is true in options', async () => {
-      await service.initialize({ isGitRepo: true });
-
-      expect(GitIgnoreParser).toHaveBeenCalledWith(mockProjectRoot);
-      expect(mockGitIgnoreParser.initialize).toHaveBeenCalled();
-    });
-
-    it('should not initialize git ignore parser when isGitRepo is false in options', async () => {
-      await service.initialize({ isGitRepo: false });
-
-      expect(GitIgnoreParser).not.toHaveBeenCalled();
-      expect(mockGitIgnoreParser.initialize).not.toHaveBeenCalled();
-    });
-
-    it('should initialize git ignore parser when isGitRepo is not specified but respectGitIgnore is true', async () => {
-      await service.initialize({ respectGitIgnore: true });
-
-      expect(GitIgnoreParser).toHaveBeenCalledWith(mockProjectRoot);
-      expect(mockGitIgnoreParser.initialize).toHaveBeenCalled();
-    });
-  });
-
-  describe('shouldIgnoreFile with isGitRepo config', () => {
-    it('should respect isGitRepo option when checking if file should be ignored', async () => {
-      mockGitIgnoreParser.isIgnored.mockImplementation((path: string) =>
-        path.includes('node_modules'),
-      );
-      await service.initialize({ isGitRepo: true });
-
-      expect(
-        service.shouldIgnoreFile('node_modules/package/index.js', {
-          isGitRepo: true,
-        }),
-      ).toBe(true);
-      expect(
-        service.shouldIgnoreFile('node_modules/package/index.js', {
-          isGitRepo: false,
-        }),
-      ).toBe(false);
+      expect(service.shouldGitIgnoreFile('src/index.ts')).toBe(false);
     });
   });
 
@@ -211,13 +123,7 @@ describe('FileDiscoveryService', () => {
       expect(relativeService).toBeInstanceOf(FileDiscoveryService);
     });
 
-    it('should handle undefined options', async () => {
-      await service.initialize(undefined);
-      expect(GitIgnoreParser).toHaveBeenCalled();
-    });
-
-    it('should handle filterFiles with undefined options', async () => {
-      await service.initialize();
+    it('should handle filterFiles with undefined options', () => {
       const files = ['src/index.ts'];
       const filtered = service.filterFiles(files, undefined);
       expect(filtered).toEqual(files);