Address reviewer feedback: update stash API usage and remove ineffective tests

Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>

Author: Copilot

TESTING.md

@@ -236,6 +236,10 @@ export interface Repository {
 	commit(message: string, opts?: CommitOptions): Promise<void>;
 	merge(ref: string): Promise<void>;
 	mergeAbort(): Promise<void>;
+	
+	stash(message?: string, includeUntracked?: boolean): Promise<void>;
+	stashPop(index?: number): Promise<void>;
+	stashApply(index?: number): Promise<void>;
 }
 
 export interface RemoteSource {

src/@types/git.d.ts

@@ -209,6 +209,10 @@ export interface Repository {
 	add(paths: string[]): Promise<void>;
 	merge(ref: string): Promise<void>;
 	mergeAbort(): Promise<void>;
+	
+	stash(message?: string, includeUntracked?: boolean): Promise<void>;
+	stashPop(index?: number): Promise<void>;
+	stashApply(index?: number): Promise<void>;
 }
 
 /**

src/api/api.d.ts

@@ -87,7 +87,7 @@ async function handleUncommittedChanges(repository: Repository): Promise<boolean
 			];
 			if (allChangedFiles.length > 0) {
 				await repository.add(allChangedFiles);
-				await vscode.commands.executeCommand('git.stash', repository);
+				await repository.stash(vscode.l10n.t('Uncommitted changes from PR checkout'));
 			}
 		} else if (modalResult === DISCARD_CHANGES) {
 			// Discard all working tree changes

src/commands.ts

@@ -3,89 +3,10 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { default as assert } from 'assert';
-import * as vscode from 'vscode';
-import { SinonSandbox, createSandbox, SinonStub } from 'sinon';
-import { Status } from '../api/api1';
-import { MockRepository } from './mocks/mockRepository';
-
-// Import the function under test - we need to export it from commands.ts for testing
-// For now, we'll test the integration by checking the mock calls
-
-describe('Commands', function () {
-	let sinon: SinonSandbox;
-	let showInformationMessageStub: SinonStub;
-
-	beforeEach(function () {
-		sinon = createSandbox();
-		showInformationMessageStub = sinon.stub(vscode.window, 'showInformationMessage');
-	});
-
-	afterEach(function () {
-		sinon.restore();
-	});
-
-	describe('handleUncommittedChanges', function () {
-		it('should return true when there are no uncommitted changes', async function () {
-			const repository = new MockRepository();
-			// Default state has no changes
-			
-			// Since we can't directly test the function (it's not exported), 
-			// we test the behavior by checking that showInformationMessage is not called
-			// This is a minimal test to verify the basic logic
-			const hasWorkingTreeChanges = repository.state.workingTreeChanges.length > 0;
-			const hasIndexChanges = repository.state.indexChanges.length > 0;
-			
-			assert.strictEqual(hasWorkingTreeChanges, false);
-			assert.strictEqual(hasIndexChanges, false);
-		});
-
-		it('should detect working tree changes', function () {
-			const repository = new MockRepository();
-			
-			// Add a working tree change
-			repository.state.workingTreeChanges.push({
-				uri: vscode.Uri.file('/test/file.txt'),
-				originalUri: vscode.Uri.file('/test/file.txt'),
-				renameUri: undefined,
-				status: Status.MODIFIED,
-			});
-
-			const hasWorkingTreeChanges = repository.state.workingTreeChanges.length > 0;
-			assert.strictEqual(hasWorkingTreeChanges, true);
-		});
-
-		it('should detect index changes', function () {
-			const repository = new MockRepository();
-			
-			// Add an index change
-			repository.state.indexChanges.push({
-				uri: vscode.Uri.file('/test/file.txt'),
-				originalUri: vscode.Uri.file('/test/file.txt'),
-				renameUri: undefined,
-				status: Status.MODIFIED,
-			});
-
-			const hasIndexChanges = repository.state.indexChanges.length > 0;
-			assert.strictEqual(hasIndexChanges, true);
-		});
-
-		it('should properly mock repository.add and repository.clean methods', async function () {
-			const repository = new MockRepository();
-			
-			// Mock the add method to succeed
-			sinon.stub(repository, 'add').resolves();
-			
-			// Mock the clean method to succeed
-			sinon.stub(repository, 'clean').resolves();
-
-			// Test that the mocked methods work
-			await repository.add(['/test/file.txt']);
-			await repository.clean(['/test/file.txt']);
-			
-			// Verify the methods were called
-			assert.ok((repository.add as SinonStub).calledOnce);
-			assert.ok((repository.clean as SinonStub).calledOnce);
-		});
-	});
-});
+// This file was created to test the handleUncommittedChanges functionality,
+// but the tests were removed as they only tested mocking capabilities
+// rather than meaningful functionality.
+//
+// The handleUncommittedChanges function is tested through manual testing
+// as documented in TESTING.md, since it primarily involves user interactions
+// with modal dialogs and git operations that are better tested in integration.

src/test/commands.test.ts

@@ -331,4 +331,19 @@ export class MockRepository implements Repository {
 	mergeAbort(): Promise<void> {
 		return Promise.reject(new Error(`Unexpected mergeAbort`));
 	}
+
+	async stash(message?: string, includeUntracked?: boolean): Promise<void> {
+		// Mock implementation for stash
+		return Promise.resolve();
+	}
+
+	async stashPop(index?: number): Promise<void> {
+		// Mock implementation for stash pop
+		return Promise.resolve();
+	}
+
+	async stashApply(index?: number): Promise<void> {
+		// Mock implementation for stash apply
+		return Promise.resolve();
+	}
 }