Merge remote-tracking branch 'origin/main' into dbartol/join-order-threshold

extensions/ql-vscode/CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## [UNRELEASED]
 
+- Fix a bug where results created in older versions were thought to be unsuccessful. [#1605](https://github.com/github/vscode-codeql/pull/1605)
+
 ## 1.7.1 - 12 October 2022
 
 - Fix a bug where it was not possible to add a database folder if the folder name starts with `db-`. [#1565](https://github.com/github/vscode-codeql/pull/1565)

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/databases.ts

@@ -559,9 +559,6 @@ export class DatabaseManager extends DisposableObject {
     super();
 
     qs.onStart(this.reregisterDatabases.bind(this));
-
-    // Let this run async.
-    void this.loadPersistedState();
   }
 
   public async openDatabase(
@@ -691,7 +688,7 @@ export class DatabaseManager extends DisposableObject {
     return item;
   }
 
-  private async loadPersistedState(): Promise<void> {
+  public async loadPersistedState(): Promise<void> {
     return withProgress({
       location: vscode.ProgressLocation.Notification
     },

extensions/ql-vscode/src/extension.ts

@@ -452,6 +452,10 @@ async function activateWithInstalledDistribution(
 
   void logger.log('Initializing database manager.');
   const dbm = new DatabaseManager(ctx, qs, cliServer, logger);
+
+  // Let this run async.
+  void dbm.loadPersistedState();
+
   ctx.subscriptions.push(dbm);
   void logger.log('Initializing database panel.');
   const databaseUI = new DatabaseUI(

extensions/ql-vscode/src/pure/date.ts

@@ -24,3 +24,16 @@ export function formatDate(value: Date): string {
 
   return dateFormatter.format(value);
 }
+
+// These are overloads for the function that allow us to not add an extra
+// type check when the value is definitely not undefined.
+export function parseDate(value: string): Date;
+export function parseDate(value: string | undefined | null): Date | undefined;
+
+export function parseDate(value: string | undefined | null): Date | undefined {
+  if (value === undefined || value === null) {
+    return undefined;
+  }
+
+  return new Date(value);
+}

extensions/ql-vscode/src/query-serialization.ts

@@ -7,6 +7,7 @@ import { CompletedQueryInfo, LocalQueryInfo } from './query-results';
 import { QueryHistoryInfo } from './query-history-info';
 import { QueryStatus } from './query-status';
 import { QueryEvaluationInfo } from './run-queries-shared';
+import { QueryResultType } from './pure/legacy-messages';
 
 export async function slurpQueryHistory(fsPath: string): Promise<QueryHistoryInfo[]> {
   try {
@@ -39,6 +40,17 @@ export async function slurpQueryHistory(fsPath: string): Promise<QueryHistoryInf
           Object.setPrototypeOf(q.completedQuery.query, QueryEvaluationInfo.prototype);
           // slurped queries do not need to be disposed
           q.completedQuery.dispose = () => { /**/ };
+
+          // Previously, there was a typo in the completedQuery type. There was a field
+          // `sucessful` and it was renamed to `successful`. We need to handle this case.
+          if ('sucessful' in q.completedQuery) {
+            (q.completedQuery as any).successful = (q.completedQuery as any).sucessful;
+            delete (q.completedQuery as any).sucessful;
+          }
+
+          if (!('successful' in q.completedQuery)) {
+            (q.completedQuery as any).successful = q.completedQuery.result?.resultType === QueryResultType.SUCCESS;
+          }
         }
       } else if (q.t === 'remote') {
         // A bug was introduced that didn't set the completed flag in query history
@@ -91,7 +103,10 @@ export async function splatQueryHistory(queries: QueryHistoryInfo[], fsPath: str
     // remove incomplete local queries since they cannot be recreated on restart
     const filteredQueries = queries.filter(q => q.t === 'local' ? q.completedQuery !== undefined : true);
     const data = JSON.stringify({
-      version: 2, // version 2 adds the `variant-analysis` type.
+      // version 2:
+      // - adds the `variant-analysis` type
+      // - ensures a `successful` property exists on completedQuery
+      version: 2,
       queries: filteredQueries
     }, null, 2);
     await fs.writeFile(fsPath, data);

extensions/ql-vscode/src/remote-queries/gh-api/variant-analysis.ts

@@ -24,7 +24,10 @@ export interface VariantAnalysis {
   actor_id: number,
   query_language: VariantAnalysisQueryLanguage,
   query_pack_url: string,
+  created_at: string,
+  updated_at: string,
   status: VariantAnalysisStatus,
+  completed_at?: string,
   actions_workflow_run_id?: number,
   failure_reason?: VariantAnalysisFailureReason,
   scanned_repositories?: VariantAnalysisScannedRepository[],

extensions/ql-vscode/src/remote-queries/shared/variant-analysis.ts

@@ -15,8 +15,11 @@ export interface VariantAnalysis {
     repositoryLists?: string[],
     repositoryOwners?: string[],
   },
+  createdAt: string,
+  updatedAt: string,
   executionStartTime: number;
   status: VariantAnalysisStatus,
+  completedAt?: string,
   actionsWorkflowRunId?: number,
   failureReason?: VariantAnalysisFailureReason,
   scannedRepos?: VariantAnalysisScannedRepository[],

extensions/ql-vscode/src/remote-queries/variant-analysis-processor.ts

@@ -56,7 +56,10 @@ export function processUpdatedVariantAnalysis(
     query: previousVariantAnalysis.query,
     databases: previousVariantAnalysis.databases,
     executionStartTime: previousVariantAnalysis.executionStartTime,
+    createdAt: response.created_at,
+    updatedAt: response.updated_at,
     status: processApiStatus(response.status),
+    completedAt: response.completed_at,
     actionsWorkflowRunId: response.actions_workflow_run_id,
     scannedRepos: scannedRepos,
     skippedRepos: skippedRepos

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisHeader.tsx

@@ -10,13 +10,11 @@ import {
 import { QueryDetails } from './QueryDetails';
 import { VariantAnalysisActions } from './VariantAnalysisActions';
 import { VariantAnalysisStats } from './VariantAnalysisStats';
+import { parseDate } from '../../pure/date';
 
 export type VariantAnalysisHeaderProps = {
   variantAnalysis: VariantAnalysis;
 
-  duration?: number | undefined;
-  completedAt?: Date | undefined;
-
   onOpenQueryFileClick: () => void;
   onViewQueryTextClick: () => void;
 
@@ -41,8 +39,6 @@ const Row = styled.div`
 
 export const VariantAnalysisHeader = ({
   variantAnalysis,
-  duration,
-  completedAt,
   onOpenQueryFileClick,
   onViewQueryTextClick,
   onStopQueryClick,
@@ -85,8 +81,8 @@ export const VariantAnalysisHeader = ({
         completedRepositoryCount={completedRepositoryCount}
         resultCount={resultCount}
         hasWarnings={hasSkippedRepos}
-        duration={duration}
-        completedAt={completedAt}
+        createdAt={parseDate(variantAnalysis.createdAt)}
+        completedAt={parseDate(variantAnalysis.completedAt)}
         onViewLogsClick={onViewLogsClick}
       />
     </Container>

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisStats.tsx

@@ -17,7 +17,7 @@ export type VariantAnalysisStatsProps = {
   hasWarnings?: boolean;
 
   resultCount?: number | undefined;
-  duration?: number | undefined;
+  createdAt: Date;
   completedAt?: Date | undefined;
 
   onViewLogsClick: () => void;
@@ -35,7 +35,7 @@ export const VariantAnalysisStats = ({
   completedRepositoryCount = 0,
   hasWarnings,
   resultCount,
-  duration,
+  createdAt,
   completedAt,
   onViewLogsClick,
 }: VariantAnalysisStatsProps) => {
@@ -59,6 +59,14 @@ export const VariantAnalysisStats = ({
     return 'Succeeded';
   }, [variantAnalysisStatus, hasWarnings]);
 
+  const duration = useMemo(() => {
+    if (!completedAt) {
+      return undefined;
+    }
+
+    return completedAt.getTime() - createdAt.getTime();
+  }, [completedAt, createdAt]);
+
   return (
     <Row>
       <StatItem title="Results">

extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisAnalyzedRepos.spec.tsx

@@ -21,6 +21,8 @@ describe(VariantAnalysisAnalyzedRepos.name, () => {
     },
     databases: {},
     executionStartTime: 1611234567890,
+    createdAt: '2021-01-21T13:09:27.890Z',
+    updatedAt: '2021-01-21T13:09:27.890Z',
     status: VariantAnalysisStatus.InProgress,
     scannedRepos: [
       {

extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisOutcomePanels.spec.tsx

@@ -20,6 +20,8 @@ describe(VariantAnalysisOutcomePanels.name, () => {
     },
     databases: {},
     executionStartTime: 1611234567890,
+    createdAt: '2021-01-21T13:09:27.890Z',
+    updatedAt: '2021-01-21T13:09:27.890Z',
     status: VariantAnalysisStatus.InProgress,
     scannedRepos: [
       {

extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStats.spec.tsx

@@ -17,6 +17,7 @@ describe(VariantAnalysisStats.name, () => {
         variantAnalysisStatus={VariantAnalysisStatus.InProgress}
         totalRepositoryCount={10}
         onViewLogsClick={onViewLogsClick}
+        createdAt={new Date()}
         {...props}
       />
     );
@@ -31,6 +32,7 @@ describe(VariantAnalysisStats.name, () => {
     render({ resultCount: 123456 });
 
     expect(screen.getByText('123,456')).toBeInTheDocument();
+    expect(screen.queryAllByText('-').length).toBe(1);
   });
 
   it('renders the number of repositories as a formatted number', () => {
@@ -100,4 +102,30 @@ describe(VariantAnalysisStats.name, () => {
     expect(screen.getByText('Succeeded')).toBeInTheDocument();
     expect(screen.queryByText('Succeeded warnings')).not.toBeInTheDocument();
   });
+
+  it('does not render the duration when the completedAt is not set', () => {
+    render({ createdAt: new Date('2021-05-01T00:00:00Z') });
+
+    expect(screen.queryAllByText('-').length).toBe(2);
+    expect(screen.queryByText('Less than a second')).not.toBeInTheDocument();
+  });
+
+  it('renders the duration when it is less than a second', () => {
+    render({ createdAt: new Date('2021-05-01T00:00:00Z'), completedAt: new Date('2021-05-01T00:00:00Z') });
+
+    expect(screen.getByText('Less than a second')).toBeInTheDocument();
+    expect(screen.queryAllByText('-').length).toBe(1);
+  });
+
+  it('renders the duration when it is less than a minute', () => {
+    render({ createdAt: new Date('2021-05-01T00:00:00Z'), completedAt: new Date('2021-05-01T00:00:34Z') });
+
+    expect(screen.getByText('34 seconds')).toBeInTheDocument();
+  });
+
+  it('renders the duration when it is more than a minute', () => {
+    render({ createdAt: new Date('2021-05-01T00:00:00Z'), completedAt: new Date('2021-05-01T00:10:22Z') });
+
+    expect(screen.getByText('10 minutes')).toBeInTheDocument();
+  });
 });

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/cli-integration/remote-queries/variant-analysis-processor.test.ts

@@ -37,7 +37,10 @@ describe('Variant Analysis processor', function() {
         'repositories': ['1', '2', '3'],
       },
       'executionStartTime': mockSubmission.startTime,
+      'createdAt': mockApiResponse.created_at,
+      'updatedAt': mockApiResponse.updated_at,
       'status': 'succeeded',
+      'completedAt': mockApiResponse.completed_at,
       'actionsWorkflowRunId': mockApiResponse.actions_workflow_run_id,
       'scannedRepos': [
         transformScannedRepo(VariantAnalysisRepoStatus.Succeeded, scannedRepos[0]),

extensions/ql-vscode/src/vscode-tests/factories/local-queries/local-query-history-item.ts

@@ -0,0 +1,33 @@
+import {
+  InitialQueryInfo,
+  CompletedQueryInfo,
+  CompletedLocalQueryInfo,
+  LocalQueryInfo,
+} from '../../../query-results';
+
+export function createMockLocalQueryInfo(
+  startTime: string,
+  userSpecifiedLabel?: string
+): LocalQueryInfo {
+  return ({
+    t: 'local',
+    userSpecifiedLabel,
+    startTime: startTime,
+    getQueryFileName() {
+      return 'query-file.ql';
+    },
+    getQueryName() {
+      return 'query-name';
+    },
+    initialInfo: ({
+      databaseInfo: {
+        databaseUri: 'unused',
+        name: 'db-name',
+      },
+    } as unknown) as InitialQueryInfo,
+    completedQuery: ({
+      resultCount: 456,
+      statusString: 'in progress',
+    } as unknown) as CompletedQueryInfo,
+  } as unknown) as CompletedLocalQueryInfo;
+}

extensions/ql-vscode/src/vscode-tests/factories/remote-queries/gh-api/variant-analysis-api-response.ts

@@ -28,6 +28,8 @@ export function createMockApiResponse(
     actor_id: faker.datatype.number(),
     query_language: VariantAnalysisQueryLanguage.Javascript,
     query_pack_url: 'https://example.com/foo',
+    created_at: faker.date.recent().toISOString(),
+    updated_at: faker.date.recent().toISOString(),
     status: status,
     actions_workflow_run_id: faker.datatype.number(),
     scanned_repositories: scannedRepos,

extensions/ql-vscode/src/vscode-tests/factories/remote-queries/remote-query-history-item.ts

@@ -0,0 +1,31 @@
+import { RemoteQueryHistoryItem } from '../../../remote-queries/remote-query-history-item';
+
+export function createMockRemoteQueryHistoryItem({
+  date = new Date('2022-01-01T00:00:00.000Z'),
+  resultCount = 16,
+  userSpecifiedLabel = undefined,
+  repositoryCount = 0,
+}: {
+  date?: Date;
+  resultCount?: number;
+  userSpecifiedLabel?: string;
+  repositoryCount?: number;
+}): RemoteQueryHistoryItem {
+  return ({
+    t: 'remote',
+    userSpecifiedLabel,
+    remoteQuery: {
+      executionStartTime: date.getTime(),
+      queryName: 'query-name',
+      queryFilePath: 'query-file.ql',
+      controllerRepository: {
+        owner: 'github',
+        name: 'vscode-codeql-integration-tests',
+      },
+      language: 'javascript',
+      repositoryCount,
+    },
+    status: 'in progress',
+    resultCount,
+  } as unknown) as RemoteQueryHistoryItem;
+}

extensions/ql-vscode/src/vscode-tests/factories/remote-queries/shared/variant-analysis.ts

@@ -27,6 +27,8 @@ export function createMockVariantAnalysis(
       repositories: ['1', '2', '3'],
     },
     executionStartTime: faker.datatype.number(),
+    createdAt: faker.date.recent().toISOString(),
+    updatedAt: faker.date.recent().toISOString(),
     status: status,
     actionsWorkflowRunId: faker.datatype.number(),
     scannedRepos: scannedRepos,

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/minimal-workspace/databases.test.ts

@@ -77,7 +77,9 @@ describe('databases', () => {
         },
         resolveDatabase: resolveDatabaseSpy
       } as unknown as CodeQLCliServer,
-      {} as Logger,
+      {
+        log: () => { /**/ }
+      } as unknown as Logger,
     );
 
     // Unfortunately, during a test it is not possible to convert from

extensions/ql-vscode/src/vscode-tests/minimal-workspace/index.ts

@@ -12,3 +12,12 @@ chai.use(sinonChai);
 export function run(): Promise<void> {
   return runTestsInDirectory(__dirname);
 }
+
+process.addListener('unhandledRejection', (reason) => {
+  if (reason instanceof Error && reason.message === 'Canceled') {
+    console.log('Cancellation requested after the test has ended.');
+    process.exit(0);
+  } else {
+    fail(String(reason));
+  }
+});

extensions/ql-vscode/src/vscode-tests/no-workspace/history-item-label-provider.test.ts

@@ -2,9 +2,8 @@ import { env } from 'vscode';
 import { expect } from 'chai';
 import { QueryHistoryConfig } from '../../config';
 import { HistoryItemLabelProvider } from '../../history-item-label-provider';
-import { CompletedLocalQueryInfo, CompletedQueryInfo, InitialQueryInfo } from '../../query-results';
-import { QueryHistoryInfo } from '../../query-history-info';
-import { RemoteQueryHistoryItem } from '../../remote-queries/remote-query-history-item';
+import { createMockLocalQueryInfo } from '../factories/local-queries/local-query-history-item';
+import { createMockRemoteQueryHistoryItem } from '../factories/remote-queries/remote-query-history-item';
 
 
 describe('HistoryItemLabelProvider', () => {
@@ -23,7 +22,7 @@ describe('HistoryItemLabelProvider', () => {
 
   describe('local queries', () => {
     it('should interpolate query when user specified', () => {
-      const fqi = createMockLocalQueryInfo('xxx');
+      const fqi = createMockLocalQueryInfo(dateStr, 'xxx');
 
       expect(labelProvider.getLabel(fqi)).to.eq('xxx');
 
@@ -35,7 +34,7 @@ describe('HistoryItemLabelProvider', () => {
     });
 
     it('should interpolate query when not user specified', () => {
-      const fqi = createMockLocalQueryInfo();
+      const fqi = createMockLocalQueryInfo(dateStr);
 
       expect(labelProvider.getLabel(fqi)).to.eq('xxx query-name xxx');
 
@@ -48,7 +47,7 @@ describe('HistoryItemLabelProvider', () => {
     });
 
     it('should get query short label', () => {
-      const fqi = createMockLocalQueryInfo('xxx');
+      const fqi = createMockLocalQueryInfo(dateStr, 'xxx');
 
       // fall back on user specified if one exists.
       expect(labelProvider.getShortLabel(fqi)).to.eq('xxx');
@@ -57,35 +56,11 @@ describe('HistoryItemLabelProvider', () => {
       delete (fqi as any).userSpecifiedLabel;
       expect(labelProvider.getShortLabel(fqi)).to.eq('query-name');
     });
-
-    function createMockLocalQueryInfo(userSpecifiedLabel?: string) {
-      return {
-        t: 'local',
-        userSpecifiedLabel,
-        startTime: date.toLocaleString(env.language),
-        getQueryFileName() {
-          return 'query-file.ql';
-        },
-        getQueryName() {
-          return 'query-name';
-        },
-        initialInfo: {
-          databaseInfo: {
-            databaseUri: 'unused',
-            name: 'db-name'
-          }
-        } as unknown as InitialQueryInfo,
-        completedQuery: {
-          resultCount: 456,
-          statusString: 'in progress',
-        } as unknown as CompletedQueryInfo,
-      } as unknown as CompletedLocalQueryInfo;
-    }
   });
 
   describe('remote queries', () => {
     it('should interpolate query when user specified', () => {
-      const fqi = createMockRemoteQueryInfo({ userSpecifiedLabel: 'xxx' });
+      const fqi = createMockRemoteQueryHistoryItem({ userSpecifiedLabel: 'xxx' });
 
       expect(labelProvider.getLabel(fqi)).to.eq('xxx');
 
@@ -97,7 +72,7 @@ describe('HistoryItemLabelProvider', () => {
     });
 
     it('should interpolate query when not user-specified', () => {
-      const fqi = createMockRemoteQueryInfo({});
+      const fqi = createMockRemoteQueryHistoryItem({});
 
       expect(labelProvider.getLabel(fqi)).to.eq('xxx query-name (javascript) xxx');
 
@@ -110,14 +85,14 @@ describe('HistoryItemLabelProvider', () => {
     });
 
     it('should use number of repositories instead of controller repo if available', () => {
-      const fqi = createMockRemoteQueryInfo({ repositoryCount: 2 });
+      const fqi = createMockRemoteQueryHistoryItem({ repositoryCount: 2 });
 
       config.format = '%t %q %d %s %f %r %%';
       expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name (javascript) 2 repositories in progress query-file.ql (16 results) %`);
     });
 
     it('should get query short label', () => {
-      const fqi = createMockRemoteQueryInfo({ userSpecifiedLabel: 'xxx' });
+      const fqi = createMockRemoteQueryHistoryItem({ userSpecifiedLabel: 'xxx' });
 
       // fall back on user specified if one exists.
       expect(labelProvider.getShortLabel(fqi)).to.eq('xxx');
@@ -129,7 +104,7 @@ describe('HistoryItemLabelProvider', () => {
 
     describe('when results are present', () => {
       it('should display results if there are any', () => {
-        const fqi = createMockRemoteQueryInfo({ resultCount: 16, repositoryCount: 2 });
+        const fqi = createMockRemoteQueryHistoryItem({ resultCount: 16, repositoryCount: 2 });
         config.format = '%t %q %d %s %f %r %%';
         expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name (javascript) 2 repositories in progress query-file.ql (16 results) %`);
       });
@@ -137,7 +112,7 @@ describe('HistoryItemLabelProvider', () => {
 
     describe('when results are not present', () => {
       it('should skip displaying them', () => {
-        const fqi = createMockRemoteQueryInfo({ resultCount: 0, repositoryCount: 2 });
+        const fqi = createMockRemoteQueryHistoryItem({ resultCount: 0, repositoryCount: 2 });
         config.format = '%t %q %d %s %f %r %%';
         expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name (javascript) 2 repositories in progress query-file.ql %`);
       });
@@ -145,7 +120,7 @@ describe('HistoryItemLabelProvider', () => {
 
     describe('when extra whitespace is present in the middle of the label', () => {
       it('should squash it down to a single whitespace', () => {
-        const fqi = createMockRemoteQueryInfo({ resultCount: 0, repositoryCount: 2 });
+        const fqi = createMockRemoteQueryHistoryItem({ resultCount: 0, repositoryCount: 2 });
         config.format = '%t   %q        %d %s   %f   %r %%';
         expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name (javascript) 2 repositories in progress query-file.ql %`);
       });
@@ -153,7 +128,7 @@ describe('HistoryItemLabelProvider', () => {
 
     describe('when extra whitespace is present at the start of the label', () => {
       it('should squash it down to a single whitespace', () => {
-        const fqi = createMockRemoteQueryInfo({ resultCount: 0, repositoryCount: 2 });
+        const fqi = createMockRemoteQueryHistoryItem({ resultCount: 0, repositoryCount: 2 });
         config.format = '   %t %q %d %s %f %r %%';
         expect(labelProvider.getLabel(fqi)).to.eq(` ${dateStr} query-name (javascript) 2 repositories in progress query-file.ql %`);
       });
@@ -161,38 +136,10 @@ describe('HistoryItemLabelProvider', () => {
 
     describe('when extra whitespace is present at the end of the label', () => {
       it('should squash it down to a single whitespace', () => {
-        const fqi = createMockRemoteQueryInfo({ resultCount: 0, repositoryCount: 2 });
+        const fqi = createMockRemoteQueryHistoryItem({ resultCount: 0, repositoryCount: 2 });
         config.format = '%t %q %d %s %f %r %%   ';
         expect(labelProvider.getLabel(fqi)).to.eq(`${dateStr} query-name (javascript) 2 repositories in progress query-file.ql % `);
       });
     });
-
-    function createMockRemoteQueryInfo({
-      resultCount = 16,
-      userSpecifiedLabel = undefined,
-      repositoryCount = 0
-    }: {
-      resultCount?: number;
-      userSpecifiedLabel?: string;
-      repositoryCount?: number;
-    }): QueryHistoryInfo {
-      return {
-        t: 'remote',
-        userSpecifiedLabel,
-        remoteQuery: {
-          executionStartTime: date.getTime(),
-          queryName: 'query-name',
-          queryFilePath: 'query-file.ql',
-          controllerRepository: {
-            owner: 'github',
-            name: 'vscode-codeql-integration-tests'
-          },
-          language: 'javascript',
-          repositoryCount,
-        },
-        status: 'in progress',
-        resultCount,
-      } as unknown as RemoteQueryHistoryItem;
-    }
   });
 });

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()));
+    }
+  });
+};