From 7c2b06008f5be7bf8ab70cd619ce875ee2392f67 Mon Sep 17 00:00:00 2001 From: Guillaume Peillex Date: Sat, 2 Nov 2024 14:39:01 +0100 Subject: [PATCH 1/4] fix: CTest Test Suite Delimiter prevents running all tests (#4092) * Introduces type alias (DriverMapT) to factorize type declaration * Introduces the getProjectDriver method * Adds comments * Adds entry in the CHANGELOG --- CHANGELOG.md | 1 + src/ctest.ts | 74 ++++++++++++++++++++++++++++++++++++---------------- 2 files changed, 52 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99891b996..38bd76b12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Improvements: Bug Fixes: +- Fix issue where setting test suite delimiter prevent execution of all tests. [#4092](https://github.com/microsoft/vscode-cmake-tools/issues/4092) - Fix our setting of `isUserPreset` for presets, only set it to `true` if it's defined in a user presets file. [#4059](https://github.com/microsoft/vscode-cmake-tools/issues/4059) - Fix issue where duplicate presets are being listed in dropdown. [#4104](https://github.com/microsoft/vscode-cmake-tools/issues/4104) - Fix various GCC compiler errors and GCC linker errors not showing up in Problems View [#2864](https://github.com/microsoft/vscode-cmake-tools/issues/2864) diff --git a/src/ctest.ts b/src/ctest.ts index 5d1021bdb..99c90e963 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -32,6 +32,8 @@ interface SiteAttributes {} type TestStatus = ('failed' | 'notrun' | 'passed'); +type DriverMapT = Map; + export interface TestMeasurement { type: string; name: string; @@ -395,9 +397,34 @@ export class CTestDriver implements vscode.Disposable { run.failed(test, message, duration); } - private async runCTestHelper(tests: vscode.TestItem[], run: vscode.TestRun, cancellation: vscode.CancellationToken, driver?: CMakeDriver, ctestPath?: string, ctestArgs?: string[], customizedTask: boolean = false, consumer?: proc.OutputConsumer, entryPoint: RunCTestHelperEntryPoint = RunCTestHelperEntryPoint.RunTests): Promise { - let returnCode: number = 0; - const driverMap = new Map(); + /** + * Retrieve the driver from the test in argument + * + * @param test : test to retrieve the driver from + * @returns : the driver or an error message + */ + private async getProjectDriver(test: vscode.TestItem): Promise { + const folder = this.getTestRootFolder(test); + const project = await this.projectController?.getProjectForFolder(folder); + if (!project) { + return localize('no.project.found', 'No project found for folder {0}', folder); + } + const _driver = await project.getCMakeDriverInstance(); + if (!_driver) { + return localize('no.driver.found', 'No driver found for folder {0}', folder); + } + return _driver; + } + + /** + * Recursively collect all tests informations among all the test suite tree and store them in the driverMap + * + */ + private async fillDriverMap(tests: vscode.TestItem[], run: vscode.TestRun, cancellation: vscode.CancellationToken, driverMap?: DriverMapT, driver?: CMakeDriver, ctestPath?: string, ctestArgs?: string[], customizedTask: boolean = false): Promise { + let _driverMap = {}; + if (driverMap) { + _driverMap = driverMap; + } /** * Loop through the tests and get the driver, ctestPath, ctestArgs for each test. Construct a map for each soure directory, mapping drivers to tests. @@ -407,17 +434,13 @@ export class CTestDriver implements vscode.Disposable { if (driver) { _driver = driver; } else { - 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) }); - continue; - } - _driver = await project.getCMakeDriverInstance(); - if (!_driver) { - this.ctestErrored(test, run, { message: localize('no.driver.found', 'No driver found for folder {0}', folder) }); + const _maybe_driver = this.getProjectDriver(test); + if (typeof _maybe_driver === 'string') { + this.ctestErrored(test, run, { message: _maybe_driver }); continue; - } + } else { + _driver = _maybe_driver as unknown as CMakeDriver; + }; } let _ctestPath: string | null; @@ -444,21 +467,26 @@ export class CTestDriver implements vscode.Disposable { } if (test.children.size > 0) { - // Shouldn't reach here now, but not hard to write so keeping it in case we want to have more complicated test hierarchies + // If test has children then it is a suite, so we need to recursively call this function const children = this.testItemCollectionToArray(test.children); - if (await this.runCTestHelper(children, run, cancellation, _driver, _ctestPath, _ctestArgs, customizedTask, consumer, entryPoint)) { - returnCode = -1; - } - return returnCode; + await this.fillDriverMap(children, run, cancellation, _driverMap, _driver, _ctestPath, _ctestArgs, customizedTask); } else { - if (!driverMap.has(_driver.sourceDir)) { - driverMap.set(_driver.sourceDir, { driver: _driver, ctestPath: _ctestPath, ctestArgs: _ctestArgs, tests: [test] }); + // If test has no children, it is a leaf of the test suite hierarchy so add its characteristics to the driver map + if (!_driverMap.has(_driver.sourceDir)) { + _driverMap.set(_driver.sourceDir, { driver: _driver, ctestPath: _ctestPath, ctestArgs: _ctestArgs, tests: [test] }); } else { - const d = driverMap.get(_driver.sourceDir); - driverMap.set(_driver.sourceDir, { driver: d!.driver, ctestPath: d!.ctestPath, ctestArgs: d!.ctestArgs, tests: d!.tests.concat(test) }); + const d = _driverMap.get(_driver.sourceDir); + _driverMap.set(_driver.sourceDir, { driver: d!.driver, ctestPath: d!.ctestPath, ctestArgs: d!.ctestArgs, tests: d!.tests.concat(test) }); } } } + }; + + private async runCTestHelper(tests: vscode.TestItem[], run: vscode.TestRun, cancellation: vscode.CancellationToken, driver?: CMakeDriver, ctestPath?: string, ctestArgs?: string[], customizedTask: boolean = false, consumer?: proc.OutputConsumer, entryPoint: RunCTestHelperEntryPoint = RunCTestHelperEntryPoint.RunTests): Promise { + let returnCode: number = 0; + const driverMap = {}; + + await this.fillDriverMap(tests, run, cancellation, driverMap, driver, ctestPath, ctestArgs, customizedTask); if (!this.ws.config.ctestAllowParallelJobs) { for (const driver of driverMap.values()) { @@ -777,7 +805,7 @@ export class CTestDriver implements vscode.Disposable { } } - const testAndParentSuite = this.createTestItemAndSuiteTree(test.name, testExplorerRoot, initializedTestExplorer, testDefFile ? vscode.Uri.file(testDefFile) : undefined); + const testAndParentSuite = this.createTestItemAndSuiteTree(test.name, testExplorerRoot, initializedTestExplorer, testDefFile ? vscode.Uri.file(testDefFile) : undefined); const testItem = testAndParentSuite.test; const parentSuiteItem = testAndParentSuite.parentSuite; From 059e73659c15f1665aa9667a433c7a8bc060f6f5 Mon Sep 17 00:00:00 2001 From: Guillaume Peillex Date: Sun, 3 Nov 2024 13:04:28 +0100 Subject: [PATCH 2/4] Fix wrong object initialization --- src/ctest.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ctest.ts b/src/ctest.ts index 99c90e963..e67cb8180 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -421,7 +421,7 @@ export class CTestDriver implements vscode.Disposable { * */ private async fillDriverMap(tests: vscode.TestItem[], run: vscode.TestRun, cancellation: vscode.CancellationToken, driverMap?: DriverMapT, driver?: CMakeDriver, ctestPath?: string, ctestArgs?: string[], customizedTask: boolean = false): Promise { - let _driverMap = {}; + let _driverMap = new Map(); if (driverMap) { _driverMap = driverMap; } @@ -434,7 +434,7 @@ export class CTestDriver implements vscode.Disposable { if (driver) { _driver = driver; } else { - const _maybe_driver = this.getProjectDriver(test); + const _maybe_driver = await this.getProjectDriver(test); if (typeof _maybe_driver === 'string') { this.ctestErrored(test, run, { message: _maybe_driver }); continue; @@ -484,7 +484,7 @@ export class CTestDriver implements vscode.Disposable { private async runCTestHelper(tests: vscode.TestItem[], run: vscode.TestRun, cancellation: vscode.CancellationToken, driver?: CMakeDriver, ctestPath?: string, ctestArgs?: string[], customizedTask: boolean = false, consumer?: proc.OutputConsumer, entryPoint: RunCTestHelperEntryPoint = RunCTestHelperEntryPoint.RunTests): Promise { let returnCode: number = 0; - const driverMap = {}; + const driverMap = new Map(); await this.fillDriverMap(tests, run, cancellation, driverMap, driver, ctestPath, ctestArgs, customizedTask); From dfcad30c35c732ac136f1c1abe5d7abf28e0567b Mon Sep 17 00:00:00 2001 From: Guillaume Peillex Date: Mon, 4 Nov 2024 20:30:01 +0100 Subject: [PATCH 3/4] Takes into account code review remarks. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do not return CMakeDriver | String but do return CMakeDriver and throw error in case of problem --- src/ctest.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/ctest.ts b/src/ctest.ts index e67cb8180..881c26f4a 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -401,17 +401,17 @@ export class CTestDriver implements vscode.Disposable { * Retrieve the driver from the test in argument * * @param test : test to retrieve the driver from - * @returns : the driver or an error message + * @returns : the driver */ - private async getProjectDriver(test: vscode.TestItem): Promise { + private async getProjectDriver(test: vscode.TestItem): Promise { const folder = this.getTestRootFolder(test); const project = await this.projectController?.getProjectForFolder(folder); if (!project) { - return localize('no.project.found', 'No project found for folder {0}', folder); + throw new Error(localize('no.project.found', 'No project found for folder {0}', folder)); } const _driver = await project.getCMakeDriverInstance(); if (!_driver) { - return localize('no.driver.found', 'No driver found for folder {0}', folder); + throw new Error(localize('no.driver.found', 'No driver found for folder {0}', folder)); } return _driver; } @@ -434,13 +434,12 @@ export class CTestDriver implements vscode.Disposable { if (driver) { _driver = driver; } else { - const _maybe_driver = await this.getProjectDriver(test); - if (typeof _maybe_driver === 'string') { - this.ctestErrored(test, run, { message: _maybe_driver }); + try { + _driver = await this.getProjectDriver(test); + } catch (err: any) { + this.ctestErrored(test, run, { message: err.message }); continue; - } else { - _driver = _maybe_driver as unknown as CMakeDriver; - }; + } } let _ctestPath: string | null; From 8b4dd4f4af15dcb86d37766b4921e162989bc142 Mon Sep 17 00:00:00 2001 From: Guillaume Peillex Date: Tue, 5 Nov 2024 20:19:09 +0100 Subject: [PATCH 4/4] Fix lint issue --- src/ctest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ctest.ts b/src/ctest.ts index 881c26f4a..a731694f9 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -401,7 +401,7 @@ export class CTestDriver implements vscode.Disposable { * Retrieve the driver from the test in argument * * @param test : test to retrieve the driver from - * @returns : the driver + * @returns : the driver */ private async getProjectDriver(test: vscode.TestItem): Promise { const folder = this.getTestRootFolder(test);