Reregister testproj databases around test runs

extensions/ql-vscode/CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - Display CodeQL CLI version being downloaded during an upgrade. [#862](https://github.com/github/vscode-codeql/pull/862)
 - Display a helpful message and link to documentation when a query produces no results. [#866](https://github.com/github/vscode-codeql/pull/866)
+- Refresh test databases automatically after a test run. [#868](https://github.com/github/vscode-codeql/pull/868)
 
 ## 1.4.8 - 05 May 2021
 

extensions/ql-vscode/src/databases.ts

@@ -265,6 +265,11 @@ export interface DatabaseItem {
    */
   belongsToSourceArchiveExplorerUri(uri: vscode.Uri): boolean;
 
+  /**
+   * Whether the database may be affected by test execution for the given path.
+   */
+  isAffectedByTest(testPath: string): Promise<boolean>;
+
   /**
    * Gets the state of this database, to be persisted in the workspace state.
    */
@@ -470,6 +475,27 @@ export class DatabaseItemImpl implements DatabaseItem {
     return uri.scheme === zipArchiveScheme &&
       decodeSourceArchiveUri(uri).sourceArchiveZipPath === this.sourceArchive.fsPath;
   }
+
+  public async isAffectedByTest(testPath: string): Promise<boolean> {
+    const databasePath = this.databaseUri.fsPath;
+    if (!databasePath.endsWith('.testproj')) {
+      return false;
+    }
+    try {
+      const stats = await fs.stat(testPath);
+      if (stats.isDirectory()) {
+        return !path.relative(testPath, databasePath).startsWith('..');
+      } else {
+        // database for /one/two/three/test.ql is at /one/two/three/three.testproj
+        const testdir = path.dirname(testPath);
+        const testdirbase = path.basename(testdir);
+        return databasePath == path.join(testdir, testdirbase + '.testproj');
+      }
+    } catch {
+      // No information available for test path - assume database is unaffected.
+      return false;
+    }
+  }
 }
 
 /**

extensions/ql-vscode/src/extension.ts

@@ -526,7 +526,7 @@ async function activateWithInstalledDistribution(
   );
   if (testExplorerExtension) {
     const testHub = testExplorerExtension.exports;
-    const testAdapterFactory = new QLTestAdapterFactory(testHub, cliServer);
+    const testAdapterFactory = new QLTestAdapterFactory(testHub, cliServer, dbm);
     ctx.subscriptions.push(testAdapterFactory);
 
     const testUIService = new TestUIService(testHub);

extensions/ql-vscode/src/test-adapter.ts

@@ -1,3 +1,4 @@
+import * as fs from 'fs-extra';
 import * as path from 'path';
 import * as vscode from 'vscode';
 import {
@@ -17,8 +18,9 @@ import { QLTestFile, QLTestNode, QLTestDirectory, QLTestDiscovery } from './qlte
 import { Event, EventEmitter, CancellationTokenSource, CancellationToken } from 'vscode';
 import { DisposableObject } from './pure/disposable-object';
 import { CodeQLCliServer } from './cli';
-import { getOnDiskWorkspaceFolders } from './helpers';
+import { getOnDiskWorkspaceFolders, showAndLogErrorMessage, showAndLogWarningMessage } from './helpers';
 import { testLogger } from './logging';
+import { DatabaseItem, DatabaseManager } from './databases';
 
 /**
  * Get the full path of the `.expected` file for the specified QL test.
@@ -57,13 +59,13 @@ function getTestOutputFile(testPath: string, extension: string): string {
  * A factory service that creates `QLTestAdapter` objects for workspace folders on demand.
  */
 export class QLTestAdapterFactory extends DisposableObject {
-  constructor(testHub: TestHub, cliServer: CodeQLCliServer) {
+  constructor(testHub: TestHub, cliServer: CodeQLCliServer, databaseManager: DatabaseManager) {
     super();
 
     // this will register a QLTestAdapter for each WorkspaceFolder
     this.push(new TestAdapterRegistrar(
       testHub,
-      workspaceFolder => new QLTestAdapter(workspaceFolder, cliServer)
+      workspaceFolder => new QLTestAdapter(workspaceFolder, cliServer, databaseManager)
     ));
   }
 }
@@ -91,7 +93,8 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter {
 
   constructor(
     public readonly workspaceFolder: vscode.WorkspaceFolder,
-    private readonly cliServer: CodeQLCliServer
+    private readonly cliServer: CodeQLCliServer,
+    private readonly databaseManager: DatabaseManager
   ) {
     super();
 
@@ -182,19 +185,79 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter {
     testLogger.outputChannel.show(true);
 
     this.runningTask = this.track(new CancellationTokenSource());
+    const token = this.runningTask.token;
 
     this._testStates.fire({ type: 'started', tests: tests } as TestRunStartedEvent);
 
+    const currentDatabaseUri = this.databaseManager.currentDatabaseItem?.databaseUri;
+    const databasesUnderTest = this.databaseManager.databaseItems
+      .filter(database => tests.find(testPath => database.isAffectedByTest(testPath)));
+
+    await this.removeDatabasesBeforeTests(databasesUnderTest, token);
     try {
-      await this.runTests(tests, this.runningTask.token);
-    }
-    catch (e) {
-      /**/
+      await this.runTests(tests, token);
+    } catch (e) {
+      // CodeQL testing can throw exception even in normal scenarios. For example, if the test run
+      // produces no output (which is normal), the testing command would throw an exception on
+      // unexpected EOF during json parsing. So nothing needs to be done here - all the relevant
+      // error information (if any) should have already been written to the test logger.
     }
+    await this.reopenDatabasesAfterTests(databasesUnderTest, currentDatabaseUri, token);
+
     this._testStates.fire({ type: 'finished' } as TestRunFinishedEvent);
     this.clearTask();
   }
 
+  private async removeDatabasesBeforeTests(
+    databasesUnderTest: DatabaseItem[], token: vscode.CancellationToken): Promise<void> {
+    for (const database of databasesUnderTest) {
+      try {
+        await this.databaseManager
+          .removeDatabaseItem(_ => { /* no progress reporting */ }, token, database);
+      } catch (e) {
+        // This method is invoked from Test Explorer UI, and testing indicates that Test
+        // Explorer UI swallows any thrown exception without reporting it to the user.
+        // So we need to display the error message ourselves and then rethrow.
+        showAndLogErrorMessage(`Cannot remove database ${database.name}: ${e}`);
+        throw e;
+      }
+    }
+  }
+
+  private async reopenDatabasesAfterTests(
+    databasesUnderTest: DatabaseItem[],
+    currentDatabaseUri: vscode.Uri | undefined,
+    token: vscode.CancellationToken): Promise<void> {
+    for (const closedDatabase of databasesUnderTest) {
+      const uri = closedDatabase.databaseUri;
+      if (await this.isFileAccessible(uri)) {
+        try {
+          const reopenedDatabase = await this.databaseManager
+            .openDatabase(_ => { /* no progress reporting */ }, token, uri);
+          await this.databaseManager.renameDatabaseItem(reopenedDatabase, closedDatabase.name);
+          if (currentDatabaseUri == uri) {
+            await this.databaseManager.setCurrentDatabaseItem(reopenedDatabase, true);
+          }
+        } catch (e) {
+          // This method is invoked from Test Explorer UI, and testing indicates that Test
+          // Explorer UI swallows any thrown exception without reporting it to the user.
+          // So we need to display the error message ourselves and then rethrow.
+          showAndLogWarningMessage(`Cannot reopen database ${uri}: ${e}`);
+          throw e;
+        }
+      }
+    }
+  }
+
+  private async isFileAccessible(uri: vscode.Uri): Promise<boolean> {
+    try {
+      await fs.access(uri.fsPath);
+      return true;
+    } catch {
+      return false;
+    }
+  }
+
   private clearTask(): void {
     if (this.runningTask !== undefined) {
       const runningTask = this.runningTask;

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

@@ -179,7 +179,7 @@ describe('databases', () => {
       expect(spy).to.have.been.calledWith(mockEvent);
     });
 
-    it('should add a database item source archive', async function() {
+    it('should add a database item source archive', async function () {
       const mockDbItem = createMockDB();
       mockDbItem.name = 'xxx';
       await (databaseManager as any).addDatabaseSourceArchiveFolder(mockDbItem);
@@ -414,6 +414,72 @@ describe('databases', () => {
     expect(result).to.eq('');
   });
 
+  describe('isAffectedByTest', () => {
+    const directoryStats = new fs.Stats();
+    const fileStats = new fs.Stats();
+
+    before(() => {
+      sinon.stub(directoryStats, 'isDirectory').returns(true);
+      sinon.stub(fileStats, 'isDirectory').returns(false);
+    });
+
+    it('should return true for testproj database in test directory', async () => {
+      sandbox.stub(fs, 'stat').resolves(directoryStats);
+      const db = createMockDB(sourceLocationUri(), Uri.file('/path/to/dir/dir.testproj'));
+      expect(await db.isAffectedByTest('/path/to/dir')).to.true;
+    });
+
+    it('should return false for non-existent test directory', async () => {
+      sandbox.stub(fs, 'stat').throws('Simulated Error: ENOENT');
+      const db = createMockDB(sourceLocationUri(), Uri.file('/path/to/dir/dir.testproj'));
+      expect(await db.isAffectedByTest('/path/to/dir')).to.false;
+    });
+
+    it('should return false for non-testproj database in test directory', async () => {
+      sandbox.stub(fs, 'stat').resolves(directoryStats);
+      const db = createMockDB(sourceLocationUri(), Uri.file('/path/to/dir/dir.proj'));
+      expect(await db.isAffectedByTest('/path/to/dir')).to.false;
+    });
+
+    it('should return false for testproj database outside test directory', async () => {
+      sandbox.stub(fs, 'stat').resolves(directoryStats);
+      const db = createMockDB(sourceLocationUri(), Uri.file('/path/to/other/dir.testproj'));
+      expect(await db.isAffectedByTest('/path/to/dir')).to.false;
+    });
+
+    it('should return false for testproj database for prefix directory', async () => {
+      sandbox.stub(fs, 'stat').resolves(directoryStats);
+      const db = createMockDB(sourceLocationUri(), Uri.file('/path/to/dir/dir.testproj'));
+      // /path/to/d is a prefix of /path/to/dir/dir.testproj, but
+      // /path/to/dir/dir.testproj is not under /path/to/d
+      expect(await db.isAffectedByTest('/path/to/d')).to.false;
+    });
+
+    it('should return true for testproj database for test file', async () => {
+      sandbox.stub(fs, 'stat').resolves(fileStats);
+      const db = createMockDB(sourceLocationUri(), Uri.file('/path/to/dir/dir.testproj'));
+      expect(await db.isAffectedByTest('/path/to/dir/test.ql')).to.true;
+    });
+
+    it('should return false for non-existent test file', async () => {
+      sandbox.stub(fs, 'stat').throws('Simulated Error: ENOENT');
+      const db = createMockDB(sourceLocationUri(), Uri.file('/path/to/dir/dir.testproj'));
+      expect(await db.isAffectedByTest('/path/to/dir/test.ql')).to.false;
+    });
+
+    it('should return false for non-testproj database for test file', async () => {
+      sandbox.stub(fs, 'stat').resolves(fileStats);
+      const db = createMockDB(sourceLocationUri(), Uri.file('/path/to/dir/dir.proj'));
+      expect(await db.isAffectedByTest('/path/to/dir/test.ql')).to.false;
+    });
+
+    it('should return false for testproj database not matching test file', async () => {
+      sandbox.stub(fs, 'stat').resolves(fileStats);
+      const db = createMockDB(sourceLocationUri(), Uri.file('/path/to/dir/dir.testproj'));
+      expect(await db.isAffectedByTest('/path/to/test.ql')).to.false;
+    });
+  });
+
   function createMockDB(
     // source archive location must be a real(-ish) location since
     // tests will add this to the workspace location

extensions/ql-vscode/src/vscode-tests/no-workspace/test-adapter.test.ts

@@ -1,32 +1,70 @@
 import 'vscode-test';
 import 'mocha';
 import * as sinon from 'sinon';
+import * as fs from 'fs-extra';
 import { Uri, WorkspaceFolder } from 'vscode';
 import { expect } from 'chai';
 
 import { QLTestAdapter } from '../../test-adapter';
 import { CodeQLCliServer } from '../../cli';
+import { DatabaseItem, DatabaseItemImpl, DatabaseManager, FullDatabaseOptions } from '../../databases';
 
 describe('test-adapter', () => {
   let adapter: QLTestAdapter;
+  let fakeDatabaseManager: DatabaseManager;
+  let currentDatabaseItem: DatabaseItem | undefined;
+  let databaseItems: DatabaseItem[] = [];
+  let openDatabaseSpy: sinon.SinonStub;
+  let removeDatabaseItemSpy: sinon.SinonStub;
+  let renameDatabaseItemSpy: sinon.SinonStub;
+  let setCurrentDatabaseItemSpy: sinon.SinonStub;
   let runTestsSpy: sinon.SinonStub;
   let resolveTestsSpy: sinon.SinonStub;
   let resolveQlpacksSpy: sinon.SinonStub;
   let sandox: sinon.SinonSandbox;
 
+  const preTestDatabaseItem = new DatabaseItemImpl(
+    Uri.file('/path/to/test/dir/dir.testproj'),
+    undefined,
+    { displayName: 'custom display name' } as unknown as FullDatabaseOptions,
+    (_) => { /* no change event listener */ }
+  );
+  const postTestDatabaseItem = new DatabaseItemImpl(
+    Uri.file('/path/to/test/dir/dir.testproj'),
+    undefined,
+    { displayName: 'default name' } as unknown as FullDatabaseOptions,
+    (_) => { /* no change event listener */ }
+  );
+
   beforeEach(() => {
     sandox = sinon.createSandbox();
     mockRunTests();
+    openDatabaseSpy = sandox.stub().resolves(postTestDatabaseItem);
+    removeDatabaseItemSpy = sandox.stub().resolves();
+    renameDatabaseItemSpy = sandox.stub().resolves();
+    setCurrentDatabaseItemSpy = sandox.stub().resolves();
     resolveQlpacksSpy = sandox.stub().resolves({});
     resolveTestsSpy = sandox.stub().resolves([]);
+    fakeDatabaseManager = {
+      currentDatabaseItem: undefined,
+      databaseItems: undefined,
+      openDatabase: openDatabaseSpy,
+      removeDatabaseItem: removeDatabaseItemSpy,
+      renameDatabaseItem: renameDatabaseItemSpy,
+      setCurrentDatabaseItem: setCurrentDatabaseItemSpy,
+    } as unknown as DatabaseManager;
+    sandox.stub(fakeDatabaseManager, 'currentDatabaseItem').get(() => currentDatabaseItem);
+    sandox.stub(fakeDatabaseManager, 'databaseItems').get(() => databaseItems);
+    sandox.stub(preTestDatabaseItem, 'isAffectedByTest').resolves(true);
     adapter = new QLTestAdapter({
       name: 'ABC',
       uri: Uri.parse('file:/ab/c')
     } as WorkspaceFolder, {
       runTests: runTestsSpy,
       resolveQlpacks: resolveQlpacksSpy,
       resolveTests: resolveTestsSpy
-    } as unknown as CodeQLCliServer);
+    } as unknown as CodeQLCliServer,
+      fakeDatabaseManager);
   });
 
   afterEach(() => {
@@ -74,12 +112,33 @@ describe('test-adapter', () => {
     expect(listenerSpy).to.have.callCount(5);
   });
 
+  it('should reregister testproj databases around test run', async () => {
+    sandox.stub(fs, 'access').resolves();
+    currentDatabaseItem = preTestDatabaseItem;
+    databaseItems = [preTestDatabaseItem];
+    await adapter.run(['/path/to/test/dir']);
+
+    removeDatabaseItemSpy.getCall(0).calledBefore(runTestsSpy.getCall(0));
+    openDatabaseSpy.getCall(0).calledAfter(runTestsSpy.getCall(0));
+    renameDatabaseItemSpy.getCall(0).calledAfter(openDatabaseSpy.getCall(0));
+    setCurrentDatabaseItemSpy.getCall(0).calledAfter(openDatabaseSpy.getCall(0));
+
+    sinon.assert.calledOnceWithExactly(
+      removeDatabaseItemSpy, sinon.match.any, sinon.match.any, preTestDatabaseItem);
+    sinon.assert.calledOnceWithExactly(
+      openDatabaseSpy, sinon.match.any, sinon.match.any, preTestDatabaseItem.databaseUri);
+    sinon.assert.calledOnceWithExactly(
+      renameDatabaseItemSpy, postTestDatabaseItem, preTestDatabaseItem.name);
+    sinon.assert.calledOnceWithExactly(
+      setCurrentDatabaseItemSpy, postTestDatabaseItem, true);
+  });
+
   function mockRunTests() {
     // runTests is an async generator function. This is not directly supported in sinon
     // However, we can pretend the same thing by just returning an async array.
     runTestsSpy = sandox.stub();
     runTestsSpy.returns(
-      (async function*() {
+      (async function* () {
         yield Promise.resolve({
           test: Uri.parse('file:/ab/c/d.ql').fsPath,
           pass: true,