Add unit tests

Author: mguzmanm

src/views/solution-outline/commands/merge-command.test.ts

@@ -29,6 +29,7 @@ import { COutlineItem } from '../tree-structure/solution-outline-item';
 import * as fs from 'fs';
 import * as child_process from 'child_process';
 import * as os from 'os';
+import * as path from 'path';
 
 jest.mock('fs');
 jest.mock('child_process');
@@ -46,6 +47,7 @@ describe('MergeCommand', () => {
     const mockedFs = fs as jest.Mocked<typeof fs>;
     const mockedExec = child_process.exec as jest.MockedFunction<typeof child_process.exec>;
     const mockedExecSync = child_process.execSync as jest.MockedFunction<typeof child_process.execSync>;
+    const mockedPath = path as jest.Mocked<typeof path>;
 
     beforeEach(async () => {
         commandsProvider = commandsProviderFactory();
@@ -91,6 +93,27 @@ describe('MergeCommand', () => {
         expect(showErrorMessageSpy).toHaveBeenCalledWith('Required local file is missing to perform merge.');
     });
 
+    it('shows error if update file attribute is missing', async () => {
+        const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
+        const node = new COutlineItem('file');
+        node.setAttribute('local', '/tmp/local.c');
+
+        await command['runVSCodeMerge'](node);
+
+        expect(showErrorMessageSpy).toHaveBeenCalledWith('Required update file is missing to perform merge.');
+    });
+
+    it('shows error if base file attribute is missing', async () => {
+        const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
+        const node = new COutlineItem('file');
+        node.setAttribute('local', '/tmp/local.c');
+        node.setAttribute('update', '/tmp/update.c');
+
+        await command['runVSCodeMerge'](node);
+
+        expect(showErrorMessageSpy).toHaveBeenCalledWith('Required base file is missing to perform merge.');
+    });
+
     it('shows error if VS Code executable not found', async () => {
         jest.spyOn(os, 'platform').mockReturnValue('linux');
         mockedExecSync.mockImplementation(() => {
@@ -115,4 +138,111 @@ describe('MergeCommand', () => {
         await command['runVSCodeMerge'](fileNode);
         expect(errorSpy).toHaveBeenCalledWith('Merge operations failed:', expect.any(Error));
     });
+
+    it('warns and skips post-merge file operations on non-zero merge exit code', async () => {
+        const commandPrivate = command as unknown as {
+            getVSCodeExecutablePath: () => string | undefined;
+            doOpen3WayMerge: (cmd: string) => Promise<number>;
+        };
+        jest.spyOn(commandPrivate, 'getVSCodeExecutablePath').mockReturnValue('/usr/bin/code');
+        jest.spyOn(commandPrivate, 'doOpen3WayMerge').mockResolvedValue(1);
+        mockedPath.resolve.mockImplementation((p: string) => p);
+        mockedPath.isAbsolute.mockReturnValue(true);
+        mockedFs.copyFileSync.mockImplementation(() => { });
+        mockedFs.existsSync.mockReturnValue(true);
+        mockedFs.statSync.mockReturnValue({ mtimeMs: 1000 } as fs.Stats);
+
+        const warningSpy = jest.spyOn(console, 'warn').mockImplementation(() => { });
+
+        await command['runVSCodeMerge'](fileNode);
+
+        expect(warningSpy).toHaveBeenCalledWith('Merge exited with code 1. Conflicts may exist.');
+        expect(mockedFs.unlinkSync).not.toHaveBeenCalled();
+        expect(mockedFs.renameSync).not.toHaveBeenCalled();
+        expect(activeSolutionTracker.triggerReload).not.toHaveBeenCalled();
+    });
+
+    it('performs post-merge file operations and triggers reload when merged file changes', async () => {
+        const commandPrivate = command as unknown as {
+            getVSCodeExecutablePath: () => string | undefined;
+            doOpen3WayMerge: (cmd: string) => Promise<number>;
+        };
+        jest.spyOn(commandPrivate, 'getVSCodeExecutablePath').mockReturnValue('/usr/bin/code');
+        jest.spyOn(commandPrivate, 'doOpen3WayMerge').mockResolvedValue(0);
+        mockedPath.resolve.mockImplementation((p: string) => p);
+        mockedPath.isAbsolute.mockReturnValue(true);
+        mockedPath.basename.mockReturnValue('component.update.c');
+        mockedPath.dirname.mockReturnValue('/tmp');
+        mockedPath.join.mockReturnValue('/tmp/component.base.c');
+        mockedFs.copyFileSync.mockImplementation(() => { });
+        mockedFs.existsSync.mockReturnValue(true);
+        mockedFs.statSync
+            .mockReturnValueOnce({ mtimeMs: 1000 } as fs.Stats)
+            .mockReturnValueOnce({ mtimeMs: 2000 } as fs.Stats);
+
+        await command['runVSCodeMerge'](fileNode);
+
+        expect(mockedFs.copyFileSync).toHaveBeenCalledWith('localPath', 'localPath.merged');
+        expect(mockedFs.copyFileSync).toHaveBeenCalledWith('localPath', 'localPath.bak');
+        expect(mockedFs.unlinkSync).toHaveBeenCalledWith('localPath');
+        expect(mockedFs.unlinkSync).toHaveBeenCalledWith('basePath');
+        expect(mockedFs.renameSync).toHaveBeenCalledWith('updatePath', '/tmp/component.base.c');
+        expect(mockedFs.renameSync).toHaveBeenCalledWith('localPath.merged', 'localPath');
+        expect(activeSolutionTracker.triggerReload).toHaveBeenCalledTimes(1);
+    });
+
+    it('builds merge command with validated absolute paths', () => {
+        mockedPath.isAbsolute.mockReturnValue(true);
+
+        const result = command['buildMergeCommand'](
+            '/usr/bin/code',
+            '/tmp/local.c',
+            '/tmp/update.c',
+            '/tmp/base.c',
+            '/tmp/local.c.merged',
+        );
+
+        expect(result).toEqual('"/usr/bin/code" --wait --merge "/tmp/local.c" "/tmp/update.c" "/tmp/base.c" "/tmp/local.c.merged"');
+    });
+
+    it('throws for non-absolute merge paths', () => {
+        mockedPath.isAbsolute.mockReturnValue(false);
+
+        expect(() => command['assertMergeFilePath']('relative/path', 'local file')).toThrow('Invalid local file: path must be absolute.');
+    });
+
+    it('throws for shell-sensitive characters in merge paths', () => {
+        mockedPath.isAbsolute.mockReturnValue(true);
+
+        expect(() => command['assertMergeFilePath']('C:/safe/path&bad', 'local file')).toThrow('Invalid local file: contains unsupported shell-sensitive characters.');
+    });
+
+    it('throws for double quotes in merge paths', () => {
+        mockedPath.isAbsolute.mockReturnValue(true);
+
+        expect(() => command['assertMergeFilePath']('C:/safe/"quoted"/path', 'local file')).toThrow('Invalid local file: contains unsupported shell-sensitive characters.');
+    });
+
+    it('throws for single quotes in merge paths', () => {
+        mockedPath.isAbsolute.mockReturnValue(true);
+
+        expect(() => command['assertMergeFilePath']("C:/safe/'quoted'/path", 'local file')).toThrow('Invalid local file: contains unsupported shell-sensitive characters.');
+    });
+
+    it.each([
+        ['ampersand', 'C:/safe/path&bad'],
+        ['pipe', 'C:/safe/path|bad'],
+        ['input redirection', 'C:/safe/path<bad'],
+        ['output redirection', 'C:/safe/path>bad'],
+        ['caret', 'C:/safe/path^bad'],
+        ['percent', 'C:/safe/path%bad'],
+        ['double quote', 'C:/safe/path"bad'],
+        ['single quote', "C:/safe/path'bad"],
+        ['line feed', 'C:/safe/path\nbad'],
+        ['carriage return', 'C:/safe/path\rbad'],
+    ])('rejects shell-sensitive edge case: %s', (_label, filePath) => {
+        mockedPath.isAbsolute.mockReturnValue(true);
+
+        expect(() => command['assertMergeFilePath'](filePath, 'local file')).toThrow('Invalid local file: contains unsupported shell-sensitive characters.');
+    });
 });

src/views/solution-outline/commands/merge-command.ts

@@ -26,7 +26,7 @@ import { ActiveSolutionTracker } from '../../../solutions/active-solution-tracke
 
 export class MergeCommand {
     public static readonly mergeFile = `${PACKAGE_NAME}.mergeFile`;
-    private static readonly disallowedCmdChars  = /[\r\n&|<>^%]/;
+    private static readonly disallowedCmdChars  = /[\r\n&|<>^%"']/;
 
     constructor(
         private readonly commandsProvider: CommandsProvider,