Fix dubious index check (#692)

* Fix dubious index check

* Add unit tests for add/remove database

In order to do this, needed to move `databases.test.ts` to the
`minimal-workspace` test folder because these tests require that there
be some kind of workspace open in order to check on workspace folders.

Unfortunately, during tests vscode does not allow you to convert from a
single root workspace to multi-root and so several of the workspace
functions needed to be stubbed out.

* Update changelog

Co-authored-by: Andrew Eisenberg <aeisenberg@github.com>

extensions/ql-vscode/CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## [UNRELEASED]
 
+- Fix bug when removing databases where sometimes the source folder would not be removed from the workspace or the database files would not be removed from the workspace storage location. [#692](https://github.com/github/vscode-codeql/pull/692)
+
 ## 1.3.7 - 24 November 2020
 
 - Editors opened by navigating from the results view are no longer opened in _preview mode_. Now they are opened as a persistent editor. [#630](https://github.com/github/vscode-codeql/pull/630)

extensions/ql-vscode/src/databases.ts

@@ -38,7 +38,7 @@ export interface DatabaseOptions {
   dateAdded?: number | undefined;
 }
 
-interface FullDatabaseOptions extends DatabaseOptions {
+export interface FullDatabaseOptions extends DatabaseOptions {
   ignoreSourceArchive: boolean;
   dateAdded: number | undefined;
 }
@@ -674,8 +674,9 @@ export class DatabaseManager extends DisposableObject {
   }
 
   public removeDatabaseItem(item: DatabaseItem) {
-    if (this._currentDatabaseItem == item)
+    if (this._currentDatabaseItem == item) {
       this._currentDatabaseItem = undefined;
+    }
     const index = this.databaseItems.findIndex(searchItem => searchItem === item);
     if (index >= 0) {
       this._databaseItems.splice(index, 1);
@@ -683,8 +684,10 @@ export class DatabaseManager extends DisposableObject {
     this.updatePersistedDatabaseList();
 
     // Delete folder from workspace, if it is still there
-    const folderIndex = (vscode.workspace.workspaceFolders || []).findIndex(folder => item.belongsToSourceArchiveExplorerUri(folder.uri));
-    if (index >= 0) {
+    const folderIndex = (vscode.workspace.workspaceFolders || []).findIndex(
+      folder => item.belongsToSourceArchiveExplorerUri(folder.uri)
+    );
+    if (folderIndex >= 0) {
       logger.log(`Removing workspace folder at index ${folderIndex}`);
       vscode.workspace.updateWorkspaceFolders(folderIndex, 1);
     }
@@ -692,9 +695,9 @@ export class DatabaseManager extends DisposableObject {
     // Delete folder from file system only if it is controlled by the extension
     if (this.isExtensionControlledLocation(item.databaseUri)) {
       logger.log('Deleting database from filesystem.');
-      fs.remove(item.databaseUri.path).then(
-        () => logger.log(`Deleted '${item.databaseUri.path}'`),
-        e => logger.log(`Failed to delete '${item.databaseUri.path}'. Reason: ${e.message}`));
+      fs.remove(item.databaseUri.fsPath).then(
+        () => logger.log(`Deleted '${item.databaseUri.fsPath}'`),
+        e => logger.log(`Failed to delete '${item.databaseUri.fsPath}'. Reason: ${e.message}`));
     }
 
     // note that we use undefined as the item in order to reset the entire tree
@@ -715,7 +718,13 @@ export class DatabaseManager extends DisposableObject {
 
   private isExtensionControlledLocation(uri: vscode.Uri) {
     const storagePath = this.ctx.storagePath || this.ctx.globalStoragePath;
-    return uri.path.startsWith(storagePath);
+    // the uri.fsPath function on windows returns a lowercase drive letter,
+    // but storagePath will have an uppercase drive letter. Be sure to compare
+    // URIs to URIs only
+    if (storagePath) {
+      return uri.fsPath.startsWith(vscode.Uri.file(storagePath).fsPath);
+    }
+    return false;
   }
 }
 

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

@@ -0,0 +1,309 @@
+import 'vscode-test';
+import 'mocha';
+import * as sinon from 'sinon';
+import * as tmp from 'tmp';
+import * as fs from 'fs-extra';
+import * as path from 'path';
+import { expect } from 'chai';
+import { ExtensionContext, Uri, workspace } from 'vscode';
+
+import {
+  DatabaseEventKind,
+  DatabaseManager,
+  DatabaseItemImpl,
+  DatabaseContents,
+  isLikelyDbLanguageFolder,
+  FullDatabaseOptions
+} from '../../databases';
+import { QueryServerConfig } from '../../config';
+import { Logger } from '../../logging';
+import { encodeArchiveBasePath, encodeSourceArchiveUri } from '../../archive-filesystem-provider';
+
+describe('databases', () => {
+
+  const MOCK_DB_OPTIONS: FullDatabaseOptions = {
+    dateAdded: 123,
+    ignoreSourceArchive: false
+  };
+
+  let databaseManager: DatabaseManager;
+  let updateSpy: sinon.SinonSpy;
+  let getSpy: sinon.SinonStub;
+  let dbChangedHandler: sinon.SinonSpy;
+
+  let sandbox: sinon.SinonSandbox;
+  let dir: tmp.DirResult;
+
+
+
+  beforeEach(() => {
+    dir = tmp.dirSync();
+
+    sandbox = sinon.createSandbox();
+    updateSpy = sandbox.spy();
+    getSpy = sandbox.stub();
+    dbChangedHandler = sandbox.spy();
+    databaseManager = new DatabaseManager(
+      {
+        workspaceState: {
+          update: updateSpy,
+          get: getSpy
+        },
+        // pretend like databases added in the temp dir are controlled by the extension
+        // so that they are deleted upon removal
+        storagePath: dir.name
+      } as unknown as ExtensionContext,
+      {} as QueryServerConfig,
+      {} as Logger,
+    );
+
+    // Unfortunately, during a test it is not possible to convert from
+    // a single root workspace to multi-root, so must stub out relevant
+    // functions
+    sandbox.stub(workspace, 'updateWorkspaceFolders');
+    sandbox.spy(workspace, 'onDidChangeWorkspaceFolders');
+  });
+
+  afterEach(async () => {
+    dir.removeCallback();
+    sandbox.restore();
+  });
+
+  it('should fire events when adding and removing a db item', () => {
+    const mockDbItem = createMockDB();
+    const spy = sinon.spy();
+    databaseManager.onDidChangeDatabaseItem(spy);
+    (databaseManager as any).addDatabaseItem(mockDbItem);
+
+    expect((databaseManager as any)._databaseItems).to.deep.eq([mockDbItem]);
+    expect(updateSpy).to.have.been.calledWith('databaseList', [{
+      options: MOCK_DB_OPTIONS,
+      uri: dbLocationUri().toString(true)
+    }]);
+    expect(spy).to.have.been.calledWith({
+      item: undefined,
+      kind: DatabaseEventKind.Add
+    });
+
+    sinon.reset();
+
+    // now remove the item
+    databaseManager.removeDatabaseItem(mockDbItem);
+    expect((databaseManager as any)._databaseItems).to.deep.eq([]);
+    expect(updateSpy).to.have.been.calledWith('databaseList', []);
+    expect(spy).to.have.been.calledWith({
+      item: undefined,
+      kind: DatabaseEventKind.Remove
+    });
+  });
+
+  it('should rename a db item and emit an event', () => {
+    const mockDbItem = createMockDB();
+    const spy = sinon.spy();
+    databaseManager.onDidChangeDatabaseItem(spy);
+    (databaseManager as any).addDatabaseItem(mockDbItem);
+    sinon.restore();
+
+    databaseManager.renameDatabaseItem(mockDbItem, 'new name');
+
+    expect(mockDbItem.name).to.eq('new name');
+    expect(updateSpy).to.have.been.calledWith('databaseList', [{
+      options: { ...MOCK_DB_OPTIONS, displayName: 'new name' },
+      uri: dbLocationUri().toString(true)
+    }]);
+
+    expect(spy).to.have.been.calledWith({
+      item: undefined,
+      kind: DatabaseEventKind.Rename
+    });
+  });
+
+  describe('add / remove database items', () => {
+    it('should add a database item', async () => {
+      const spy = sandbox.spy();
+      databaseManager.onDidChangeDatabaseItem(spy);
+      const mockDbItem = createMockDB();
+
+      await (databaseManager as any).addDatabaseItem(mockDbItem);
+
+      expect(databaseManager.databaseItems).to.deep.eq([mockDbItem]);
+      expect(updateSpy).to.have.been.calledWith('databaseList', [{
+        uri: dbLocationUri().toString(true),
+        options: MOCK_DB_OPTIONS
+      }]);
+
+      const mockEvent = {
+        item: undefined,
+        kind: DatabaseEventKind.Add
+      };
+      expect(spy).to.have.been.calledWith(mockEvent);
+    });
+
+    it('should add a database item source archive', async function() {
+      const mockDbItem = createMockDB();
+      mockDbItem.name = 'xxx';
+      await (databaseManager as any).addDatabaseSourceArchiveFolder(mockDbItem);
+
+      // workspace folders should be updated. We can only check the mocks since
+      // when running as a test, we are not allowed to update the workspace folders
+      expect(workspace.updateWorkspaceFolders).to.have.been.calledWith(1, 0, {
+        name: '[xxx source archive]',
+        // must use a matcher here since vscode URIs with the same path
+        // are not always equal due to internal state.
+        uri: sinon.match.has('fsPath', encodeArchiveBasePath(sourceLocationUri().fsPath).fsPath)
+      });
+    });
+
+    it('should remove a database item', async () => {
+      const mockDbItem = createMockDB();
+      sandbox.stub(fs, 'remove').resolves();
+
+      // pretend that this item is the first workspace folder in the list
+      sandbox.stub(mockDbItem, 'belongsToSourceArchiveExplorerUri').returns(true);
+
+      await (databaseManager as any).addDatabaseItem(mockDbItem);
+      updateSpy.resetHistory();
+
+      await databaseManager.removeDatabaseItem(mockDbItem);
+
+      expect(databaseManager.databaseItems).to.deep.eq([]);
+      expect(updateSpy).to.have.been.calledWith('databaseList', []);
+      // should remove the folder
+      expect(workspace.updateWorkspaceFolders).to.have.been.calledWith(0, 1);
+
+      // should also delete the db contents
+      expect(fs.remove).to.have.been.calledWith(mockDbItem.databaseUri.fsPath);
+    });
+
+    it('should remove a database item outside of the extension controlled area', async () => {
+      const mockDbItem = createMockDB();
+      sandbox.stub(fs, 'remove').resolves();
+
+      // pretend that this item is the first workspace folder in the list
+      sandbox.stub(mockDbItem, 'belongsToSourceArchiveExplorerUri').returns(true);
+
+      await (databaseManager as any).addDatabaseItem(mockDbItem);
+      updateSpy.resetHistory();
+
+      // pretend that the database location is not controlled by the extension
+      (databaseManager as any).ctx.storagePath = 'hucairz';
+
+      await databaseManager.removeDatabaseItem(mockDbItem);
+
+      expect(databaseManager.databaseItems).to.deep.eq([]);
+      expect(updateSpy).to.have.been.calledWith('databaseList', []);
+      // should remove the folder
+      expect(workspace.updateWorkspaceFolders).to.have.been.calledWith(0, 1);
+
+      // should NOT delete the db contents
+      expect(fs.remove).not.to.have.been.called;
+    });
+  });
+
+  describe('resolveSourceFile', () => {
+    it('should fail to resolve when not a uri', () => {
+      const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
+      (db as any)._contents.sourceArchiveUri = undefined;
+      expect(() => db.resolveSourceFile('abc')).to.throw('Scheme is missing');
+    });
+
+    it('should fail to resolve when not a file uri', () => {
+      const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
+      (db as any)._contents.sourceArchiveUri = undefined;
+      expect(() => db.resolveSourceFile('http://abc')).to.throw('Invalid uri scheme');
+    });
+
+    describe('no source archive', () => {
+      it('should resolve undefined', () => {
+        const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
+        (db as any)._contents.sourceArchiveUri = undefined;
+        const resolved = db.resolveSourceFile(undefined);
+        expect(resolved.toString(true)).to.eq(dbLocationUri().toString(true));
+      });
+
+      it('should resolve an empty file', () => {
+        const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
+        (db as any)._contents.sourceArchiveUri = undefined;
+        const resolved = db.resolveSourceFile('file:');
+        expect(resolved.toString()).to.eq('file:///');
+      });
+    });
+
+    describe('zipped source archive', () => {
+      it('should encode a source archive url', () => {
+        const db = createMockDB(encodeSourceArchiveUri({
+          sourceArchiveZipPath: 'sourceArchive-uri',
+          pathWithinSourceArchive: 'def'
+        }));
+        const resolved = db.resolveSourceFile(Uri.file('abc').toString());
+
+        // must recreate an encoded archive uri instead of typing out the
+        // text since the uris will be different on windows and ubuntu.
+        expect(resolved.toString()).to.eq(encodeSourceArchiveUri({
+          sourceArchiveZipPath: 'sourceArchive-uri',
+          pathWithinSourceArchive: 'def/abc'
+        }).toString());
+      });
+
+      it('should encode a source archive url with trailing slash', () => {
+        const db = createMockDB(encodeSourceArchiveUri({
+          sourceArchiveZipPath: 'sourceArchive-uri',
+          pathWithinSourceArchive: 'def/'
+        }));
+        const resolved = db.resolveSourceFile(Uri.file('abc').toString());
+
+        // must recreate an encoded archive uri instead of typing out the
+        // text since the uris will be different on windows and ubuntu.
+        expect(resolved.toString()).to.eq(encodeSourceArchiveUri({
+          sourceArchiveZipPath: 'sourceArchive-uri',
+          pathWithinSourceArchive: 'def/abc'
+        }).toString());
+      });
+
+      it('should encode an empty source archive url', () => {
+        const db = createMockDB(encodeSourceArchiveUri({
+          sourceArchiveZipPath: 'sourceArchive-uri',
+          pathWithinSourceArchive: 'def'
+        }));
+        const resolved = db.resolveSourceFile('file:');
+        expect(resolved.toString()).to.eq('codeql-zip-archive://1-18/sourceArchive-uri/def/');
+      });
+    });
+
+    it('should handle an empty file', () => {
+      const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
+      const resolved = db.resolveSourceFile('');
+      expect(resolved.toString()).to.eq('file:///sourceArchive-uri/');
+    });
+  });
+
+  it('should find likely db language folders', () => {
+    expect(isLikelyDbLanguageFolder('db-javascript')).to.be.true;
+    expect(isLikelyDbLanguageFolder('dbnot-a-db')).to.be.false;
+  });
+
+  function createMockDB(
+    // source archive location must be a real(-ish) location since
+    // tests will add this to the workspace location
+    sourceArchiveUri = sourceLocationUri(),
+    databaseUri = dbLocationUri()
+  ): DatabaseItemImpl {
+
+    return new DatabaseItemImpl(
+      databaseUri,
+      {
+        sourceArchiveUri
+      } as DatabaseContents,
+      MOCK_DB_OPTIONS,
+      dbChangedHandler
+    );
+  }
+
+  function sourceLocationUri() {
+    return Uri.file(path.join(dir.name, 'src.zip'));
+  }
+
+  function dbLocationUri() {
+    return Uri.file(path.join(dir.name, 'db'));
+  }
+});

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

@@ -1,4 +1,12 @@
 import { runTestsInDirectory } from '../index-template';
+
+import * as sinonChai from 'sinon-chai';
+import * as chai from 'chai';
+import * as chaiAsPromised from 'chai-as-promised';
+chai.use(chaiAsPromised);
+chai.use(sinonChai);
+
+
 export function run(): Promise<void> {
   return runTestsInDirectory(__dirname);
 }

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

@@ -1,188 +0,0 @@
-import 'vscode-test';
-import 'mocha';
-import * as sinon from 'sinon';
-import { expect } from 'chai';
-import { ExtensionContext, Uri } from 'vscode';
-
-import {
-  DatabaseEventKind,
-  DatabaseItem,
-  DatabaseManager,
-  DatabaseItemImpl,
-  DatabaseContents,
-  isLikelyDbLanguageFolder
-} from '../../databases';
-import { QueryServerConfig } from '../../config';
-import { Logger } from '../../logging';
-import { encodeSourceArchiveUri } from '../../archive-filesystem-provider';
-
-describe('databases', () => {
-  let databaseManager: DatabaseManager;
-  let updateSpy: sinon.SinonSpy;
-
-  beforeEach(() => {
-    updateSpy = sinon.spy();
-    databaseManager = new DatabaseManager(
-      {
-        workspaceState: {
-          update: updateSpy,
-          get: sinon.stub()
-        },
-      } as unknown as ExtensionContext,
-      {} as QueryServerConfig,
-      {} as Logger,
-    );
-  });
-
-  it('should fire events when adding and removing a db item', () => {
-    const mockDbItem = {
-      databaseUri: { path: 'file:/abc' },
-      name: 'abc',
-      getPersistedState() {
-        return this.name;
-      }
-    };
-    const spy = sinon.spy();
-    databaseManager.onDidChangeDatabaseItem(spy);
-    (databaseManager as any).addDatabaseItem(mockDbItem);
-
-    expect((databaseManager as any)._databaseItems).to.deep.eq([mockDbItem]);
-    expect(updateSpy).to.have.been.calledWith('databaseList', ['abc']);
-    expect(spy).to.have.been.calledWith({
-      item: undefined,
-      kind: DatabaseEventKind.Add
-    });
-
-    sinon.reset();
-
-    // now remove the item
-    databaseManager.removeDatabaseItem(mockDbItem as unknown as DatabaseItem);
-    expect((databaseManager as any)._databaseItems).to.deep.eq([]);
-    expect(updateSpy).to.have.been.calledWith('databaseList', []);
-    expect(spy).to.have.been.calledWith({
-      item: undefined,
-      kind: DatabaseEventKind.Remove
-    });
-  });
-
-  it('should rename a db item and emit an event', () => {
-    const mockDbItem = {
-      databaseUri: 'file:/abc',
-      name: 'abc',
-      getPersistedState() {
-        return this.name;
-      }
-    };
-    const spy = sinon.spy();
-    databaseManager.onDidChangeDatabaseItem(spy);
-    (databaseManager as any).addDatabaseItem(mockDbItem);
-    sinon.restore();
-
-    databaseManager.renameDatabaseItem(mockDbItem as unknown as DatabaseItem, 'new name');
-
-    expect(mockDbItem.name).to.eq('new name');
-    expect(updateSpy).to.have.been.calledWith('databaseList', ['new name']);
-    expect(spy).to.have.been.calledWith({
-      item: undefined,
-      kind: DatabaseEventKind.Rename
-    });
-  });
-
-  describe('resolveSourceFile', () => {
-    it('should fail to resolve when not a uri', () => {
-      const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
-      (db as any)._contents.sourceArchiveUri = undefined;
-      expect(() => db.resolveSourceFile('abc')).to.throw('Scheme is missing');
-    });
-
-    it('should fail to resolve when not a file uri', () => {
-      const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
-      (db as any)._contents.sourceArchiveUri = undefined;
-      expect(() => db.resolveSourceFile('http://abc')).to.throw('Invalid uri scheme');
-    });
-
-    describe('no source archive', () => {
-      it('should resolve undefined', () => {
-        const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
-        (db as any)._contents.sourceArchiveUri = undefined;
-        const resolved = db.resolveSourceFile(undefined);
-        expect(resolved.toString()).to.eq('file:///database-uri');
-      });
-
-      it('should resolve an empty file', () => {
-        const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
-        (db as any)._contents.sourceArchiveUri = undefined;
-        const resolved = db.resolveSourceFile('file:');
-        expect(resolved.toString()).to.eq('file:///');
-      });
-    });
-
-    describe('zipped source archive', () => {
-      it('should encode a source archive url', () => {
-        const db = createMockDB(encodeSourceArchiveUri({
-          sourceArchiveZipPath: 'sourceArchive-uri',
-          pathWithinSourceArchive: 'def'
-        }));
-        const resolved = db.resolveSourceFile(Uri.file('abc').toString());
-
-        // must recreate an encoded archive uri instead of typing out the
-        // text since the uris will be different on windows and ubuntu.
-        expect(resolved.toString()).to.eq(encodeSourceArchiveUri({
-          sourceArchiveZipPath: 'sourceArchive-uri',
-          pathWithinSourceArchive: 'def/abc'
-        }).toString());
-      });
-
-      it('should encode a source archive url with trailing slash', () => {
-        const db = createMockDB(encodeSourceArchiveUri({
-          sourceArchiveZipPath: 'sourceArchive-uri',
-          pathWithinSourceArchive: 'def/'
-        }));
-        const resolved = db.resolveSourceFile(Uri.file('abc').toString());
-
-        // must recreate an encoded archive uri instead of typing out the
-        // text since the uris will be different on windows and ubuntu.
-        expect(resolved.toString()).to.eq(encodeSourceArchiveUri({
-          sourceArchiveZipPath: 'sourceArchive-uri',
-          pathWithinSourceArchive: 'def/abc'
-        }).toString());
-      });
-
-      it('should encode an empty source archive url', () => {
-        const db = createMockDB(encodeSourceArchiveUri({
-          sourceArchiveZipPath: 'sourceArchive-uri',
-          pathWithinSourceArchive: 'def'
-        }));
-        const resolved = db.resolveSourceFile('file:');
-        expect(resolved.toString()).to.eq('codeql-zip-archive://1-18/sourceArchive-uri/def/');
-      });
-    });
-
-    it('should handle an empty file', () => {
-      const db = createMockDB(Uri.parse('file:/sourceArchive-uri/'));
-      const resolved = db.resolveSourceFile('');
-      expect(resolved.toString()).to.eq('file:///sourceArchive-uri/');
-    });
-    function createMockDB(
-      sourceArchiveUri = Uri.parse('file:/sourceArchive-uri'),
-      databaseUri = Uri.parse('file:/database-uri')
-    ) {
-      return new DatabaseItemImpl(
-        databaseUri,
-        {
-          sourceArchiveUri
-        } as DatabaseContents,
-        {
-          dateAdded: 123,
-          ignoreSourceArchive: false
-        },
-        () => { /**/ }
-      );
-    }
-  });
-
-  it('should find likely db language folders', () => {
-    expect(isLikelyDbLanguageFolder('db-javascript')).to.be.true;
-    expect(isLikelyDbLanguageFolder('dbnot-a-db')).to.be.false;
-  });
-});

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

@@ -19,28 +19,26 @@ describe('query-history', () => {
   let showQuickPickSpy: sinon.SinonStub;
 
   let tryOpenExternalFile: Function;
+  let sandbox: sinon.SinonSandbox;
 
   beforeEach(() => {
-    showTextDocumentSpy = sinon.stub(vscode.window, 'showTextDocument');
-    showInformationMessageSpy = sinon.stub(
+    sandbox = sinon.createSandbox();
+    showTextDocumentSpy = sandbox.stub(vscode.window, 'showTextDocument');
+    showInformationMessageSpy = sandbox.stub(
       vscode.window,
       'showInformationMessage'
     );
-    showQuickPickSpy = sinon.stub(
+    showQuickPickSpy = sandbox.stub(
       vscode.window,
       'showQuickPick'
     );
-    executeCommandSpy = sinon.stub(vscode.commands, 'executeCommand');
-    sinon.stub(logger, 'log');
+    executeCommandSpy = sandbox.stub(vscode.commands, 'executeCommand');
+    sandbox.stub(logger, 'log');
     tryOpenExternalFile = (QueryHistoryManager.prototype as any).tryOpenExternalFile;
   });
 
   afterEach(() => {
-    (vscode.window.showTextDocument as sinon.SinonStub).restore();
-    (vscode.commands.executeCommand as sinon.SinonStub).restore();
-    (logger.log as sinon.SinonStub).restore();
-    (vscode.window.showInformationMessage as sinon.SinonStub).restore();
-    (vscode.window.showQuickPick as sinon.SinonStub).restore();
+    sandbox.restore();
   });
 
   describe('tryOpenExternalFile', () => {