Skip to content

Commit

Permalink
Add a setting to enable/disable the Test Explorer Integration (#3545)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@gcampbell-msft
gcampbell-msft authored Jan 23, 2024
1 parent 2bdf3bf commit b486c5a
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 47 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
18 changes: 12 additions & 6 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
{
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions package.nls.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
7 changes: 5 additions & 2 deletions src/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -526,7 +529,7 @@ export class ConfigurationReader implements vscode.Disposable {
buildToolArgs: new vscode.EventEmitter<string[]>(),
parallelJobs: new vscode.EventEmitter<number>(),
ctestPath: new vscode.EventEmitter<string>(),
ctest: new vscode.EventEmitter<{ parallelJobs: number; allowParallelJobs: boolean }>(),
ctest: new vscode.EventEmitter<{ parallelJobs: number; allowParallelJobs: boolean; testExplorerIntegrationEnabled: boolean }>(),
parseBuildDiagnostics: new vscode.EventEmitter<boolean>(),
enabledOutputParsers: new vscode.EventEmitter<string[]>(),
debugConfig: new vscode.EventEmitter<CppDebugConfiguration>(),
Expand Down
109 changes: 71 additions & 38 deletions src/ctest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<number> {
// 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) {
Expand Down Expand Up @@ -519,6 +531,13 @@ export class CTestDriver implements vscode.Disposable {
* @returns 0 when successful
*/
async refreshTests(driver: CMakeDriver): Promise<number> {
// 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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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');

Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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<boolean> {
// NOTE: We expect the testExplorer to be undefined when the cmake.ctest.testExplorerIntegrationEnabled is disabled.
if (!testExplorer) {
return false;
}
Expand All @@ -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;
}
}
13 changes: 13 additions & 0 deletions src/extension.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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) => {
Expand Down
3 changes: 2 additions & 1 deletion test/unit-tests/config.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ function createConfig(conf: Partial<ExtensionConfigurationSettings>): Configurat
ctestPath: '',
ctest: {
parallelJobs: 0,
allowParallelJobs: false
allowParallelJobs: false,
testExplorerIntegrationEnabled: true
},
parseBuildDiagnostics: true,
enabledOutputParsers: [],
Expand Down

0 comments on commit b486c5a

Please sign in to comment.