From 89def458d775a8c71579ffcb7c6196a3c4c82db3 Mon Sep 17 00:00:00 2001 From: Brian Cody Date: Tue, 22 Jul 2025 16:10:12 -0400 Subject: [PATCH 01/12] Build the necessary targets prior to running tests. --- CHANGELOG.md | 4 +++ src/ctest.ts | 100 +++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 77 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e71f98d86..50510958bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # What's New? +Improvements: + +- When CMake is invoked prior to running tests, build targets required for the test rather than everything. [#4515](https://github.com/microsoft/vscode-cmake-tools/issues/4515) + ## 1.21 Features: diff --git a/src/ctest.ts b/src/ctest.ts index 7e9ab68547..c6defb86ed 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -134,7 +134,7 @@ interface ProjectCoverageConfig { function parseXmlString(xml: string): Promise { return new Promise((resolve, reject) => { - xml2js.parseString(xml, (err, result) => { + xml2js.parseString(xml, (err: any, result: T | PromiseLike) => { if (err) { reject(err); } else { @@ -1270,44 +1270,90 @@ export class CTestDriver implements vscode.Disposable { return currentTestItem.id; } + /** + * Determine and build all targets needed to rebuild the selected TestItems. Returns a false if anything goes + * wrong, and returns an early success if build-before-run is disabled. + */ private async buildTests(tests: vscode.TestItem[], run: vscode.TestRun): Promise { // If buildBeforeRun is set to false, we skip the build step if (!this.ws.config.buildBeforeRun) { return true; } - // Folder => status - const builtFolder = new Map(); - let status: number = 0; + const foundTarget = new Map>(); for (const test of tests) { - const folder = this.getTestRootFolder(test); - if (!builtFolder.has(folder)) { - const project = await this.projectController?.getProjectForFolder(folder); - if (!project) { - status = 1; - } else { - try { - if (extensionManager !== undefined && extensionManager !== null) { - extensionManager.cleanOutputChannel(); - } - const buildResult = await project.build(undefined, false, false); - if (buildResult !== 0) { - status = 2; - } - } catch (e) { - status = 2; - } - } + if (!await this.getTestTargets(test, foundTarget, run)) { + return false; + } + } + + return this.buildTestTargets(foundTarget, run); + } + + /** + * Given the TestItem, determine the owning CMakeProject and build targets to build the tests. If the given + * TestItem is a suite, then recurse. Information is passed back through the foundTarget parameter. A failure to + * identify the project of the test will result in a ctest error and a false value being returned. + */ + private async getTestTargets(test: vscode.TestItem, foundTarget: Map>, run: vscode.TestRun): Promise { + if (test.children.size > 0) { + const children = this.testItemCollectionToArray(test.children); + for (const child of children) { + await this.getTestTargets(child, foundTarget, run); } - builtFolder.set(folder, status); - if (status === 1) { + } else { + const testProgram = this.testProgram(test.id); + const folder = this.getTestRootFolder(test); + const project = await this.projectController?.getProjectForFolder(folder); + if (!project) { this.ctestErrored(test, run, { message: localize('no.project.found', 'No project found for folder {0}', folder) }); - } else if (status === 2) { - this.ctestErrored(test, run, { message: localize('build.failed', 'Build failed') }); + return false; + } + if (!foundTarget.has(project!)) { + foundTarget.set(project!, new Map([[testProgram, [test]]])); + } else if (!foundTarget.get(project!)?.has(testProgram)) { + foundTarget.get(project!)?.set(testProgram, [test]); + } else { + foundTarget.get(project!)?.get(testProgram)?.push(test); } } + return true; + } - return Array.from(builtFolder.values()).filter(v => v !== 0).length === 0; + /** + * Build the targets provided in foundTarget. CMake will be invoked once per CMakeProject for efficiency. On error, + * the associated tests will be flagged with a ctest error and a false value will be returned. + */ + private async buildTestTargets(foundTarget: Map>, run: vscode.TestRun): Promise { + let overallSuccess = true; + for (const [project, targets] of foundTarget) { + const binaryDir = (await project.binaryDir).toString(); + const accmulatedTestList: vscode.TestItem[] = []; + const accumulatedTargets: string[] = []; + let success: boolean = true; + for (const [targetName, testList] of targets) { + accumulatedTargets.push(path.relative(binaryDir, targetName)); + accmulatedTestList.concat(testList); + } + try { + if (extensionManager !== undefined && extensionManager !== null) { + extensionManager.cleanOutputChannel(); + } + const buildResult = await project.build(accumulatedTargets, false, false); + if (buildResult !== 0) { + success = false; + } + } catch (e) { + success = false; + } + if (!success) { + overallSuccess = false; + accmulatedTestList.forEach(test => { + this.ctestErrored(test, run, { message: localize('build.failed', 'Build failed') }); + }); + } + }; + return overallSuccess; } /** From 9e6cdcf8ac6c463630f7eef6cc2c772eb1d09f81 Mon Sep 17 00:00:00 2001 From: Brian Cody Date: Tue, 22 Jul 2025 16:24:50 -0400 Subject: [PATCH 02/12] Cleaned up change log and ctest impl a bit. --- CHANGELOG.md | 5 +---- src/ctest.ts | 15 +++++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29881a2fb2..cff3cb9fbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,5 @@ # What's New? -Improvements: - -- When CMake is invoked prior to running tests, build targets required for the test rather than everything. [#4515](https://github.com/microsoft/vscode-cmake-tools/issues/4515) - ## 1.21 Features: @@ -21,6 +17,7 @@ Improvements: - No longer convert paths on lowercase on MacOS to enable cpp tools to resolve them. [#4325](https://github.com/microsoft/vscode-cmake-tools/pull/4325) [@tringenbach](https://github.com/tringenbach) - Speedup & reduce heap allocations in shlex split module function. Significant gains for mid-large compile_commands.json - CompilationDatabase construction. [#4458](https://github.com/microsoft/vscode-cmake-tools/pull/4458) [@borjamunozf](https://github.com/borjamunozf) - In the Test Explorer, associate CTest tests with outermost function or macro invocation that calls `add_test()` instead of with the `add_test()` call itself. [#4490](https://github.com/microsoft/vscode-cmake-tools/issues/4490) [@malsyned](https://github.com/malsyned) +- When CMake is invoked prior to running tests, build targets required for the test rather than everything. [#4515](https://github.com/microsoft/vscode-cmake-tools/issues/4515) [@epistax](https://github.com/epistax) Bug Fixes: diff --git a/src/ctest.ts b/src/ctest.ts index 03d67f1b78..26f76816d5 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -134,7 +134,7 @@ interface ProjectCoverageConfig { function parseXmlString(xml: string): Promise { return new Promise((resolve, reject) => { - xml2js.parseString(xml, (err: any, result: T | PromiseLike) => { + xml2js.parseString(xml, (err, result) => { if (err) { reject(err); } else { @@ -1314,12 +1314,15 @@ export class CTestDriver implements vscode.Disposable { this.ctestErrored(test, run, { message: localize('no.project.found', 'No project found for folder {0}', folder) }); return false; } - if (!foundTarget.has(project!)) { - foundTarget.set(project!, new Map([[testProgram, [test]]])); - } else if (!foundTarget.get(project!)?.has(testProgram)) { - foundTarget.get(project!)?.set(testProgram, [test]); + if (!foundTarget.has(project)) { + foundTarget.set(project, new Map([[testProgram, [test]]])); } else { - foundTarget.get(project!)?.get(testProgram)?.push(test); + const prj = foundTarget.get(project)!; + if (!prj.has(testProgram)) { + prj.set(testProgram, [test]); + } else { + prj.get(testProgram)!.push(test); + } } } return true; From d548f90995d5ef33dec1827175519b5efb49827f Mon Sep 17 00:00:00 2001 From: MacGyver Codilla Date: Tue, 7 Apr 2026 10:30:21 -0700 Subject: [PATCH 03/12] Fix build-before-test: correct target name resolution and error reporting - BLOCKER: Replace path.relative() with executableTargets lookup to map test program paths to actual CMake target names. The previous approach produced file paths (e.g. 'tests/my_test') instead of target names (e.g. 'my_test'), causing cmake --build --target to fail. - BLOCKER: Fix accmulatedTestList.concat() no-op by using push(...) so tests are properly marked as errored on build failure. - MAJOR: Check return value of recursive getTestTargets() call so child failures propagate correctly to the caller. - MINOR: Add guard for empty testProgram() return to prevent nonsensical build targets from empty strings. - MINOR: Fix typo accmulatedTestList -> accumulatedTestList. - MINOR: Remove extra whitespace in for statement and redundant .toString() call. - NIT: Remove trailing semicolon after for...of block. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/ctest.ts | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/ctest.ts b/src/ctest.ts index 26f76816d5..267fbce948 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -1304,10 +1304,16 @@ export class CTestDriver implements vscode.Disposable { if (test.children.size > 0) { const children = this.testItemCollectionToArray(test.children); for (const child of children) { - await this.getTestTargets(child, foundTarget, run); + if (!await this.getTestTargets(child, foundTarget, run)) { + return false; + } } } else { const testProgram = this.testProgram(test.id); + if (!testProgram) { + this.ctestErrored(test, run, { message: localize('test.program.not.found', 'Test program not found for {0}', test.id) }); + return false; + } const folder = this.getTestRootFolder(test); const project = await this.projectController?.getProjectForFolder(folder); if (!project) { @@ -1335,13 +1341,24 @@ export class CTestDriver implements vscode.Disposable { private async buildTestTargets(foundTarget: Map>, run: vscode.TestRun): Promise { let overallSuccess = true; for (const [project, targets] of foundTarget) { - const binaryDir = (await project.binaryDir).toString(); - const accmulatedTestList: vscode.TestItem[] = []; + const execTargets = await project.executableTargets; + const exePathToTargetName = new Map(); + for (const t of execTargets) { + exePathToTargetName.set(path.normalize(t.path), t.name); + } + const accumulatedTestList: vscode.TestItem[] = []; const accumulatedTargets: string[] = []; let success: boolean = true; - for (const [targetName, testList] of targets) { - accumulatedTargets.push(path.relative(binaryDir, targetName)); - accmulatedTestList.concat(testList); + for (const [testProgramPath, testList] of targets) { + const normalizedPath = path.normalize(testProgramPath); + const targetName = exePathToTargetName.get(normalizedPath); + if (targetName) { + accumulatedTargets.push(targetName); + } else { + // Fallback: use the file name without extension as the target name + accumulatedTargets.push(path.basename(testProgramPath, path.extname(testProgramPath))); + } + accumulatedTestList.push(...testList); } try { if (extensionManager !== undefined && extensionManager !== null) { @@ -1356,11 +1373,11 @@ export class CTestDriver implements vscode.Disposable { } if (!success) { overallSuccess = false; - accmulatedTestList.forEach(test => { + accumulatedTestList.forEach(test => { this.ctestErrored(test, run, { message: localize('build.failed', 'Build failed') }); }); } - }; + } return overallSuccess; } From 7fec8a3669b1373d3ab0b70df3f8f328cf80a548 Mon Sep 17 00:00:00 2001 From: MacGyver Codilla Date: Mon, 13 Apr 2026 10:48:45 -0700 Subject: [PATCH 04/12] Revert "Fix build-before-test: correct target name resolution and error reporting" This reverts commit d548f90995d5ef33dec1827175519b5efb49827f. --- src/ctest.ts | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/src/ctest.ts b/src/ctest.ts index 267fbce948..26f76816d5 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -1304,16 +1304,10 @@ export class CTestDriver implements vscode.Disposable { if (test.children.size > 0) { const children = this.testItemCollectionToArray(test.children); for (const child of children) { - if (!await this.getTestTargets(child, foundTarget, run)) { - return false; - } + await this.getTestTargets(child, foundTarget, run); } } else { const testProgram = this.testProgram(test.id); - if (!testProgram) { - this.ctestErrored(test, run, { message: localize('test.program.not.found', 'Test program not found for {0}', test.id) }); - return false; - } const folder = this.getTestRootFolder(test); const project = await this.projectController?.getProjectForFolder(folder); if (!project) { @@ -1341,24 +1335,13 @@ export class CTestDriver implements vscode.Disposable { private async buildTestTargets(foundTarget: Map>, run: vscode.TestRun): Promise { let overallSuccess = true; for (const [project, targets] of foundTarget) { - const execTargets = await project.executableTargets; - const exePathToTargetName = new Map(); - for (const t of execTargets) { - exePathToTargetName.set(path.normalize(t.path), t.name); - } - const accumulatedTestList: vscode.TestItem[] = []; + const binaryDir = (await project.binaryDir).toString(); + const accmulatedTestList: vscode.TestItem[] = []; const accumulatedTargets: string[] = []; let success: boolean = true; - for (const [testProgramPath, testList] of targets) { - const normalizedPath = path.normalize(testProgramPath); - const targetName = exePathToTargetName.get(normalizedPath); - if (targetName) { - accumulatedTargets.push(targetName); - } else { - // Fallback: use the file name without extension as the target name - accumulatedTargets.push(path.basename(testProgramPath, path.extname(testProgramPath))); - } - accumulatedTestList.push(...testList); + for (const [targetName, testList] of targets) { + accumulatedTargets.push(path.relative(binaryDir, targetName)); + accmulatedTestList.concat(testList); } try { if (extensionManager !== undefined && extensionManager !== null) { @@ -1373,11 +1356,11 @@ export class CTestDriver implements vscode.Disposable { } if (!success) { overallSuccess = false; - accumulatedTestList.forEach(test => { + accmulatedTestList.forEach(test => { this.ctestErrored(test, run, { message: localize('build.failed', 'Build failed') }); }); } - } + }; return overallSuccess; } From 66c3263f66d5067cba1c0eedacfa5cdbc4dd378f Mon Sep 17 00:00:00 2001 From: MacGyver Codilla Date: Mon, 13 Apr 2026 11:34:39 -0700 Subject: [PATCH 05/12] Closing brace --- src/ctest.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ctest.ts b/src/ctest.ts index 8a5c1a27b0..525447c247 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -1846,6 +1846,7 @@ export class CTestDriver implements vscode.Disposable { for (const test of tests) { if (!await this.getTestTargets(test, foundTarget, run)) { return false; + } const folder = this.getTestRootFolder(test); if (!builtFolder.has(folder)) { const project = await this.projectController?.getProjectForFolder(folder); @@ -1929,7 +1930,7 @@ export class CTestDriver implements vscode.Disposable { extensionManager.cleanOutputChannel(); } const buildResult = await project.build(accumulatedTargets, false, false); - if (buildResult !== 0) { + if (buildResult.exitCode !== 0) { success = false; } } catch (e) { From 8195cc63a94b872602dd5fd9401338c5f5941322 Mon Sep 17 00:00:00 2001 From: MacGyver Codilla Date: Mon, 13 Apr 2026 11:50:12 -0700 Subject: [PATCH 06/12] Readd these --- src/ctest.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ctest.ts b/src/ctest.ts index 525447c247..6cd0e35eb1 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -1842,6 +1842,9 @@ export class CTestDriver implements vscode.Disposable { return true; } + // Folder => status + const builtFolder = new Map(); + let status: number = 0; const foundTarget = new Map>(); for (const test of tests) { if (!await this.getTestTargets(test, foundTarget, run)) { From c5c3485c62671382e43e6eae1878d63236af0f58 Mon Sep 17 00:00:00 2001 From: MacGyver Codilla Date: Mon, 13 Apr 2026 13:39:23 -0700 Subject: [PATCH 07/12] Compute the target name instead of the executable --- src/ctest.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/ctest.ts b/src/ctest.ts index 6cd0e35eb1..5ed6c5b97a 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -1920,13 +1920,17 @@ export class CTestDriver implements vscode.Disposable { private async buildTestTargets(foundTarget: Map>, run: vscode.TestRun): Promise { let overallSuccess = true; for (const [project, targets] of foundTarget) { - const binaryDir = (await project.binaryDir).toString(); + const execTargets = await project.executableTargets; const accmulatedTestList: vscode.TestItem[] = []; const accumulatedTargets: string[] = []; let success: boolean = true; - for (const [targetName, testList] of targets) { - accumulatedTargets.push(path.relative(binaryDir, targetName)); - accmulatedTestList.concat(testList); + for (const [targetPath, testList] of targets) { + // Look up the CMake target name from executable targets by matching + // the executable path. Using path.relative() uses the executable instead of the target name + const normalizedTargetPath = util.platformNormalizePath(targetPath); + const execTarget = execTargets.find(t => util.platformNormalizePath(t.path) === normalizedTargetPath); + accumulatedTargets.push(execTarget ? execTarget.name : path.parse(targetPath).name); + accmulatedTestList.push(...testList); } try { if (extensionManager !== undefined && extensionManager !== null) { From 3ae3c121616131faca615062707509049b3c22c1 Mon Sep 17 00:00:00 2001 From: MacGyver Codilla Date: Mon, 20 Apr 2026 08:24:38 -0700 Subject: [PATCH 08/12] Fix typo in variable name --- src/ctest.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ctest.ts b/src/ctest.ts index 5ed6c5b97a..dd30305853 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -1921,7 +1921,7 @@ export class CTestDriver implements vscode.Disposable { let overallSuccess = true; for (const [project, targets] of foundTarget) { const execTargets = await project.executableTargets; - const accmulatedTestList: vscode.TestItem[] = []; + const accumulatedTestList: vscode.TestItem[] = []; const accumulatedTargets: string[] = []; let success: boolean = true; for (const [targetPath, testList] of targets) { @@ -1930,7 +1930,7 @@ export class CTestDriver implements vscode.Disposable { const normalizedTargetPath = util.platformNormalizePath(targetPath); const execTarget = execTargets.find(t => util.platformNormalizePath(t.path) === normalizedTargetPath); accumulatedTargets.push(execTarget ? execTarget.name : path.parse(targetPath).name); - accmulatedTestList.push(...testList); + accumulatedTestList.push(...testList); } try { if (extensionManager !== undefined && extensionManager !== null) { @@ -1945,7 +1945,7 @@ export class CTestDriver implements vscode.Disposable { } if (!success) { overallSuccess = false; - accmulatedTestList.forEach(test => { + accumulatedTestList.forEach(test => { this.ctestErrored(test, run, { message: localize('build.failed', 'Build failed') }); }); } From 632b758b4889633dc1caafb88a9f4b5c72e343b6 Mon Sep 17 00:00:00 2001 From: MacGyver Codilla Date: Mon, 20 Apr 2026 08:27:32 -0700 Subject: [PATCH 09/12] Update src/ctest.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/ctest.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ctest.ts b/src/ctest.ts index 5ed6c5b97a..02bbf2b787 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -1889,7 +1889,9 @@ export class CTestDriver implements vscode.Disposable { if (test.children.size > 0) { const children = this.testItemCollectionToArray(test.children); for (const child of children) { - await this.getTestTargets(child, foundTarget, run); + if (!await this.getTestTargets(child, foundTarget, run)) { + return false; + } } } else { const testProgram = this.testProgram(test.id); From ea4ee78b9af1d4fadc4fc1f5ef86592f7294a645 Mon Sep 17 00:00:00 2001 From: MacGyver Codilla Date: Mon, 20 Apr 2026 08:35:03 -0700 Subject: [PATCH 10/12] Use a precomputed map --- src/ctest.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/ctest.ts b/src/ctest.ts index 739de8ad44..ecf37dab50 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -1923,15 +1923,17 @@ export class CTestDriver implements vscode.Disposable { let overallSuccess = true; for (const [project, targets] of foundTarget) { const execTargets = await project.executableTargets; + // Precompute a lookup map from normalized executable path to target name + const execPathToName = new Map(); + for (const t of execTargets) { + execPathToName.set(util.platformNormalizePath(t.path), t.name); + } const accumulatedTestList: vscode.TestItem[] = []; const accumulatedTargets: string[] = []; let success: boolean = true; - for (const [targetPath, testList] of targets) { - // Look up the CMake target name from executable targets by matching - // the executable path. Using path.relative() uses the executable instead of the target name + for (const [targetPath, testList] of targets) { const normalizedTargetPath = util.platformNormalizePath(targetPath); - const execTarget = execTargets.find(t => util.platformNormalizePath(t.path) === normalizedTargetPath); - accumulatedTargets.push(execTarget ? execTarget.name : path.parse(targetPath).name); + accumulatedTargets.push(execPathToName.get(normalizedTargetPath) ?? path.parse(targetPath).name); accumulatedTestList.push(...testList); } try { From 1398c4ecdaaaa14687b7eb3a1cbf78cbe45106f0 Mon Sep 17 00:00:00 2001 From: MacGyver Codilla Date: Mon, 20 Apr 2026 09:41:33 -0700 Subject: [PATCH 11/12] Update src/ctest.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/ctest.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ctest.ts b/src/ctest.ts index ecf37dab50..6150284d02 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -1895,6 +1895,10 @@ export class CTestDriver implements vscode.Disposable { } } else { const testProgram = this.testProgram(test.id); + if (!testProgram) { + this.ctestErrored(test, run, { message: localize('test.program.not.found', 'Could not determine the test program for test {0}', test.id) }); + return false; + } const folder = this.getTestRootFolder(test); const project = await this.projectController?.getProjectForFolder(folder); if (!project) { From 5e177192709954c4aa1f8149c011d51e7aa6eb46 Mon Sep 17 00:00:00 2001 From: MacGyver Codilla Date: Thu, 23 Apr 2026 10:16:29 -0700 Subject: [PATCH 12/12] fix log target names and exit code when build-before-test fails --- src/ctest.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ctest.ts b/src/ctest.ts index ecf37dab50..d117be3b3c 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -1942,9 +1942,11 @@ export class CTestDriver implements vscode.Disposable { } const buildResult = await project.build(accumulatedTargets, false, false); if (buildResult.exitCode !== 0) { + log.error(localize('build.targets.failed.with.code', 'Building targets [{0}] failed with exit code {1}.', accumulatedTargets.join(', '), buildResult.exitCode)); success = false; } } catch (e) { + log.error(localize('build.targets.threw', 'Building targets [{0}] threw an error: {1}', accumulatedTargets.join(', '), (e as Error)?.message ?? String(e))); success = false; } if (!success) {