From b486c5a369b08d525e68733211aac4144bb58c11 Mon Sep 17 00:00:00 2001 From: Garrett Campbell <86264750+gcampbell-msft@users.noreply.github.com> Date: Tue, 23 Jan 2024 14:43:51 -0500 Subject: [PATCH] Add a setting to enable/disable the Test Explorer Integration (#3545) * Add initial setting and use in runCTest There is still work to be done to ensure that we cover all cases/scenarios and code locations to ensure that our integration with the test explorer doesn't happen based on this setting * Hide commands, handle TestController state. Hide the commands based on the Test Explorer when the testExplorer integration is disabled. Ensure the TestController gets disposed when the setting is dynamically changed. * attempt to refresh the tests when re-enabling integration * add changelog * only dispose if needed --- CHANGELOG.md | 1 + package.json | 18 ++++-- package.nls.json | 1 + src/config.ts | 7 ++- src/ctest.ts | 109 +++++++++++++++++++++------------ src/extension.ts | 13 ++++ test/unit-tests/config.test.ts | 3 +- 7 files changed, 105 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8572038d..dae8a7c6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Features: - Update `api.ts` to add the `getActiveFolderPath` method. [#3528](https://github.com/microsoft/vscode-cmake-tools/pull/3528) [@Kemaweyan](https://github.com/Kemaweyan) +- Add a setting that allows users to enable/disable the Test Explorer integration. [#3145](https://github.com/microsoft/vscode-cmake-tools/issues/3145) Improvements: diff --git a/package.json b/package.json index c1180a1f2..fa2a35245 100644 --- a/package.json +++ b/package.json @@ -564,19 +564,19 @@ { "command": "cmake.revealTestExplorer", "title": "%cmake-tools.command.cmake.revealTestExplorer.title%", - "when": "cmake:enableFullFeatureSet", + "when": "cmake:enableFullFeatureSet && cmake:testExplorerIntegrationEnabled", "category": "CMake" }, { "command": "cmake.refreshTests", "title": "%cmake-tools.command.cmake.refreshTests.title%", - "when": "cmake:enableFullFeatureSet", + "when": "cmake:enableFullFeatureSet && cmake:testExplorerIntegrationEnabled", "category": "CMake" }, { "command": "cmake.refreshTestsAll", "title": "%cmake-tools.command.cmake.refreshTestsAll.title%", - "when": "cmake:enableFullFeatureSet", + "when": "cmake:enableFullFeatureSet && cmake:testExplorerIntegrationEnabled", "category": "CMake" }, { @@ -1164,15 +1164,15 @@ }, { "command": "cmake.revealTestExplorer", - "when": "cmake:enableFullFeatureSet" + "when": "cmake:enableFullFeatureSet && cmake:testExplorerIntegrationEnabled" }, { "command": "cmake.refreshTests", - "when": "cmake:enableFullFeatureSet" + "when": "cmake:enableFullFeatureSet && cmake:testExplorerIntegrationEnabled" }, { "command": "cmake.refreshTestsAll", - "when": "cmake:enableFullFeatureSet" + "when": "cmake:enableFullFeatureSet && cmake:testExplorerIntegrationEnabled" }, { "when": "cmake:enableFullFeatureSet", @@ -2030,6 +2030,12 @@ "description": "%cmake-tools.configuration.cmake.ctest.allowParallelJobs.description%", "scope": "resource" }, + "cmake.ctest.testExplorerIntegrationEnabled": { + "type": "boolean", + "default": true, + "description": "%cmake-tools.configuration.cmake.ctest.testExplorerIntegrationEnabled.description%", + "scope": "application" + }, "cmake.parseBuildDiagnostics": { "type": "boolean", "default": true, diff --git a/package.nls.json b/package.nls.json index 96dfa83ca..a3e90a92f 100644 --- a/package.nls.json +++ b/package.nls.json @@ -96,6 +96,7 @@ "comment": "Markdown text between `` should not be translated or localized (they represent literal text) and the capitalization, spacing, and punctuation (including the ``) should not be altered." }, "cmake-tools.configuration.cmake.ctest.allowParallelJobs.description": "Allows ctests to be run in parallel, however the result output may be garbled as a result.", + "cmake-tools.configuration.cmake.ctest.testExplorerIntegrationEnabled.description": "Whether or not the integration with the test explorer is enabled. This is helpful to disable if you prefer using a different extension for test integration.", "cmake-tools.configuration.cmake.parseBuildDiagnostics.description": "Parse compiler output for warnings and errors.", "cmake-tools.configuration.cmake.enabledOutputParsers.description": { "message": "Output parsers to use. Supported parsers `cmake`, `gcc`, `gnuld` for GNULD-style linker output, `msvc` for Microsoft Visual C++, `ghs` for the Green Hills compiler with --no_wrap_diagnostics --brief_diagnostics, and `diab` for the Wind River Diab compiler.", diff --git a/src/config.ts b/src/config.ts index d7d9f6baa..b7029d377 100644 --- a/src/config.ts +++ b/src/config.ts @@ -147,7 +147,7 @@ export interface ExtensionConfigurationSettings { buildToolArgs: string[]; parallelJobs: number | undefined; ctestPath: string; - ctest: { parallelJobs: number; allowParallelJobs: boolean }; + ctest: { parallelJobs: number; allowParallelJobs: boolean; testExplorerIntegrationEnabled: boolean }; parseBuildDiagnostics: boolean; enabledOutputParsers: string[]; debugConfig: CppDebugConfiguration; @@ -344,6 +344,9 @@ export class ConfigurationReader implements vscode.Disposable { get ctestAllowParallelJobs(): boolean { return this.configData.ctest.allowParallelJobs; } + get testExplorerIntegrationEnabled(): boolean { + return this.configData.ctest.testExplorerIntegrationEnabled; + } get parseBuildDiagnostics(): boolean { return !!this.configData.parseBuildDiagnostics; } @@ -526,7 +529,7 @@ export class ConfigurationReader implements vscode.Disposable { buildToolArgs: new vscode.EventEmitter(), parallelJobs: new vscode.EventEmitter(), ctestPath: new vscode.EventEmitter(), - ctest: new vscode.EventEmitter<{ parallelJobs: number; allowParallelJobs: boolean }>(), + ctest: new vscode.EventEmitter<{ parallelJobs: number; allowParallelJobs: boolean; testExplorerIntegrationEnabled: boolean }>(), parseBuildDiagnostics: new vscode.EventEmitter(), enabledOutputParsers: new vscode.EventEmitter(), debugConfig: new vscode.EventEmitter(), diff --git a/src/ctest.ts b/src/ctest.ts index 5b7e3db69..671baa6c1 100644 --- a/src/ctest.ts +++ b/src/ctest.ts @@ -279,53 +279,65 @@ export class CTestDriver implements vscode.Disposable { log.showChannel(); } - if (!testExplorer) { - await this.refreshTests(driver); - } - - if (!testExplorer) { - log.info(localize('no.tests.found', 'No tests found')); - return -1; - } else if (!this.ws.config.ctestAllowParallelJobs) { - const tests = this.testItemCollectionToArray(testExplorer.items); - const run = testExplorer.createTestRun(new vscode.TestRunRequest()); - const ctestArgs = await this.getCTestArgs(driver, customizedTask, testPreset); - const returnCode = await this.runCTestHelper(tests, run, driver, undefined, ctestArgs, undefined, customizedTask, consumer); - run.end(); - return returnCode; - } else { - // below code taken from #3032 PR (before changes in how tests are run) - const ctestpath = await this.ws.getCTestPath(driver.cmakePathFromPreset); - if (ctestpath === null) { - log.info(localize('ctest.path.not.set', 'CTest path is not set')); - return -2; + if (this.ws.config.testExplorerIntegrationEnabled) { + if (!testExplorer) { + await this.refreshTests(driver); } - const ctestArgs = await this.getCTestArgs(driver, customizedTask, testPreset); - - if (!driver.testPreset) { - log.error(localize('test.preset.not.set', 'Test preset is not set')); - return -3; + if (!testExplorer) { + log.info(localize('no.tests.found', 'No tests found')); + return -1; } - const child = driver.executeCommand( - ctestpath, - ctestArgs, - ((customizedTask && consumer) ? consumer : new CTestOutputLogger()), - { environment: await driver.getCTestCommandEnvironment(), cwd: driver.binaryDir }); - const res = await child.result; - // not sure if direct comparison can be made to replace reloadTests with refreshTests - await this.refreshTests(driver); - if (res.retc === null) { - log.info(localize('ctest.run.terminated', 'CTest run was terminated')); - return -1; + if (!this.ws.config.ctestAllowParallelJobs) { + const tests = this.testItemCollectionToArray(testExplorer.items); + const run = testExplorer.createTestRun(new vscode.TestRunRequest()); + const ctestArgs = await this.getCTestArgs(driver, customizedTask, testPreset); + const returnCode = await this.runCTestHelper(tests, run, driver, undefined, ctestArgs, undefined, customizedTask, consumer); + run.end(); + return returnCode; } else { - log.info(localize('ctest.finished.with.code', 'CTest finished with return code {0}', res.retc)); + const retc = await this.runCTestDirectly(driver, customizedTask, testPreset, consumer); + + // not sure if direct comparison can be made to replace reloadTests with refreshTests + await this.refreshTests(driver); + return retc; } - return res.retc; + } else { + return this.runCTestDirectly(driver, customizedTask, testPreset, consumer); } } + private async runCTestDirectly(driver: CMakeDriver, customizedTask: boolean = false, testPreset?: TestPreset, consumer?: proc.OutputConsumer): Promise { + // below code taken from #3032 PR (before changes in how tests are run) + const ctestpath = await this.ws.getCTestPath(driver.cmakePathFromPreset); + if (ctestpath === null) { + log.info(localize('ctest.path.not.set', 'CTest path is not set')); + return -2; + } + + const ctestArgs = await this.getCTestArgs(driver, customizedTask, testPreset); + + if (!driver.testPreset) { + log.error('test.preset.not.set', 'Test preset is not set'); + return -3; + } + + const child = driver.executeCommand( + ctestpath, + ctestArgs, + ((customizedTask && consumer) ? consumer : new CTestOutputLogger()), + { environment: await driver.getCTestCommandEnvironment(), cwd: driver.binaryDir }); + const res = await child.result; + if (res.retc === null) { + log.info(localize('ctest.run.terminated', 'CTest run was terminated')); + return -1; + } else { + log.info(localize('ctest.finished.with.code', 'CTest finished with return code {0}', res.retc)); + } + return res.retc; + } + private ctestsEnqueued(tests: vscode.TestItem[], run: vscode.TestRun) { for (const test of tests) { if (test.children.size > 0) { @@ -519,6 +531,13 @@ export class CTestDriver implements vscode.Disposable { * @returns 0 when successful */ async refreshTests(driver: CMakeDriver): Promise { + // NOTE: If the cmake.ctest.testExplorerIntegrationEnabled is disabled, we should return early and not initialize + // the testExplorer. + if (!driver.config.testExplorerIntegrationEnabled) { + // Test Explorer integration is disabled + return -1; + } + if (util.isTestMode()) { // ProjectController can't be initialized in test mode, so we don't have a usable test explorer return 0; @@ -621,6 +640,7 @@ export class CTestDriver implements vscode.Disposable { } clearTests(driver: CMakeDriver) { + // NOTE: We expect the testExplorer to be undefined when the cmake.ctest.testExplorerIntegrationEnabled is disabled. if (!testExplorer) { return; } @@ -684,6 +704,7 @@ export class CTestDriver implements vscode.Disposable { } private async runTestHandler(request: vscode.TestRunRequest, cancellation: vscode.CancellationToken) { + // NOTE: We expect the testExplorer to be undefined when the cmake.ctest.testExplorerIntegrationEnabled is disabled. if (!testExplorer) { return; } @@ -918,6 +939,7 @@ export class CTestDriver implements vscode.Disposable { } private async debugTestHandler(request: vscode.TestRunRequest, cancellation: vscode.CancellationToken) { + // NOTE: We expect the testExplorer to be undefined when the cmake.ctest.testExplorerIntegrationEnabled is disabled. if (!testExplorer) { return; } @@ -976,6 +998,7 @@ export class CTestDriver implements vscode.Disposable { * Should only be called by refreshTests since it adds tests to the controller. */ private ensureTestExplorerInitialized(): vscode.TestController { + // NOTE: We expect the testExplorer to be undefined when the cmake.ctest.testExplorerIntegrationEnabled is disabled. if (!testExplorer) { testExplorer = vscode.tests.createTestController('cmake-tools.CTest', 'CTest'); @@ -1007,6 +1030,7 @@ export class CTestDriver implements vscode.Disposable { } addTestExplorerRoot(folder: string) { + // NOTE: We expect the testExplorer to be undefined when the cmake.ctest.testExplorerIntegrationEnabled is disabled. if (!testExplorer) { return; } @@ -1016,6 +1040,7 @@ export class CTestDriver implements vscode.Disposable { } removeTestExplorerRoot(folder: string) { + // NOTE: We expect the testExplorer to be undefined when the cmake.ctest.testExplorerIntegrationEnabled is disabled. if (!testExplorer) { return; } @@ -1029,6 +1054,7 @@ export class CTestDriver implements vscode.Disposable { * Since there's no way to reveal the explorer itself, this function reveals the first test in the test explorer. */ async revealTestExplorer(): Promise { + // NOTE: We expect the testExplorer to be undefined when the cmake.ctest.testExplorerIntegrationEnabled is disabled. if (!testExplorer) { return false; } @@ -1045,3 +1071,10 @@ export class CTestDriver implements vscode.Disposable { // Only have one instance of the test controller let testExplorer: vscode.TestController | undefined; + +export function deIntegrateTestExplorer(): void { + if (testExplorer) { + testExplorer.dispose(); + testExplorer = undefined; + } +} diff --git a/src/extension.ts b/src/extension.ts index 7801c6b84..74d5e5c4c 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -47,6 +47,7 @@ import { DebugAdapterNamedPipeServerDescriptorFactory } from './debug/debugAdapt import { getCMakeExecutableInformation } from './cmake/cmakeExecutable'; import { DebuggerInformation, getDebuggerPipeName } from './debug/debuggerConfigureDriver'; import { DebugConfigurationProvider, DynamicDebugConfigurationProvider } from './debug/debugConfigurationProvider'; +import { deIntegrateTestExplorer } from './ctest'; nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })(); const localize: nls.LocalizeFunc = nls.loadMessageBundle(); @@ -122,6 +123,18 @@ export class ExtensionManager implements vscode.Disposable { // initialize the state of the cmake exe await getCMakeExecutableInformation(this.workspaceConfig.rawCMakePath); + await util.setContextValue("cmake:testExplorerIntegrationEnabled", this.workspaceConfig.testExplorerIntegrationEnabled); + this.workspaceConfig.onChange("ctest", async (value) => { + await util.setContextValue("cmake:testExplorerIntegrationEnabled", value.testExplorerIntegrationEnabled); + if (!value.testExplorerIntegrationEnabled) { + // Dynamically de-integrate the test explorer. + deIntegrateTestExplorer(); + } else { + // Attempt to refresh the tests when dynamically re-integrating the test explorer. + await getActiveProject()?.refreshTests(); + } + }); + this.onDidChangeActiveTextEditorSub = vscode.window.onDidChangeActiveTextEditor(e => this.onDidChangeActiveTextEditor(e), this); this.projectController.onAfterAddFolder(async (folderProjectMap: FolderProjectType) => { diff --git a/test/unit-tests/config.test.ts b/test/unit-tests/config.test.ts index 2e96273b0..e9a376940 100644 --- a/test/unit-tests/config.test.ts +++ b/test/unit-tests/config.test.ts @@ -24,7 +24,8 @@ function createConfig(conf: Partial): Configurat ctestPath: '', ctest: { parallelJobs: 0, - allowParallelJobs: false + allowParallelJobs: false, + testExplorerIntegrationEnabled: true }, parseBuildDiagnostics: true, enabledOutputParsers: [],