Skip to content

Commit 9dfc2f6

Browse files
mguzmanmmguzmanm
committed
Update utility functions and update tests
1 parent f80858e commit 9dfc2f6

6 files changed

Lines changed: 58 additions & 73 deletions

File tree

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

Lines changed: 42 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,10 @@ import { COutlineItem } from '../tree-structure/solution-outline-item';
4141
import * as child_process from 'child_process';
4242
import * as os from 'os';
4343
import * as path from 'path';
44-
import * as fsUtilsModule from '../../../utils/fs-utils';
44+
import * as fsUtils from '../../../utils/fs-utils';
4545

4646
jest.mock('child_process');
4747
jest.mock('os');
48-
jest.mock('../../../utils/fs-utils');
4948

5049
describe('MergeCommand', () => {
5150
let commandsProvider: MockCommandsProvider;
@@ -57,11 +56,9 @@ describe('MergeCommand', () => {
5756
let componentNode: COutlineItem;
5857
let fileNode: COutlineItem;
5958

60-
const mockedFsUtils = fsUtilsModule as jest.Mocked<typeof fsUtilsModule>;
6159
const mockedExec = child_process.exec as jest.MockedFunction<typeof child_process.exec>;
6260
const mockedExecSync = child_process.execSync as jest.MockedFunction<typeof child_process.execSync>;
6361
const mockedPath = path as jest.Mocked<typeof path>;
64-
const fsUtils = jest.requireActual('../../../utils/fs-utils') as typeof import('../../../utils/fs-utils');
6562

6663
beforeAll(() => {
6764
tmpDir = testDataHandler.tmpDir;
@@ -72,6 +69,7 @@ describe('MergeCommand', () => {
7269
});
7370

7471
beforeEach(async () => {
72+
jest.restoreAllMocks();
7573
jest.resetAllMocks();
7674
mockedPath.isAbsolute.mockImplementation((filePath: string) => actualPath.isAbsolute(filePath));
7775
testDataHandler.rmTmpDir();
@@ -81,8 +79,6 @@ describe('MergeCommand', () => {
8179
fsUtils.writeTextFile(path.join(tmpDir, 'component.c.update@1.0.0'), '// update\n');
8280
fsUtils.writeTextFile(path.join(tmpDir, 'component.c.base@1.0.0'), '// base\n');
8381

84-
mockedFsUtils.readDirectory.mockImplementation((dir: string) => fsUtils.readDirectory(dir));
85-
8682
commandsProvider = commandsProviderFactory();
8783
activeSolutionTracker = activeSolutionTrackerFactory();
8884
command = new MergeCommand(commandsProvider, activeSolutionTracker);
@@ -128,7 +124,7 @@ describe('MergeCommand', () => {
128124
fsUtils.writeTextFile(path.join(tmpDir, 'component.c.update@invalid'), '// ignored update\n');
129125
fsUtils.writeTextFile(path.join(tmpDir, 'component.c.base@invalid'), '// ignored base\n');
130126

131-
const result = command['findMergeFiles'](path.join(tmpDir, 'component.c'));
127+
const result = command['findNewestMergeFiles'](path.join(tmpDir, 'component.c'));
132128

133129
expect(result).toEqual({
134130
update: path.join(tmpDir, 'component.c.update@1.2.0'),
@@ -137,19 +133,19 @@ describe('MergeCommand', () => {
137133
});
138134

139135
it('returns undefined siblings when reading the local directory fails', () => {
140-
mockedFsUtils.readDirectory.mockImplementation(() => {
136+
jest.spyOn(fsUtils, 'readDirectory').mockImplementation(() => {
141137
throw new Error('directory unavailable');
142138
});
143139

144-
const result = command['findMergeFiles'](path.join(tmpDir, 'component.c'));
140+
const result = command['findNewestMergeFiles'](path.join(tmpDir, 'component.c'));
145141

146142
expect(result).toEqual({ update: undefined, base: undefined });
147143
});
148144
});
149145

150146
describe('merge sibling resolution', () => {
151-
it('resolveMergeSibling filters by prefix and returns the highest valid semver file', () => {
152-
const result = command['resolveMergeSibling']([
147+
it('resolveNewestMergeSibling filters by prefix and returns the highest valid semver file', () => {
148+
const result = command['resolveNewestMergeSibling']([
153149
'component.c.base@1.0.0',
154150
'component.c.update@1.0.0',
155151
'component.c.update@2.0.0',
@@ -160,31 +156,31 @@ describe('MergeCommand', () => {
160156
expect(result).toBe('component.c.update@2.0.0');
161157
});
162158

163-
it('resolveMergeSibling returns undefined when no file matches the prefix', () => {
164-
const result = command['resolveMergeSibling']([
159+
it('resolveNewestMergeSibling returns undefined when no file matches the prefix', () => {
160+
const result = command['resolveNewestMergeSibling']([
165161
'component.c.base@1.0.0',
166162
'other.c.update@2.0.0',
167163
], 'component.c.update@');
168164

169165
expect(result).toBeUndefined();
170166
});
171167

172-
it('selectMergeSibling ignores non-semver suffixes and picks the latest version', () => {
173-
const result = command['selectMergeSibling']([
168+
it('selectNewestMergeSibling ignores non-semver suffixes and picks the latest version', () => {
169+
const result = command['selectNewestMergeSibling']([
174170
'component.c.update@1.9.0',
175171
'component.c.update@invalid',
176172
'component.c.update@2.0.0-beta.1',
177173
'component.c.update@2.0.0',
178-
], 'component.c.update@');
174+
]);
179175

180176
expect(result).toBe('component.c.update@2.0.0');
181177
});
182178

183-
it('selectMergeSibling returns undefined when there are no valid semver candidates', () => {
184-
const result = command['selectMergeSibling']([
179+
it('selectNewestMergeSibling returns undefined when there are no valid semver candidates', () => {
180+
const result = command['selectNewestMergeSibling']([
185181
'component.c.update@draft',
186182
'component.c.update@next',
187-
], 'component.c.update@');
183+
]);
188184

189185
expect(result).toBeUndefined();
190186
});
@@ -266,7 +262,7 @@ describe('MergeCommand', () => {
266262
it('returns Windows VS Code CLI path when found in standard locations', () => {
267263
const expectedCodePath = path.join('C:', 'Program Files', 'Microsoft VS Code', 'bin', 'code.cmd');
268264
jest.spyOn(os, 'platform').mockReturnValue('win32');
269-
mockedFsUtils.fileExists.mockImplementation((filePath?: string): filePath is string => filePath === expectedCodePath);
265+
jest.spyOn(fsUtils, 'fileExists').mockImplementation((filePath?: string): filePath is string => filePath === expectedCodePath);
270266

271267
const result = command['getVSCodeExecutablePath']();
272268

@@ -292,28 +288,30 @@ describe('MergeCommand', () => {
292288
getVSCodeExecutablePath: () => string | undefined;
293289
doOpen3WayMerge: (cmd: string) => Promise<number>;
294290
};
291+
const deleteFileIfExistsSpy = jest.spyOn(fsUtils, 'deleteFileIfExists');
292+
const renameFileSpy = jest.spyOn(fsUtils, 'renameFile');
293+
jest.spyOn(fsUtils, 'copyFile').mockImplementation(() => { });
294+
jest.spyOn(fsUtils, 'getFileModificationTime').mockReturnValue(1000);
295295
jest.spyOn(commandPrivate, 'getVSCodeExecutablePath').mockReturnValue('/usr/bin/code');
296296
jest.spyOn(commandPrivate, 'doOpen3WayMerge').mockResolvedValue(1);
297-
mockedFsUtils.copyFile.mockImplementation(() => { });
298-
mockedFsUtils.getFileModificationTime.mockReturnValue(1000);
299297

300298
const warningSpy = jest.spyOn(console, 'warn').mockImplementation(() => { });
301299

302300
await command['runVSCodeMerge'](fileNode);
303301

304302
expect(warningSpy).toHaveBeenCalledWith('Merge exited with code 1. Conflicts may exist.');
305-
expect(mockedFsUtils.deleteFileIfExists).not.toHaveBeenCalled();
306-
expect(mockedFsUtils.renameFile).not.toHaveBeenCalled();
303+
expect(deleteFileIfExistsSpy).not.toHaveBeenCalled();
304+
expect(renameFileSpy).not.toHaveBeenCalled();
307305
expect(activeSolutionTracker.triggerReload).not.toHaveBeenCalled();
308306
});
309307

310308
it('handles merge errors gracefully', async () => {
311309
const codePath = '/usr/bin/code';
312310
jest.spyOn(os, 'platform').mockReturnValue('linux');
313311
mockedExecSync.mockReturnValue(codePath);
314-
mockedFsUtils.copyFile.mockImplementation(() => { });
315-
mockedFsUtils.fileExists.mockReturnValue(true);
316-
mockedFsUtils.getFileModificationTime.mockReturnValue(1000);
312+
jest.spyOn(fsUtils, 'copyFile').mockImplementation(() => { });
313+
jest.spyOn(fsUtils, 'fileExists').mockReturnValue(true);
314+
jest.spyOn(fsUtils, 'getFileModificationTime').mockReturnValue(1000);
317315
mockedExec.mockImplementation((_cmd, _cb) => { throw new Error('unexpected'); });
318316

319317
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => { });
@@ -330,15 +328,15 @@ describe('MergeCommand', () => {
330328

331329
const commandPrivate = command as unknown as {
332330
getVSCodeExecutablePath: () => string | undefined;
333-
findMergeFiles: (localPath: string) => { update: string | undefined; base: string | undefined };
331+
findNewestMergeFiles: (localPath: string) => { update: string | undefined; base: string | undefined };
334332
};
335333
jest.spyOn(commandPrivate, 'getVSCodeExecutablePath').mockReturnValue('/usr/bin/code');
336-
jest.spyOn(commandPrivate, 'findMergeFiles').mockReturnValue({
334+
jest.spyOn(commandPrivate, 'findNewestMergeFiles').mockReturnValue({
337335
update: `${local}.update@1.0.0&unsafe`,
338336
base: `${local}.base@1.0.0`,
339337
});
340-
mockedFsUtils.copyFile.mockImplementation(() => { });
341-
mockedFsUtils.getFileModificationTime.mockReturnValue(1000);
338+
jest.spyOn(fsUtils, 'copyFile').mockImplementation(() => { });
339+
jest.spyOn(fsUtils, 'getFileModificationTime').mockReturnValue(1000);
342340

343341
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
344342

@@ -362,21 +360,24 @@ describe('MergeCommand', () => {
362360
getVSCodeExecutablePath: () => string | undefined;
363361
doOpen3WayMerge: (cmd: string) => Promise<number>;
364362
};
365-
jest.spyOn(commandPrivate, 'getVSCodeExecutablePath').mockReturnValue('/usr/bin/code');
366-
jest.spyOn(commandPrivate, 'doOpen3WayMerge').mockResolvedValue(0);
367-
mockedFsUtils.copyFile.mockImplementation(() => { });
368-
mockedFsUtils.getFileModificationTime
363+
const copyFileSpy = jest.spyOn(fsUtils, 'copyFile').mockImplementation(() => { });
364+
const getFileModificationTimeSpy = jest.spyOn(fsUtils, 'getFileModificationTime')
369365
.mockReturnValueOnce(1000)
370366
.mockReturnValueOnce(2000);
367+
const deleteFileIfExistsSpy = jest.spyOn(fsUtils, 'deleteFileIfExists').mockImplementation(() => { });
368+
const renameFileSpy = jest.spyOn(fsUtils, 'renameFile').mockImplementation(() => { });
369+
jest.spyOn(commandPrivate, 'getVSCodeExecutablePath').mockReturnValue('/usr/bin/code');
370+
jest.spyOn(commandPrivate, 'doOpen3WayMerge').mockResolvedValue(0);
371371

372372
await command['runVSCodeMerge'](node);
373373

374-
expect(mockedFsUtils.copyFile).toHaveBeenCalledWith(local, merged);
375-
expect(mockedFsUtils.copyFile).toHaveBeenCalledWith(local, `${local}.bak`);
376-
expect(mockedFsUtils.deleteFileIfExists).toHaveBeenCalledWith(local);
377-
expect(mockedFsUtils.deleteFileIfExists).toHaveBeenCalledWith(base);
378-
expect(mockedFsUtils.renameFile).toHaveBeenCalledWith(update, expectedBase);
379-
expect(mockedFsUtils.renameFile).toHaveBeenCalledWith(merged, local);
374+
expect(copyFileSpy).toHaveBeenCalledWith(local, merged);
375+
expect(copyFileSpy).toHaveBeenCalledWith(local, `${local}.bak`);
376+
expect(getFileModificationTimeSpy).toHaveBeenCalledTimes(2);
377+
expect(deleteFileIfExistsSpy).toHaveBeenCalledWith(local);
378+
expect(deleteFileIfExistsSpy).toHaveBeenCalledWith(base);
379+
expect(renameFileSpy).toHaveBeenCalledWith(update, expectedBase);
380+
expect(renameFileSpy).toHaveBeenCalledWith(merged, local);
380381
expect(activeSolutionTracker.triggerReload).toHaveBeenCalledTimes(1);
381382
});
382383
});

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import path from 'path';
2323
import * as os from 'os';
2424
import { ActiveSolutionTracker } from '../../../solutions/active-solution-tracker';
2525
import semver from 'semver';
26-
import { extractSuffix } from '../../../utils/string-utils';
26+
import { extractVersion } from '../../../utils/string-utils';
2727
import * as fsUtils from '../../../utils/fs-utils';
2828

2929
export class MergeCommand {
@@ -108,7 +108,7 @@ export class MergeCommand {
108108
return undefined;
109109
}
110110

111-
const discovered = this.findMergeFiles(local);
111+
const discovered = this.findNewestMergeFiles(local);
112112
const update = discovered.update;
113113
if (!update) {
114114
vscode.window.showErrorMessage('Required update file is missing to perform merge.');
@@ -238,7 +238,7 @@ export class MergeCommand {
238238
});
239239
}
240240

241-
private findMergeFiles(localPath: string): { update: string | undefined; base: string | undefined } {
241+
private findNewestMergeFiles(localPath: string): { update: string | undefined; base: string | undefined } {
242242
const dir = path.dirname(localPath);
243243
const fileName = path.basename(localPath);
244244

@@ -248,12 +248,11 @@ export class MergeCommand {
248248
} catch {
249249
return { update: undefined, base: undefined };
250250
}
251-
252251
const updatePrefix = `${fileName}.update@`;
253252
const basePrefix = `${fileName}.base@`;
254253

255-
const update = this.resolveMergeSibling(fileNames, updatePrefix);
256-
const base = this.resolveMergeSibling(fileNames, basePrefix);
254+
const update = this.resolveNewestMergeSibling(fileNames, updatePrefix);
255+
const base = this.resolveNewestMergeSibling(fileNames, basePrefix);
257256

258257
if (!update || !base) {
259258
return { update: undefined, base: undefined };
@@ -265,23 +264,24 @@ export class MergeCommand {
265264
};
266265
}
267266

268-
private resolveMergeSibling(fileNames: string[], prefix: string): string | undefined {
267+
private resolveNewestMergeSibling(fileNames: string[], prefix: string): string | undefined {
269268
const matches = fileNames.filter(fileName => fileName.startsWith(prefix));
270-
return this.selectMergeSibling(matches, prefix);
269+
return this.selectNewestMergeSibling(matches);
271270
}
272271

273-
private selectMergeSibling(fileNames: string[], prefix: string): string | undefined {
272+
private selectNewestMergeSibling(fileNames: string[]): string | undefined {
274273
if (fileNames.length === 0) {
275274
return undefined;
276275
}
277276

278277
return fileNames
279278
.map(fileName => {
280-
const version = semver.valid(extractSuffix(fileName, prefix));
279+
const version = semver.valid(extractVersion(fileName));
281280
return version ? { fileName, version } : undefined;
282281
})
283282
.filter((candidate): candidate is { fileName: string; version: string } => candidate !== undefined)
284283
.sort((left, right) => semver.rcompare(left.version, right.version))[0]?.fileName;
285284
}
286285

287286
}
287+

‎src/views/solution-outline/tree-structure/solution-outline-file-item.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ describe('FileItem', () => {
105105
expect(child.getTag()).toBe('file');
106106
expect(child.getAttribute('label')).toContain(fileLabels[idx]);
107107
expect(child.getAttribute('description')).toBeUndefined();
108-
expect(path.basename(child.getAttribute('local') ?? '')).toBe(fileLabels[idx]);
108+
expect(path.basename(child.getAttribute('resourcePath') ?? '')).toBe(fileLabels[idx]);
109109
expect(child.getAttribute('features')).toContain(manifest.MERGE_FILE_CONTEXT);
110110
});
111111

‎src/views/solution-outline/tree-structure/solution-outline-file-item.ts‎

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -93,32 +93,16 @@ export class FileItemBuilder extends SolutionOutlineItemBuilder {
9393
return item as COutlineItem;
9494
}
9595

96-
public addMergeFeature(f: ITreeItem<CTreeItem>, cfileItem: COutlineItem, options?: { skipValidation?: boolean; localPathOverride?: string }) {
96+
public addMergeFeature(f: ITreeItem<CTreeItem>, cfileItem: COutlineItem) {
9797
// Only config files with a resolvable resource path are eligible.
98-
// Callers can bypass this when they already validated inputs upstream.
99-
if (!options?.skipValidation) {
100-
const attr = f.getValue('attr');
101-
if (attr !== 'config') {
102-
return;
103-
}
104-
const localPath = cfileItem.getValue('resourcePath');
105-
if (!localPath) {
106-
return;
107-
}
108-
}
109-
110-
// Prefer an explicit local path from caller; otherwise fall back to the tree item's resource path.
111-
const localPath = options?.localPathOverride ?? cfileItem.getValue('resourcePath');
112-
if (!localPath) {
98+
const attr = f.getValue('attr');
99+
if (attr !== 'config') {
113100
return;
114101
}
115-
116102
const fileStatus = f.getValue('status');
117103
if (!fileStatus) {
118104
return;
119105
}
120-
121-
cfileItem.setAttribute('local', localPath);
122106
setMergeFileContext(cfileItem);
123107
}
124108
}

‎src/views/solution-outline/tree-structure/solution-outline-hardware-item.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ describe('HardwareItemBuilder', () => {
9393
const device = children?.[0];
9494

9595
// check merge attributes set by addMergeFeature
96-
const local = device?.getAttribute('local');
96+
const local = device?.getAttribute('resourcePath');
9797
const gotLocal = local ? path.basename(local) : '';
9898
const wantLocal = 'Hello+CS300.dbgconf';
9999
expect(gotLocal).toEqual(wantLocal);

‎src/views/solution-outline/tree-structure/solution-outline-hardware-item.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,6 @@ export class HardwareItemBuilder extends SolutionOutlineItemBuilder {
146146

147147
// add merge feature for dbgconf node
148148
const fileItem = new FileItemBuilder();
149-
fileItem.addMergeFeature(dbgConfigEntry ?? file, dbgconfFileItem, { skipValidation: true, localPathOverride: filePath });
149+
fileItem.addMergeFeature(dbgConfigEntry ?? file, dbgconfFileItem);
150150
}
151151
}

0 commit comments

Comments
 (0)