Merge pull request #1575 from github/koesie10/reset-config

Reset VSCode configuration between tests

extensions/ql-vscode/src/config.ts

@@ -4,6 +4,8 @@ import { DistributionManager } from './distribution';
 import { logger } from './logging';
 import { ONE_DAY_IN_MS } from './pure/time';
 
+export const ALL_SETTINGS: Setting[] = [];
+
 /** Helper class to look up a labelled (and possibly nested) setting. */
 export class Setting {
   name: string;
@@ -12,6 +14,7 @@ export class Setting {
   constructor(name: string, parent?: Setting) {
     this.name = name;
     this.parent = parent;
+    ALL_SETTINGS.push(this);
   }
 
   get qualifiedName(): string {
@@ -36,6 +39,18 @@ export class Setting {
     return workspace.getConfiguration(this.parent.qualifiedName).update(this.name, value, target);
   }
 
+  inspect<T>(): InspectionResult<T> | undefined {
+    if (this.parent === undefined) {
+      throw new Error('Cannot update the value of a root setting.');
+    }
+    return workspace.getConfiguration(this.parent.qualifiedName).inspect(this.name);
+  }
+}
+
+export interface InspectionResult<T> {
+  globalValue?: T;
+  workspaceValue?: T,
+  workspaceFolderValue?: T,
 }
 
 const ROOT_SETTING = new Setting('codeQL');

extensions/ql-vscode/src/vscode-tests/cli-integration/global.helper.ts

@@ -4,9 +4,11 @@ import * as fs from 'fs-extra';
 import fetch from 'node-fetch';
 
 import { fail } from 'assert';
-import { commands, ConfigurationTarget, extensions, workspace } from 'vscode';
+import { commands, extensions, workspace } from 'vscode';
 import { CodeQLExtensionInterface } from '../../extension';
 import { DatabaseManager } from '../../databases';
+import { getTestSetting } from '../test-config';
+import { CUSTOM_CODEQL_PATH_SETTING } from '../../config';
 
 // This file contains helpers shared between actual tests.
 
@@ -58,7 +60,7 @@ export default function(mocha: Mocha) {
   // Set the CLI version here before activation to ensure we don't accidentally try to download a cli
   (mocha.options as any).globalSetup.push(
     async () => {
-      await workspace.getConfiguration().update('codeQL.cli.executablePath', process.env.CLI_PATH, ConfigurationTarget.Global);
+      await getTestSetting(CUSTOM_CODEQL_PATH_SETTING)?.setInitialTestValue(process.env.CLI_PATH);
     }
   );
 

extensions/ql-vscode/src/vscode-tests/index-template.ts

@@ -1,8 +1,9 @@
 import * as path from 'path';
 import * as Mocha from 'mocha';
-import * as glob from 'glob';
+import * as glob from 'glob-promise';
 import { ensureCli } from './ensureCli';
 import { env } from 'vscode';
+import { testConfigHelper } from './test-config';
 
 
 // Use this handler to avoid swallowing unhandled rejections.
@@ -57,44 +58,42 @@ export async function runTestsInDirectory(testsRoot: string, useCli = false): Pr
 
   await ensureCli(useCli);
 
+  console.log(`Adding test cases and helpers from ${testsRoot}`);
+
+  const files = await glob('**/**.js', { cwd: testsRoot });
+
+  // Add test files to the test suite
+  files
+    .filter(f => f.endsWith('.test.js'))
+    .forEach(f => {
+      console.log(`Adding test file ${f}`);
+      mocha.addFile(path.resolve(testsRoot, f));
+    });
+
+  // Setup the config helper. This needs to run before other helpers so any config they setup
+  // is restored.
+  await testConfigHelper(mocha);
+
+  // Add helpers. Helper files add global setup and teardown blocks
+  // for a test run.
+  files
+    .filter(f => f.endsWith('.helper.js'))
+    .forEach(f => {
+      console.log(`Executing helper ${f}`);
+      // eslint-disable-next-line @typescript-eslint/no-var-requires
+      const helper = require(path.resolve(testsRoot, f)).default;
+      helper(mocha);
+    });
+
   return new Promise((resolve, reject) => {
-    console.log(`Adding test cases and helpers from ${testsRoot}`);
-    glob('**/**.js', { cwd: testsRoot }, (err, files) => {
-      if (err) {
-        return reject(err);
+    // Run the mocha test
+    mocha.run(failures => {
+      if (failures > 0) {
+        reject(new Error(`${failures} tests failed.`));
+        return;
       }
 
-      try {
-        // Add test files to the test suite
-        files
-          .filter(f => f.endsWith('.test.js'))
-          .forEach(f => {
-            console.log(`Adding test file ${f}`);
-            mocha.addFile(path.resolve(testsRoot, f));
-          });
-
-        // Add helpers. Helper files add global setup and teardown blocks
-        // for a test run.
-        files
-          .filter(f => f.endsWith('.helper.js'))
-          .forEach(f => {
-            console.log(`Executing helper ${f}`);
-            // eslint-disable-next-line @typescript-eslint/no-var-requires
-            const helper = require(path.resolve(testsRoot, f)).default;
-            helper(mocha);
-          });
-
-        // Run the mocha test
-        mocha.run(failures => {
-          if (failures > 0) {
-            reject(new Error(`${failures} tests failed.`));
-          } else {
-            resolve();
-          }
-        });
-      } catch (err) {
-        reject(err);
-      }
+      resolve();
     });
   });
 }

extensions/ql-vscode/src/vscode-tests/test-config.ts

@@ -0,0 +1,85 @@
+import { ConfigurationTarget } from 'vscode';
+import { ALL_SETTINGS, InspectionResult, Setting } from '../config';
+
+class TestSetting<T> {
+  private initialSettingState: InspectionResult<T> | undefined;
+
+  constructor(
+    public readonly setting: Setting,
+    private initialTestValue: T | undefined = undefined
+  ) { }
+
+  public async get(): Promise<T | undefined> {
+    return this.setting.getValue();
+  }
+
+  public async set(value: T | undefined, target: ConfigurationTarget = ConfigurationTarget.Global): Promise<void> {
+    await this.setting.updateValue(value, target);
+  }
+
+  public async setInitialTestValue(value: T | undefined) {
+    this.initialTestValue = value;
+  }
+
+  public async initialSetup() {
+    this.initialSettingState = this.setting.inspect();
+
+    // Unfortunately it's not well-documented how to check whether we can write to a workspace
+    // configuration. This is the best I could come up with. It only fails for initial test values
+    // which are not undefined.
+    if (this.initialSettingState?.workspaceValue !== undefined) {
+      await this.set(this.initialTestValue, ConfigurationTarget.Workspace);
+    }
+    if (this.initialSettingState?.workspaceFolderValue !== undefined) {
+      await this.set(this.initialTestValue, ConfigurationTarget.WorkspaceFolder);
+    }
+
+    await this.setup();
+  }
+
+  public async setup() {
+    await this.set(this.initialTestValue, ConfigurationTarget.Global);
+  }
+
+  public async restoreToInitialValues() {
+    const state = this.setting.inspect();
+
+    // We need to check the state of the setting before we restore it. This is less important for the global
+    // configuration target, but the workspace/workspace folder configuration might not even exist. If they
+    // don't exist, VSCode will error when trying to write the new value (even if that value is undefined).
+    if (state?.globalValue !== this.initialSettingState?.globalValue) {
+      await this.set(this.initialSettingState?.globalValue, ConfigurationTarget.Global);
+    }
+    if (state?.workspaceValue !== this.initialSettingState?.workspaceValue) {
+      await this.set(this.initialSettingState?.workspaceValue, ConfigurationTarget.Workspace);
+    }
+    if (state?.workspaceFolderValue !== this.initialSettingState?.workspaceFolderValue) {
+      await this.set(this.initialSettingState?.workspaceFolderValue, ConfigurationTarget.WorkspaceFolder);
+    }
+  }
+}
+
+// The test settings are all settings in ALL_SETTINGS which don't have any children
+const TEST_SETTINGS = ALL_SETTINGS
+  .filter(setting => ALL_SETTINGS.filter(s => s.parent === setting).length === 0)
+  .map(setting => new TestSetting(setting));
+
+export const getTestSetting = (setting: Setting): TestSetting<unknown> | undefined => {
+  return TEST_SETTINGS.find(testSetting => testSetting.setting === setting);
+};
+
+export const testConfigHelper = async (mocha: Mocha) => {
+  // Read in all current settings
+  await Promise.all(TEST_SETTINGS.map(setting => setting.initialSetup()));
+
+  mocha.rootHooks({
+    async beforeEach() {
+      // Reset the settings to their initial values before each test
+      await Promise.all(TEST_SETTINGS.map(setting => setting.setup()));
+    },
+    async afterAll() {
+      // Restore all settings to their default values after each test suite
+      await Promise.all(TEST_SETTINGS.map(setting => setting.restoreToInitialValues()));
+    }
+  });
+};