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
microsoft · Nov 3, 2024 · 7c2b060 · 7c2b060
1 parent 220ac74
commit 7c2b060
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 23 deletions.
diff --git 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
@@ -32,6 +32,8 @@ interface SiteAttributes {}
 
 type TestStatus = ('failed' | 'notrun' | 'passed');
 
+type DriverMapT = Map<string, { driver: CMakeDriver; ctestPath: string; ctestArgs: string[]; tests: vscode.TestItem[] }>;
+
 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<number> {
-        let returnCode: number = 0;
-        const driverMap = new Map<string, { driver: CMakeDriver; ctestPath: string; ctestArgs: string[]; tests: vscode.TestItem[]}>();
+    /**
+     * 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<CMakeDriver | String> {
+        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<void> {
+        let _driverMap = <DriverMapT>{};
+        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<number> {
+        let returnCode: number = 0;
+        const driverMap = <DriverMapT>{};
+
+        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;