Skip to content

Commit 5896357

Browse files
mguzmanmjkrech
andauthored
Refresh CMSIS and Problems view after saving merged file (#215)
* Refresh CMSIS and Problems view after saving merged file * Refresh non-destructively on save and harden session lifecycle * Use REFRESH_COMMAND_ID * Add a messege to inform user Merge view can be closed after merge --------- Co-authored-by: Joachim Krech <8290187+jkrech@users.noreply.github.com>
1 parent b7db22d commit 5896357

5 files changed

Lines changed: 425 additions & 83 deletions

File tree

src/desktop/extension.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ import { EditCommand } from '../views/solution-outline/commands/edit-command';
5959
import { OpenCommand } from '../views/solution-outline/commands/open-command';
6060
import { FindCommand } from '../views/solution-outline/commands/find-command';
6161
import { MergeCommand } from '../views/solution-outline/commands/merge-command';
62+
import { MergeSessionCoordinatorImpl } from '../views/solution-outline/commands/merge-session-coordinator';
6263
import { SolutionOutlineView } from '../views/solution-outline/solution-outline';
6364
import { TreeViewFileDecorationProvider } from '../views/solution-outline/treeview-decoration-provider';
6465
import { TreeViewProviderImpl } from '../views/solution-outline/treeview-provider';
@@ -220,7 +221,8 @@ export const activate = async (context: ExtensionContext): Promise<CsolutionExte
220221
const fileDecorationProviderManager = new FileDecorationProviderManagerImpl();
221222
const treeViewProviderImpl = new TreeViewProviderImpl(SolutionOutlineView.treeViewId);
222223
const treeViewFileDecorationProvider = new TreeViewFileDecorationProvider(fileDecorationProviderManager, themeProvider);
223-
const mergeCommand = new MergeCommand(commandsProvider);
224+
const mergeSessionCoordinator = new MergeSessionCoordinatorImpl(commandsProvider);
225+
const mergeCommand = new MergeCommand(commandsProvider, mergeSessionCoordinator, messageProvider);
224226
const buildCommand = new BuildCommand(buildTaskProvider, commandsProvider, buildTaskDefinitionBuilder);
225227
const runGeneratorCommand = new GeneratorCommand(commandsProvider, solutionManager, outputChannelProvider, cmsisToolboxManager);
226228
const armclangDefineGetter = new ArmclangDefineGetter(processManager, workspaceFsProvider);

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

Lines changed: 53 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
jest.mock('vscode', () => ({
1818
window: {
1919
showErrorMessage: jest.fn(),
20+
showInformationMessage: jest.fn(),
2021
showWarningMessage: jest.fn(),
2122
}
2223
}));
@@ -32,23 +33,25 @@ jest.mock('path', () => {
3233
});
3334

3435
import { TestDataHandler } from '../../../__test__/test-data';
35-
import * as vscode from 'vscode';
3636
import { extensionContextFactory } from '../../../vscode-api/extension-context.factories';
3737
import { commandsProviderFactory, MockCommandsProvider } from '../../../vscode-api/commands-provider.factories';
3838
import { MergeCommand } from './merge-command';
39-
import * as manifest from '../../../manifest';
4039
import { COutlineItem } from '../tree-structure/solution-outline-item';
4140
import * as child_process from 'child_process';
4241
import * as os from 'os';
4342
import * as path from 'path';
4443
import * as fsUtils from '../../../utils/fs-utils';
44+
import { MergeSessionCoordinator } from './merge-session-coordinator';
45+
import { messageProviderFactory, MockMessageProvider } from '../../../vscode-api/message-provider.factories';
4546

4647
jest.mock('child_process');
4748
jest.mock('os');
4849

4950
describe('MergeCommand', () => {
5051
let commandsProvider: MockCommandsProvider;
5152
let command: MergeCommand;
53+
let mergeSessionCoordinator: jest.Mocked<MergeSessionCoordinator>;
54+
let messageProvider: MockMessageProvider;
5255
const testDataHandler = new TestDataHandler();
5356
let tmpDir: string;
5457

@@ -79,7 +82,15 @@ describe('MergeCommand', () => {
7982
fsUtils.writeTextFile(path.join(tmpDir, 'component.c.base@1.0.0'), '// base\n');
8083

8184
commandsProvider = commandsProviderFactory();
82-
command = new MergeCommand(commandsProvider);
85+
mergeSessionCoordinator = {
86+
activate: jest.fn().mockResolvedValue(),
87+
onMergeApplied: jest.fn().mockReturnValue({ dispose: jest.fn() }),
88+
startSession: jest.fn(),
89+
cancelSession: jest.fn(),
90+
onMergeProcessExit: jest.fn().mockResolvedValue({ applied: false }),
91+
};
92+
messageProvider = messageProviderFactory();
93+
command = new MergeCommand(commandsProvider, mergeSessionCoordinator, messageProvider);
8394

8495
componentNode = new COutlineItem('component');
8596
componentNode.setTag('component');
@@ -98,10 +109,24 @@ describe('MergeCommand', () => {
98109
it('registers the command on activation', async () => {
99110
await command.activate(extensionContextFactory());
100111

112+
expect(mergeSessionCoordinator.activate).toHaveBeenCalledTimes(1);
113+
expect(mergeSessionCoordinator.onMergeApplied).toHaveBeenCalledTimes(1);
101114
expect(commandsProvider.registerCommand).toHaveBeenCalledTimes(1);
102115
expect(commandsProvider.registerCommand).toHaveBeenCalledWith(MergeCommand.mergeFile, expect.any(Function), expect.anything());
103116
});
104117

118+
it('shows success info when coordinator emits merge-applied event', async () => {
119+
await command.activate(extensionContextFactory());
120+
121+
const listener = mergeSessionCoordinator.onMergeApplied.mock.calls[0]?.[0] as (() => void) | undefined;
122+
expect(listener).toBeDefined();
123+
listener?.();
124+
125+
expect(messageProvider.showInformationMessage).toHaveBeenCalledWith(
126+
'Merge applied successfully. Merge View can be closed.',
127+
);
128+
});
129+
105130
it('forwards string command argument to the merge handler', async () => {
106131
const runVSCodeMergeFromPathSpy = jest.spyOn(
107132
command as unknown as { runVSCodeMergeFromPath: (localPath: string) => Promise<void> },
@@ -128,22 +153,18 @@ describe('MergeCommand', () => {
128153
});
129154

130155
it('shows error for unsupported command argument type', async () => {
131-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
132-
133156
await command.activate(extensionContextFactory());
134157
await commandsProvider.mockRunRegistered(MergeCommand.mergeFile, undefined);
135158

136-
expect(showErrorMessageSpy).toHaveBeenCalledWith('Cannot open merge view: unsupported command argument.');
159+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('Cannot open merge view: unsupported command argument.');
137160
});
138161
});
139162

140163
describe('merge file discovery', () => {
141164
it('shows error if merge path is missing when invoked from path', async () => {
142-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
143-
144165
await command['runVSCodeMergeFromPath']('');
145166

146-
expect(showErrorMessageSpy).toHaveBeenCalledWith('Cannot open merge view: merge file path is missing.');
167+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('Cannot open merge view: merge file path is missing.');
147168
});
148169

149170
it('normalizes the merge path before opening merge view from path', async () => {
@@ -159,17 +180,15 @@ describe('MergeCommand', () => {
159180
});
160181

161182
it('shows error if node is not passed', async () => {
162-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
163183
// @ts-expect-error - testing behavior when `runVSCodeMerge` receives null
164184
await command['runVSCodeMerge'](null);
165-
expect(showErrorMessageSpy).toHaveBeenCalledWith('File data is not available for merge operation.');
185+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('File data is not available for merge operation.');
166186
});
167187

168188
it('shows error if required local file is missing', async () => {
169-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
170189
const node = new COutlineItem('file');
171190
await command['runVSCodeMerge'](node);
172-
expect(showErrorMessageSpy).toHaveBeenCalledWith('Required local file is missing to perform merge.');
191+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('Required local file is missing to perform merge.');
173192
});
174193

175194
it('finds update and base files using the highest matching semver sibling', () => {
@@ -330,20 +349,17 @@ describe('MergeCommand', () => {
330349
throw new Error('not found');
331350
});
332351

333-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
334352
await command['runVSCodeMerge'](fileNode);
335-
expect(showErrorMessageSpy).toHaveBeenCalledWith('Visual Studio Code executable not found. Please ensure it is installed and available in your PATH.');
353+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('Visual Studio Code executable not found. Please ensure it is installed and available in your PATH.');
336354
});
337355
});
338356

339357
describe('merge execution flow', () => {
340-
it('warns and skips post-merge file operations on non-zero merge exit code', async () => {
358+
it('warns and delegates process exit handling on non-zero merge exit code', async () => {
341359
const commandPrivate = command as unknown as {
342360
getVSCodeExecutablePath: () => string | undefined;
343361
doOpen3WayMerge: (cmd: string) => Promise<number>;
344362
};
345-
const deleteFileIfExistsSpy = jest.spyOn(fsUtils, 'deleteFileIfExists');
346-
const renameFileSpy = jest.spyOn(fsUtils, 'renameFile');
347363
jest.spyOn(fsUtils, 'copyFile').mockImplementation(() => { });
348364
jest.spyOn(fsUtils, 'getFileModificationTime').mockReturnValue(1000);
349365
jest.spyOn(commandPrivate, 'getVSCodeExecutablePath').mockReturnValue('/usr/bin/code');
@@ -354,9 +370,9 @@ describe('MergeCommand', () => {
354370
await command['runVSCodeMerge'](fileNode);
355371

356372
expect(warningSpy).toHaveBeenCalledWith('Merge exited with code 1. Conflicts may exist.');
357-
expect(deleteFileIfExistsSpy).not.toHaveBeenCalled();
358-
expect(renameFileSpy).not.toHaveBeenCalled();
359-
expect(commandsProvider.executeCommand).not.toHaveBeenCalledWith(manifest.REFRESH_COMMAND_ID);
373+
expect(mergeSessionCoordinator.startSession).toHaveBeenCalledTimes(1);
374+
expect(mergeSessionCoordinator.onMergeProcessExit).toHaveBeenCalledWith(1);
375+
expect(mergeSessionCoordinator.cancelSession).not.toHaveBeenCalled();
360376
});
361377

362378
it('handles merge errors gracefully', async () => {
@@ -369,10 +385,10 @@ describe('MergeCommand', () => {
369385
mockedExec.mockImplementation((_cmd, _cb) => { throw new Error('unexpected'); });
370386

371387
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => { });
372-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
373388
await command['runVSCodeMerge'](fileNode);
374389
expect(errorSpy).toHaveBeenCalledWith('Merge operations failed:', expect.any(Error));
375-
expect(showErrorMessageSpy).toHaveBeenCalledWith('Merge operation failed: unexpected');
390+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('Merge operation failed: unexpected');
391+
expect(mergeSessionCoordinator.cancelSession).toHaveBeenCalledTimes(1);
376392
});
377393

378394
it('shows a merge failure message when merge command validation throws', async () => {
@@ -393,19 +409,17 @@ describe('MergeCommand', () => {
393409
jest.spyOn(fsUtils, 'copyFile').mockImplementation(() => { });
394410
jest.spyOn(fsUtils, 'getFileModificationTime').mockReturnValue(1000);
395411

396-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
397-
398412
await command['runVSCodeMerge'](node);
399413

400-
expect(showErrorMessageSpy).toHaveBeenCalledWith('Merge operation failed: Invalid update file: contains unsupported shell-sensitive characters.');
414+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('Merge operation failed: Invalid update file: contains unsupported shell-sensitive characters.');
415+
expect(mergeSessionCoordinator.cancelSession).toHaveBeenCalledTimes(1);
401416
});
402417

403-
it('performs post-merge file operations and triggers reload when merged file changes', async () => {
418+
it('starts merge session and notifies coordinator when merge exits successfully', async () => {
404419
const local = path.join(tmpDir, 'component.c');
405420
const update = path.join(tmpDir, 'component.c.update@1.0.0');
406421
const base = path.join(tmpDir, 'component.c.base@1.0.0');
407422
const merged = `${local}.merged`;
408-
const expectedBase = path.join(path.dirname(update), path.basename(update).replaceAll('update', 'base'));
409423
const node = new COutlineItem('file');
410424
node.setTag('file');
411425
node.setAttribute('label', 'Component X');
@@ -417,24 +431,23 @@ describe('MergeCommand', () => {
417431
doOpen3WayMerge: (cmd: string) => Promise<number>;
418432
};
419433
const copyFileSpy = jest.spyOn(fsUtils, 'copyFile').mockImplementation(() => { });
420-
const getFileModificationTimeSpy = jest.spyOn(fsUtils, 'getFileModificationTime')
421-
.mockReturnValueOnce(1000)
422-
.mockReturnValueOnce(2000);
423-
const deleteFileIfExistsSpy = jest.spyOn(fsUtils, 'deleteFileIfExists').mockImplementation(() => { });
424-
const renameFileSpy = jest.spyOn(fsUtils, 'renameFile').mockImplementation(() => { });
434+
jest.spyOn(fsUtils, 'getFileModificationTime').mockReturnValue(1000);
425435
jest.spyOn(commandPrivate, 'getVSCodeExecutablePath').mockReturnValue('/usr/bin/code');
426436
jest.spyOn(commandPrivate, 'doOpen3WayMerge').mockResolvedValue(0);
427437

428438
await command['runVSCodeMerge'](node);
429439

430440
expect(copyFileSpy).toHaveBeenCalledWith(local, merged);
431-
expect(copyFileSpy).toHaveBeenCalledWith(local, `${local}.bak`);
432-
expect(getFileModificationTimeSpy).toHaveBeenCalledTimes(2);
433-
expect(deleteFileIfExistsSpy).toHaveBeenCalledWith(local);
434-
expect(deleteFileIfExistsSpy).toHaveBeenCalledWith(base);
435-
expect(renameFileSpy).toHaveBeenCalledWith(update, expectedBase);
436-
expect(renameFileSpy).toHaveBeenCalledWith(merged, local);
437-
expect(commandsProvider.executeCommand).toHaveBeenCalledWith(manifest.REFRESH_COMMAND_ID);
441+
expect(mergeSessionCoordinator.startSession).toHaveBeenCalledWith({
442+
local,
443+
update,
444+
base,
445+
merged,
446+
mergedMTimeBefore: 1000,
447+
});
448+
expect(mergeSessionCoordinator.onMergeProcessExit).toHaveBeenCalledWith(0);
449+
expect(mergeSessionCoordinator.cancelSession).not.toHaveBeenCalled();
438450
});
451+
439452
});
440453
});

0 commit comments

Comments
 (0)