Run presubmit tests in windows as well as linux. (#4672)

Co-authored-by: matt korwel <matt.korwel@gmail.com>

.github/actions/post-coverage-comment/action.yml

@@ -17,6 +17,9 @@ inputs:
   node_version:
     description: 'Node.js version for context in messages'
     required: true
+  os:
+    description: 'The os for context in messages'
+    required: true
   github_token:
     description: 'GitHub token for posting comments'
     required: true
@@ -91,7 +94,7 @@ runs:
         echo "</details>" >> "$comment_file"
         echo "" >> "$comment_file"
 
-        echo "_For detailed HTML reports, please see the 'coverage-reports-${{ inputs.node_version }}' artifact from the main CI run._" >> "$comment_file"
+        echo "_For detailed HTML reports, please see the 'coverage-reports-${{ inputs.node_version }}-${{ inputs.os }}' artifact from the main CI run._" >> "$comment_file"
 
     - name: Post Coverage Comment
       uses: thollander/actions-comment-pull-request@v3

.github/workflows/ci.yml

@@ -44,7 +44,7 @@ jobs:
 
   test:
     name: Test
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
     needs: lint
     permissions:
       contents: read
@@ -52,6 +52,7 @@ jobs:
       pull-requests: write
     strategy:
       matrix:
+        os: [ubuntu-latest, windows-latest]
         node-version: [20.x, 22.x, 24.x]
     steps:
       - name: Checkout repository
@@ -70,7 +71,9 @@ jobs:
         run: npm ci # Install fresh dependencies using the downloaded package-lock.json
 
       - name: Run tests and generate reports
-        run: NO_COLOR=true npm run test:ci
+        run: npm run test:ci
+        env:
+          NO_COLOR: true
 
       - name: Publish Test Report (for non-forks)
         if: always() && (github.event.pull_request.head.repo.full_name == github.repository)
@@ -85,14 +88,14 @@ jobs:
         if: always() && (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository)
         uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
         with:
-          name: test-results-fork-${{ matrix.node-version }}
+          name: test-results-fork-${{ matrix.node-version }}-${{ matrix.os }}
           path: packages/*/junit.xml
 
       - name: Upload coverage reports
         uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
         if: always()
         with:
-          name: coverage-reports-${{ matrix.node-version }}
+          name: coverage-reports-${{ matrix.node-version }}-${{ matrix.os }}
           path: packages/*/coverage
 
   post_coverage_comment:
@@ -106,7 +109,9 @@ jobs:
       pull-requests: write # For commenting
     strategy:
       matrix:
-        node-version: [22.x] # Reduce noise by only posting the comment once
+        # Reduce noise by only posting the comment once
+        os: [ubuntu-latest]
+        node-version: [22.x]
     steps:
       - name: Checkout repository
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4
@@ -114,7 +119,7 @@ jobs:
       - name: Download coverage reports artifact
         uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
         with:
-          name: coverage-reports-${{ matrix.node-version }}
+          name: coverage-reports-${{ matrix.node-version }}-${{ matrix.os }}
           path: coverage_artifact # Download to a specific directory
 
       - name: Post Coverage Comment using Composite Action
@@ -125,6 +130,7 @@ jobs:
           cli_full_text_summary_file: coverage_artifact/cli/coverage/full-text-summary.txt
           core_full_text_summary_file: coverage_artifact/core/coverage/full-text-summary.txt
           node_version: ${{ matrix.node-version }}
+          os: ${{ matrix.os }}
           github_token: ${{ secrets.GITHUB_TOKEN }}
 
   codeql:

packages/core/src/services/gitService.test.ts

@@ -7,28 +7,16 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { GitService } from './gitService.js';
 import * as path from 'path';
-import type * as FsPromisesModule from 'fs/promises';
+import * as fs from 'fs/promises';
+import * as os from 'os';
 import type { ChildProcess } from 'node:child_process';
+import { getProjectHash, GEMINI_DIR } from '../utils/paths.js';
 
 const hoistedMockExec = vi.hoisted(() => vi.fn());
 vi.mock('node:child_process', () => ({
   exec: hoistedMockExec,
 }));
 
-const hoistedMockMkdir = vi.hoisted(() => vi.fn());
-const hoistedMockReadFile = vi.hoisted(() => vi.fn());
-const hoistedMockWriteFile = vi.hoisted(() => vi.fn());
-
-vi.mock('fs/promises', async (importOriginal) => {
-  const actual = (await importOriginal()) as typeof FsPromisesModule;
-  return {
-    ...actual,
-    mkdir: hoistedMockMkdir,
-    readFile: hoistedMockReadFile,
-    writeFile: hoistedMockWriteFile,
-  };
-});
-
 const hoistedMockEnv = vi.hoisted(() => vi.fn());
 const hoistedMockSimpleGit = vi.hoisted(() => vi.fn());
 const hoistedMockCheckIsRepo = vi.hoisted(() => vi.fn());
@@ -53,38 +41,30 @@ vi.mock('../utils/gitUtils.js', () => ({
   isGitRepository: hoistedIsGitRepositoryMock,
 }));
 
-const hoistedMockIsNodeError = vi.hoisted(() => vi.fn());
-vi.mock('../utils/errors.js', () => ({
-  isNodeError: hoistedMockIsNodeError,
-}));
-
 const hoistedMockHomedir = vi.hoisted(() => vi.fn());
-vi.mock('os', () => ({
+vi.mock('os', async (importOriginal) => {
-  homedir: hoistedMockHomedir,
+  const actual = await importOriginal<typeof os>();
-}));
-
-const hoistedMockCreateHash = vi.hoisted(() => {
-  const mockUpdate = vi.fn().mockReturnThis();
-  const mockDigest = vi.fn();
   return {
-    createHash: vi.fn(() => ({
+    ...actual,
-      update: mockUpdate,
+    homedir: hoistedMockHomedir,
-      digest: mockDigest,
-    })),
-    mockUpdate,
-    mockDigest,
   };
 });
-vi.mock('crypto', () => ({
-  createHash: hoistedMockCreateHash.createHash,
-}));
 
 describe('GitService', () => {
-  const mockProjectRoot = '/test/project';
+  let testRootDir: string;
-  const mockHomedir = '/mock/home';
+  let projectRoot: string;
-  const mockHash = 'mock-hash';
+  let homedir: string;
+  let hash: string;
+
+  beforeEach(async () => {
+    testRootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'git-service-test-'));
+    projectRoot = path.join(testRootDir, 'project');
+    homedir = path.join(testRootDir, 'home');
+    await fs.mkdir(projectRoot, { recursive: true });
+    await fs.mkdir(homedir, { recursive: true });
+
+    hash = getProjectHash(projectRoot);
 
-  beforeEach(() => {
     vi.clearAllMocks();
     hoistedIsGitRepositoryMock.mockReturnValue(true);
     hoistedMockExec.mockImplementation((command, callback) => {
@@ -95,13 +75,8 @@ describe('GitService', () => {
       }
       return {};
     });
-    hoistedMockMkdir.mockResolvedValue(undefined);
+
-    hoistedMockReadFile.mockResolvedValue('');
+    hoistedMockHomedir.mockReturnValue(homedir);
-    hoistedMockWriteFile.mockResolvedValue(undefined);
-    hoistedMockIsNodeError.mockImplementation((e) => e instanceof Error);
-    hoistedMockHomedir.mockReturnValue(mockHomedir);
-    hoistedMockCreateHash.mockUpdate.mockReturnThis();
-    hoistedMockCreateHash.mockDigest.mockReturnValue(mockHash);
 
     hoistedMockEnv.mockImplementation(() => ({
       checkIsRepo: hoistedMockCheckIsRepo,
@@ -127,19 +102,20 @@ describe('GitService', () => {
     });
   });
 
-  afterEach(() => {
+  afterEach(async () => {
     vi.restoreAllMocks();
+    await fs.rm(testRootDir, { recursive: true, force: true });
   });
 
   describe('constructor', () => {
-    it('should successfully create an instance if projectRoot is a Git repository', () => {
+    it('should successfully create an instance', () => {
-      expect(() => new GitService(mockProjectRoot)).not.toThrow();
+      expect(() => new GitService(projectRoot)).not.toThrow();
     });
   });
 
   describe('verifyGitAvailability', () => {
     it('should resolve true if git --version command succeeds', async () => {
-      const service = new GitService(mockProjectRoot);
+      const service = new GitService(projectRoot);
       await expect(service.verifyGitAvailability()).resolves.toBe(true);
     });
 
@@ -148,7 +124,7 @@ describe('GitService', () => {
         callback(new Error('git not found'));
         return {} as ChildProcess;
       });
-      const service = new GitService(mockProjectRoot);
+      const service = new GitService(projectRoot);
       await expect(service.verifyGitAvailability()).resolves.toBe(false);
     });
   });
@@ -159,14 +135,14 @@ describe('GitService', () => {
         callback(new Error('git not found'));
         return {} as ChildProcess;
       });
-      const service = new GitService(mockProjectRoot);
+      const service = new GitService(projectRoot);
       await expect(service.initialize()).rejects.toThrow(
         'Checkpointing is enabled, but Git is not installed. Please install Git or disable checkpointing to continue.',
       );
     });
 
     it('should call setupShadowGitRepository if Git is available', async () => {
-      const service = new GitService(mockProjectRoot);
+      const service = new GitService(projectRoot);
       const setupSpy = vi
         .spyOn(service, 'setupShadowGitRepository')
         .mockResolvedValue(undefined);
@@ -177,33 +153,34 @@ describe('GitService', () => {
   });
 
   describe('setupShadowGitRepository', () => {
-    const repoDir = path.join(mockHomedir, '.gemini', 'history', mockHash);
+    let repoDir: string;
-    const hiddenGitIgnorePath = path.join(repoDir, '.gitignore');
+    let gitConfigPath: string;
-    const visibleGitIgnorePath = path.join(mockProjectRoot, '.gitignore');
-    const gitConfigPath = path.join(repoDir, '.gitconfig');
 
-    it('should create a .gitconfig file with the correct content', async () => {
+    beforeEach(() => {
-      const service = new GitService(mockProjectRoot);
+      repoDir = path.join(homedir, GEMINI_DIR, 'history', hash);
-      await service.setupShadowGitRepository();
+      gitConfigPath = path.join(repoDir, '.gitconfig');
-      const expectedConfigContent =
-        '[user]\n  name = Gemini CLI\n  email = gemini-cli@google.com\n[commit]\n  gpgsign = false\n';
-      expect(hoistedMockWriteFile).toHaveBeenCalledWith(
-        gitConfigPath,
-        expectedConfigContent,
-      );
     });
 
     it('should create history and repository directories', async () => {
-      const service = new GitService(mockProjectRoot);
+      const service = new GitService(projectRoot);
       await service.setupShadowGitRepository();
-      expect(hoistedMockMkdir).toHaveBeenCalledWith(repoDir, {
+      const stats = await fs.stat(repoDir);
-        recursive: true,
+      expect(stats.isDirectory()).toBe(true);
-      });
+    });
+
+    it('should create a .gitconfig file with the correct content', async () => {
+      const service = new GitService(projectRoot);
+      await service.setupShadowGitRepository();
+
+      const expectedConfigContent =
+        '[user]\n  name = Gemini CLI\n  email = gemini-cli@google.com\n[commit]\n  gpgsign = false\n';
+      const actualConfigContent = await fs.readFile(gitConfigPath, 'utf-8');
+      expect(actualConfigContent).toBe(expectedConfigContent);
     });
 
     it('should initialize git repo in historyDir if not already initialized', async () => {
       hoistedMockCheckIsRepo.mockResolvedValue(false);
-      const service = new GitService(mockProjectRoot);
+      const service = new GitService(projectRoot);
       await service.setupShadowGitRepository();
       expect(hoistedMockSimpleGit).toHaveBeenCalledWith(repoDir);
       expect(hoistedMockInit).toHaveBeenCalled();
@@ -211,52 +188,49 @@ describe('GitService', () => {
 
     it('should not initialize git repo if already initialized', async () => {
       hoistedMockCheckIsRepo.mockResolvedValue(true);
-      const service = new GitService(mockProjectRoot);
+      const service = new GitService(projectRoot);
       await service.setupShadowGitRepository();
       expect(hoistedMockInit).not.toHaveBeenCalled();
     });
 
     it('should copy .gitignore from projectRoot if it exists', async () => {
-      const gitignoreContent = `node_modules/\n.env`;
+      const gitignoreContent = 'node_modules/\n.env';
-      hoistedMockReadFile.mockImplementation(async (filePath) => {
+      const visibleGitIgnorePath = path.join(projectRoot, '.gitignore');
-        if (filePath === visibleGitIgnorePath) {
+      await fs.writeFile(visibleGitIgnorePath, gitignoreContent);
-          return gitignoreContent;
+
-        }
+      const service = new GitService(projectRoot);
-        return '';
-      });
-      const service = new GitService(mockProjectRoot);
       await service.setupShadowGitRepository();
-      expect(hoistedMockReadFile).toHaveBeenCalledWith(
+
-        visibleGitIgnorePath,
+      const hiddenGitIgnorePath = path.join(repoDir, '.gitignore');
-        'utf-8',
+      const copiedContent = await fs.readFile(hiddenGitIgnorePath, 'utf-8');
-      );
+      expect(copiedContent).toBe(gitignoreContent);
-      expect(hoistedMockWriteFile).toHaveBeenCalledWith(
+    });
-        hiddenGitIgnorePath,
+
-        gitignoreContent,
+    it('should not create a .gitignore in shadow repo if project .gitignore does not exist', async () => {
-      );
+      const service = new GitService(projectRoot);
+      await service.setupShadowGitRepository();
+
+      const hiddenGitIgnorePath = path.join(repoDir, '.gitignore');
+      // An empty string is written if the file doesn't exist.
+      const content = await fs.readFile(hiddenGitIgnorePath, 'utf-8');
+      expect(content).toBe('');
     });
 
     it('should throw an error if reading projectRoot .gitignore fails with other errors', async () => {
-      const readError = new Error('Read permission denied');
+      const visibleGitIgnorePath = path.join(projectRoot, '.gitignore');
-      hoistedMockReadFile.mockImplementation(async (filePath) => {
+      // Create a directory instead of a file to cause a read error
-        if (filePath === visibleGitIgnorePath) {
+      await fs.mkdir(visibleGitIgnorePath);
-          throw readError;
-        }
-        return '';
-      });
-      hoistedMockIsNodeError.mockImplementation(
-        (e: unknown): e is NodeJS.ErrnoException => e instanceof Error,
-      );
 
-      const service = new GitService(mockProjectRoot);
+      const service = new GitService(projectRoot);
+      // EISDIR is the expected error code on Unix-like systems
       await expect(service.setupShadowGitRepository()).rejects.toThrow(
-        'Read permission denied',
+        /EISDIR: illegal operation on a directory, read|EBUSY: resource busy or locked, read/,
       );
     });
 
     it('should make an initial commit if no commits exist in history repo', async () => {
       hoistedMockCheckIsRepo.mockResolvedValue(false);
-      const service = new GitService(mockProjectRoot);
+      const service = new GitService(projectRoot);
       await service.setupShadowGitRepository();
       expect(hoistedMockCommit).toHaveBeenCalledWith('Initial commit', {
         '--allow-empty': null,
@@ -265,7 +239,7 @@ describe('GitService', () => {
 
     it('should not make an initial commit if commits already exist', async () => {
       hoistedMockCheckIsRepo.mockResolvedValue(true);
-      const service = new GitService(mockProjectRoot);
+      const service = new GitService(projectRoot);
       await service.setupShadowGitRepository();
       expect(hoistedMockCommit).not.toHaveBeenCalled();
     });

packages/core/src/tools/shell.test.ts

@@ -142,11 +142,9 @@ describe('ShellTool Bug Reproduction', () => {
     shellTool = new ShellTool(config);
 
     const abortSignal = new AbortController().signal;
-    const result = await shellTool.execute(
+    const command =
-      { command: 'echo "$GEMINI_CLI"' },
+      os.platform() === 'win32' ? 'echo %GEMINI_CLI%' : 'echo "$GEMINI_CLI"';
-      abortSignal,
+    const result = await shellTool.execute({ command }, abortSignal, () => {});
-      () => {},
-    );
 
     expect(result.returnDisplay).toBe('1' + os.EOL);
   });