Merge pull request #2033 from github/koesie10/mocked-config

Mock config instead of writing settings files

extensions/ql-vscode/src/config.ts

@@ -56,15 +56,6 @@ export class Setting {
       .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> {

extensions/ql-vscode/test/vscode-tests/cli-integration/jest.setup.ts

@@ -8,23 +8,20 @@ import { dirname } from "path";
 import fetch from "node-fetch";
 import { DB_URL, dbLoc, setStoragePath, storagePath } from "./global.helper";
 import * as tmp from "tmp";
-import { getTestSetting } from "../test-config";
 import { CUSTOM_CODEQL_PATH_SETTING } from "../../../src/config";
-import { extensions, workspace } from "vscode";
-
-import baseJestSetup from "../jest.setup";
-
-export default baseJestSetup;
+import { ConfigurationTarget, env, extensions, workspace } from "vscode";
+import { beforeEachAction } from "../test-config";
 
 // create an extension storage location
 let removeStorage: tmp.DirResult["removeCallback"] | undefined;
 
 beforeAll(async () => {
   // Set the CLI version here before activation to ensure we don't accidentally try to download a cli
-  await getTestSetting(CUSTOM_CODEQL_PATH_SETTING)?.setInitialTestValue(
+  await beforeEachAction();
+  await CUSTOM_CODEQL_PATH_SETTING.updateValue(
     process.env.CLI_PATH,
+    ConfigurationTarget.Workspace,
   );
-  await getTestSetting(CUSTOM_CODEQL_PATH_SETTING)?.setup();
 
   // ensure the test database is downloaded
   mkdirpSync(dirname(dbLoc));
@@ -78,6 +75,17 @@ beforeAll(async () => {
   await extensions.getExtension("GitHub.vscode-codeql")?.activate();
 });
 
+beforeEach(async () => {
+  jest.spyOn(env, "openExternal").mockResolvedValue(false);
+
+  await beforeEachAction();
+
+  await CUSTOM_CODEQL_PATH_SETTING.updateValue(
+    process.env.CLI_PATH,
+    ConfigurationTarget.Workspace,
+  );
+});
+
 // ensure extension is cleaned up.
 afterAll(async () => {
   // ensure temp directory is cleaned up.

extensions/ql-vscode/test/vscode-tests/cli-integration/remote-queries/variant-analysis-submission-integration.test.ts

@@ -3,6 +3,7 @@ import { resolve } from "path";
 import {
   authentication,
   commands,
+  ConfigurationTarget,
   extensions,
   QuickPickItem,
   TextDocument,
@@ -12,7 +13,10 @@ import {
 
 import { CodeQLExtensionInterface } from "../../../../src/extension";
 import { MockGitHubApiServer } from "../../../../src/mocks/mock-gh-api-server";
-import { mockConfiguration } from "../../utils/configuration-helpers";
+import {
+  CANARY_FEATURES,
+  setRemoteControllerRepo,
+} from "../../../../src/config";
 
 jest.setTimeout(30_000);
 
@@ -36,17 +40,8 @@ describe("Variant Analysis Submission Integration", () => {
   let showErrorMessageSpy: jest.SpiedFunction<typeof window.showErrorMessage>;
 
   beforeEach(async () => {
-    mockConfiguration({
-      values: {
-        codeQL: {
-          canary: true,
-        },
-        "codeQL.variantAnalysis": {
-          liveResults: true,
-          controllerRepo: "github/vscode-codeql",
-        },
-      },
-    });
+    await CANARY_FEATURES.updateValue(true, ConfigurationTarget.Global);
+    await setRemoteControllerRepo("github/vscode-codeql");
 
     jest.spyOn(authentication, "getSession").mockResolvedValue({
       id: "test",

extensions/ql-vscode/test/vscode-tests/jest.setup.ts

@@ -1,10 +1,8 @@
 import { env } from "vscode";
-import { jestTestConfigHelper } from "./test-config";
+import { beforeEachAction } from "./test-config";
 
-(env as any).openExternal = () => {
-  /**/
-};
+beforeEach(async () => {
+  jest.spyOn(env, "openExternal").mockResolvedValue(false);
 
-export default async function setupEnv() {
-  await jestTestConfigHelper();
-}
+  await beforeEachAction();
+});

extensions/ql-vscode/test/vscode-tests/minimal-workspace/config.test.ts

@@ -6,8 +6,13 @@ import {
   QueryHistoryConfigListener,
   QueryServerConfigListener,
 } from "../../../src/config";
+import { vscodeGetConfigurationMock } from "../test-config";
 
 describe("config listeners", () => {
+  beforeEach(() => {
+    vscodeGetConfigurationMock.mockRestore();
+  });
+
   interface TestConfig<T> {
     clazz: new () => ConfigListener;
     settings: Array<{

extensions/ql-vscode/test/vscode-tests/minimal-workspace/databases/db-panel-rendering.test.ts

@@ -13,7 +13,7 @@ import { DbTreeViewItem } from "../../../../src/databases/ui/db-tree-view-item";
 import { ExtensionApp } from "../../../../src/common/vscode/vscode-app";
 import { createMockExtensionContext } from "../../../factories/extension-context";
 import { createDbConfig } from "../../../factories/db-config-factories";
-import { mockConfiguration } from "../../utils/configuration-helpers";
+import { setRemoteControllerRepo } from "../../../../src/config";
 
 describe("db panel rendering nodes", () => {
   const workspaceStoragePath = join(__dirname, "test-workspace-storage");
@@ -50,12 +50,8 @@ describe("db panel rendering nodes", () => {
   });
 
   describe("when controller repo is not set", () => {
-    mockConfiguration({
-      values: {
-        "codeQL.variantAnalysis": {
-          controllerRepo: undefined,
-        },
-      },
+    beforeEach(async () => {
+      await setRemoteControllerRepo(undefined);
     });
 
     it("should not have any items", async () => {
@@ -81,14 +77,8 @@ describe("db panel rendering nodes", () => {
   });
 
   describe("when controller repo is set", () => {
-    beforeEach(() => {
-      mockConfiguration({
-        values: {
-          "codeQL.variantAnalysis": {
-            controllerRepo: "github/codeql",
-          },
-        },
-      });
+    beforeEach(async () => {
+      await setRemoteControllerRepo("github/codeql");
     });
 
     it("should render default remote nodes when the config is empty", async () => {

extensions/ql-vscode/test/vscode-tests/minimal-workspace/databases/db-panel.test.ts

@@ -9,7 +9,7 @@ import { DbTreeViewItem } from "../../../../src/databases/ui/db-tree-view-item";
 import { ExtensionApp } from "../../../../src/common/vscode/vscode-app";
 import { createMockExtensionContext } from "../../../factories/extension-context";
 import { createDbConfig } from "../../../factories/db-config-factories";
-import { mockConfiguration } from "../../utils/configuration-helpers";
+import { setRemoteControllerRepo } from "../../../../src/config";
 
 describe("db panel", () => {
   const workspaceStoragePath = join(__dirname, "test-workspace-storage");
@@ -40,13 +40,7 @@ describe("db panel", () => {
   beforeEach(async () => {
     await ensureDir(workspaceStoragePath);
 
-    mockConfiguration({
-      values: {
-        "codeQL.variantAnalysis": {
-          controllerRepo: "github/codeql",
-        },
-      },
-    });
+    await setRemoteControllerRepo("github/codeql");
   });
 
   afterEach(async () => {

extensions/ql-vscode/test/vscode-tests/no-workspace/databases/db-panel-selection.test.ts

@@ -15,7 +15,7 @@ import {
 import { ExtensionApp } from "../../../../src/common/vscode/vscode-app";
 import { createMockExtensionContext } from "../../../factories/extension-context";
 import { createDbConfig } from "../../../factories/db-config-factories";
-import { mockConfiguration } from "../../utils/configuration-helpers";
+import { setRemoteControllerRepo } from "../../../../src/config";
 
 describe("db panel selection", () => {
   const workspaceStoragePath = join(__dirname, "test-workspace-storage");
@@ -46,13 +46,7 @@ describe("db panel selection", () => {
   beforeEach(async () => {
     await ensureDir(workspaceStoragePath);
 
-    mockConfiguration({
-      values: {
-        "codeQL.variantAnalysis": {
-          controllerRepo: "github/codeql",
-        },
-      },
-    });
+    await setRemoteControllerRepo("github/codeql");
   });
 
   afterEach(async () => {

extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts

@@ -13,6 +13,7 @@ import { UserCancellationException } from "../../../src/commandRunner";
 import { ENABLE_TELEMETRY } from "../../../src/config";
 import * as Config from "../../../src/config";
 import { createMockExtensionContext } from "./index";
+import { vscodeGetConfigurationMock } from "../test-config";
 import { redactableError } from "../../../src/pure/errors";
 
 // setting preferences can trigger lots of background activity
@@ -41,6 +42,8 @@ describe("telemetry reporting", () => {
   >;
 
   beforeEach(async () => {
+    vscodeGetConfigurationMock.mockRestore();
+
     try {
       // in case a previous test has accidentally activated this extension,
       // need to disable it first.

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

@@ -1,126 +1,225 @@
+import {
+  ConfigurationScope,
+  ConfigurationTarget,
+  workspace,
+  WorkspaceConfiguration as VSCodeWorkspaceConfiguration,
+} from "vscode";
 import { readFileSync } from "fs-extra";
 import { join } from "path";
-import { ConfigurationTarget } from "vscode";
-import { ALL_SETTINGS, InspectionResult, Setting } from "../../src/config";
 
-class TestSetting<T> {
-  private initialSettingState: InspectionResult<T> | undefined;
+function getIn(object: any, path: string): any {
+  const parts = path.split(".");
+  let current = object;
+  for (const part of parts) {
+    current = current[part];
+    if (current === undefined) {
+      return undefined;
+    }
+  }
+  return current;
+}
 
-  constructor(
-    public readonly setting: Setting,
-    private initialTestValue: T | undefined = undefined,
-  ) {}
+function setIn(object: any, path: string, value: any): void {
+  const parts = path.split(".");
+  let current = object;
+  for (const part of parts.slice(0, -1)) {
+    if (current[part] === undefined) {
+      current[part] = {};
+    }
+    current = current[part];
+  }
+  current[parts[parts.length - 1]] = value;
+}
 
-  public async get(): Promise<T | undefined> {
-    return this.setting.getValue();
+interface WorkspaceConfiguration {
+  scope?: ConfigurationTarget;
+
+  get<T>(section: string | undefined, key: string): T | undefined;
+  has(section: string | undefined, key: string): boolean;
+  update(section: string | undefined, key: string, value: unknown): void;
+}
+
+class InMemoryConfiguration implements WorkspaceConfiguration {
+  private readonly values: Record<string, unknown> = {};
+
+  public constructor(public readonly scope: ConfigurationTarget) {}
+
+  public get<T>(section: string | undefined, key: string): T | undefined {
+    return getIn(this.values, this.getKey(section, key)) as T | undefined;
   }
 
-  public async set(
-    value: T | undefined,
-    target: ConfigurationTarget = ConfigurationTarget.Global,
-  ): Promise<void> {
-    await this.setting.updateValue(value, target);
+  public has(section: string | undefined, key: string): boolean {
+    return getIn(this.values, this.getKey(section, key)) !== undefined;
   }
 
-  public async setInitialTestValue(value: T | undefined) {
-    this.initialTestValue = value;
+  public update(
+    section: string | undefined,
+    key: string,
+    value: unknown,
+  ): void {
+    setIn(this.values, this.getKey(section, key), value);
   }
 
-  public async initialSetup() {
-    this.initialSettingState = this.setting.inspect();
+  private getKey(section: string | undefined, key: string): string {
+    return section ? `${section}.${key}` : key;
+  }
+}
 
-    // 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,
-      );
-    }
+class DefaultConfiguration implements WorkspaceConfiguration {
+  private readonly values: Record<string, unknown> = {};
 
-    await this.setup();
+  public constructor(configurations: Record<string, { default: unknown }>) {
+    for (const [section, config] of Object.entries(configurations)) {
+      setIn(this.values, section, config.default);
+    }
   }
 
-  public async setup() {
-    await this.set(this.initialTestValue, ConfigurationTarget.Global);
+  public get<T>(section: string | undefined, key: string): T | undefined {
+    return getIn(this.values, this.getKey(section, key)) as T | undefined;
   }
 
-  public async restoreToInitialValues() {
-    const state = this.setting.inspect();
+  public has(section: string | undefined, key: string): boolean {
+    return getIn(this.values, this.getKey(section, key)) !== undefined;
+  }
 
-    // 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,
-      );
+  public update(
+    _section: string | undefined,
+    _key: string,
+    _value: unknown,
+  ): void {
+    throw new Error("Cannot update default configuration");
+  }
+
+  private getKey(section: string | undefined, key: string): string {
+    return section ? `${section}.${key}` : key;
+  }
+}
+
+class ChainedInMemoryConfiguration {
+  constructor(private readonly configurations: WorkspaceConfiguration[]) {}
+
+  public getConfiguration(target: ConfigurationTarget) {
+    const configuration = this.configurations.find(
+      (configuration) => configuration.scope === target,
+    );
+
+    if (configuration === undefined) {
+      throw new Error(`Unknown configuration target ${target}`);
     }
-    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,
-      );
+
+    return configuration;
+  }
+
+  public get<T>(section: string | undefined, key: string): T | undefined {
+    for (const configuration of this.configurations) {
+      if (configuration.has(section, key)) {
+        return configuration.get(section, key);
+      }
     }
+    return undefined;
+  }
+
+  public has(section: string | undefined, key: string): boolean {
+    return this.configurations.some((configuration) =>
+      configuration.has(section, key),
+    );
+  }
+
+  public update<T>(
+    section: string | undefined,
+    key: string,
+    value: T,
+    target: ConfigurationTarget,
+  ): void {
+    const configuration = this.getConfiguration(target);
+
+    configuration.update(section, key, value);
   }
 }
 
 // Public configuration keys are the ones defined in the package.json.
 // These keys are documented in the settings page. Other keys are
 // internal and not documented.
-const PKG_CONFIGURATION: Record<string, any> =
-  (function initConfigurationKeys() {
-    // Note we are using synchronous file reads here. This is fine because
-    // we are in tests.
-    const pkg = JSON.parse(
-      readFileSync(join(__dirname, "../../package.json"), "utf-8"),
+const packageConfiguration: Record<
+  string,
+  {
+    default: any | undefined;
+  }
+> = (function initConfigurationKeys() {
+  // Note we are using synchronous file reads here. This is fine because
+  // we are in tests.
+  const pkg = JSON.parse(
+    readFileSync(join(__dirname, "../../package.json"), "utf-8"),
+  );
+  return pkg.contributes.configuration.properties;
+})();
+
+export const vsCodeGetConfiguration = workspace.getConfiguration;
+export let vscodeGetConfigurationMock: jest.SpiedFunction<
+  typeof workspace.getConfiguration
+>;
+
+export const beforeEachAction = async () => {
+  const defaultConfiguration = new DefaultConfiguration(packageConfiguration);
+
+  const configuration = new ChainedInMemoryConfiguration([
+    new InMemoryConfiguration(ConfigurationTarget.WorkspaceFolder),
+    new InMemoryConfiguration(ConfigurationTarget.Workspace),
+    new InMemoryConfiguration(ConfigurationTarget.Global),
+    defaultConfiguration,
+  ]);
+
+  vscodeGetConfigurationMock = jest
+    .spyOn(workspace, "getConfiguration")
+    .mockImplementation(
+      (
+        section?: string,
+        scope?: ConfigurationScope | null,
+      ): VSCodeWorkspaceConfiguration => {
+        if (scope) {
+          throw new Error("Scope is not supported in tests");
+        }
+
+        return {
+          get(key: string, defaultValue?: unknown) {
+            return configuration.get(section, key) ?? defaultValue;
+          },
+          has(key: string) {
+            return configuration.has(section, key);
+          },
+          inspect(_key: string) {
+            throw new Error("inspect is not supported in tests");
+          },
+          async update(
+            key: string,
+            value: unknown,
+            configurationTarget?: ConfigurationTarget | boolean | null,
+            overrideInLanguage?: boolean,
+          ) {
+            if (overrideInLanguage) {
+              throw new Error("overrideInLanguage is not supported in tests");
+            }
+
+            function getActualConfigurationTarget(): ConfigurationTarget {
+              if (
+                configurationTarget === undefined ||
+                configurationTarget === null
+              ) {
+                return ConfigurationTarget.Global;
+              }
+              if (typeof configurationTarget === "boolean") {
+                return configurationTarget
+                  ? ConfigurationTarget.Workspace
+                  : ConfigurationTarget.Global;
+              }
+              return configurationTarget;
+            }
+
+            const target = getActualConfigurationTarget();
+
+            configuration.update(section, key, value, target);
+          },
+        };
+      },
     );
-    return pkg.contributes.configuration.properties;
-  })();
-
-// The test settings are all settings in ALL_SETTINGS which don't have any children
-// and are also not hidden settings like the codeQL.canary.
-const TEST_SETTINGS = ALL_SETTINGS.filter(
-  (setting) =>
-    setting.qualifiedName in PKG_CONFIGURATION && !setting.hasChildren,
-).map((setting) => new TestSetting(setting));
-
-export const getTestSetting = (
-  setting: Setting,
-): TestSetting<unknown> | undefined => {
-  return TEST_SETTINGS.find((testSetting) => testSetting.setting === setting);
-};
-
-export const jestTestConfigHelper = async () => {
-  // Read in all current settings
-  await Promise.all(TEST_SETTINGS.map((setting) => setting.initialSetup()));
-
-  beforeEach(async () => {
-    // Reset the settings to their initial values before each test
-    await Promise.all(TEST_SETTINGS.map((setting) => setting.setup()));
-  });
-
-  afterAll(async () => {
-    // Restore all settings to their default values after each test suite
-    // Only do this outside of CI since the sometimes hangs on CI.
-    if (process.env.CI !== "true") {
-      await Promise.all(
-        TEST_SETTINGS.map((setting) => setting.restoreToInitialValues()),
-      );
-    }
-  });
 };

extensions/ql-vscode/test/vscode-tests/utils/configuration-helpers.ts

@@ -1,53 +0,0 @@
-import { workspace } from "vscode";
-
-type MockConfigurationConfig = {
-  values: {
-    [section: string]: {
-      [scope: string]: any | (() => any);
-    };
-  };
-};
-
-export function mockConfiguration(config: MockConfigurationConfig) {
-  const originalGetConfiguration = workspace.getConfiguration;
-
-  jest
-    .spyOn(workspace, "getConfiguration")
-    .mockImplementation((section, scope) => {
-      const configuration = originalGetConfiguration(section, scope);
-
-      return {
-        get(key: string, defaultValue?: unknown) {
-          if (
-            section &&
-            config.values[section] &&
-            config.values[section][key]
-          ) {
-            const value = config.values[section][key];
-            return typeof value === "function" ? value() : value;
-          }
-
-          return configuration.get(key, defaultValue);
-        },
-        has(key: string) {
-          return configuration.has(key);
-        },
-        inspect(key: string) {
-          return configuration.inspect(key);
-        },
-        update(
-          key: string,
-          value: unknown,
-          configurationTarget?: boolean,
-          overrideInLanguage?: boolean,
-        ) {
-          return configuration.update(
-            key,
-            value,
-            configurationTarget,
-            overrideInLanguage,
-          );
-        },
-      };
-    });
-}