Avoid uninteresting user facing errors

extensions/ql-vscode/CHANGELOG.md

@@ -9,6 +9,7 @@
 - Add clearer error message when running a query using a missing or invalid qlpack. [#702](https://github.com/github/vscode-codeql/pull/702)
 - Add clearer error message when trying to run a command from the query history view if no item in the history is selected. [#702](https://github.com/github/vscode-codeql/pull/702)
 - Fix a bug where it is not possible to download some database archives. This fix specifically addresses large archives and archives whose central directories do not align with file headers. [#700](https://github.com/github/vscode-codeql/pull/700)
+- Avoid error dialogs when QL test discovery or database cleanup encounters a missing directory. [#706](https://github.com/github/vscode-codeql/pull/706)
 
 ## 1.3.7 - 24 November 2020
 

extensions/ql-vscode/src/databases-ui.ts

@@ -378,7 +378,17 @@ export class DatabaseUI extends DisposableObject {
 
   handleRemoveOrphanedDatabases = async (): Promise<void> => {
     logger.log('Removing orphaned databases from workspace storage.');
-    let dbDirs =
+    let dbDirs = undefined;
+
+    if (
+      !(await fs.pathExists(this.storagePath) ||
+        !(await fs.stat(this.storagePath)).isDirectory())
+    ) {
+      logger.log('Missing or invalid storage directory. Not trying to remove orphaned databases.');
+      return;
+    }
+
+    dbDirs =
       // read directory
       (await fs.readdir(this.storagePath, { withFileTypes: true }))
         // remove non-directories

extensions/ql-vscode/src/discovery.ts

@@ -1,5 +1,5 @@
 import { DisposableObject } from './vscode-utils/disposable-object';
-import { showAndLogErrorMessage } from './helpers';
+import { logger } from './logging';
 
 /**
  * Base class for "discovery" operations, which scan the file system to find specific kinds of
@@ -62,7 +62,7 @@ export abstract class Discovery<T> extends DisposableObject {
     });
 
     discoveryPromise.catch(err => {
-      showAndLogErrorMessage(`${this.name} failed. Reason: ${err.message}`);
+      logger.log(`${this.name} failed. Reason: ${err.message}`);
     });
 
     discoveryPromise.finally(() => {

extensions/ql-vscode/src/helpers.ts

@@ -123,15 +123,16 @@ export function commandRunner(
     try {
       return await task(...args);
     } catch (e) {
+      const errorMessage = `${e.message || e} (${commandId})`;
       if (e instanceof UserCancellationException) {
         // User has cancelled this action manually
         if (e.silent) {
-          logger.log(e.message);
+          logger.log(errorMessage);
         } else {
-          showAndLogWarningMessage(e.message);
+          showAndLogWarningMessage(errorMessage);
         }
       } else {
-        showAndLogErrorMessage(e.message || e);
+        showAndLogErrorMessage(errorMessage);
       }
       return undefined;
     }
@@ -161,15 +162,16 @@ export function commandRunnerWithProgress<R>(
     try {
       return await withProgress(progressOptionsWithDefaults, task, ...args);
     } catch (e) {
+      const errorMessage = `${e.message || e} (${commandId})`;
       if (e instanceof UserCancellationException) {
         // User has cancelled this action manually
         if (e.silent) {
-          logger.log(e.message);
+          logger.log(errorMessage);
         } else {
-          showAndLogWarningMessage(e.message);
+          showAndLogWarningMessage(errorMessage);
         }
       } else {
-        showAndLogErrorMessage(e.message || e);
+        showAndLogErrorMessage(errorMessage);
       }
       return undefined;
     }

extensions/ql-vscode/src/qltest-discovery.ts

@@ -3,6 +3,7 @@ import { Discovery } from './discovery';
 import { EventEmitter, Event, Uri, RelativePattern, WorkspaceFolder, env } from 'vscode';
 import { MultiFileSystemWatcher } from './vscode-utils/multi-file-system-watcher';
 import { CodeQLCliServer } from './cli';
+import * as fs from 'fs-extra';
 
 /**
  * A node in the tree of tests. This will be either a `QLTestDirectory` or a `QLTestFile`.
@@ -180,19 +181,21 @@ export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
   private async discoverTests(): Promise<QLTestDirectory> {
     const fullPath = this.workspaceFolder.uri.fsPath;
     const name = this.workspaceFolder.name;
-    const resolvedTests = (await this.cliServer.resolveTests(fullPath))
-      .filter((testPath) => !QLTestDiscovery.ignoreTestPath(testPath));
-
     const rootDirectory = new QLTestDirectory(fullPath, name);
-    for (const testPath of resolvedTests) {
-      const relativePath = path.normalize(path.relative(fullPath, testPath));
-      const dirName = path.dirname(relativePath);
-      const parentDirectory = rootDirectory.createDirectory(dirName);
-      parentDirectory.addChild(new QLTestFile(testPath, path.basename(testPath)));
-    }
 
-    rootDirectory.finish();
+    // Don't try discovery on workspace folders that don't exist on the filesystem
+    if ((await fs.pathExists(fullPath))) {
+      const resolvedTests = (await this.cliServer.resolveTests(fullPath))
+        .filter((testPath) => !QLTestDiscovery.ignoreTestPath(testPath));
+      for (const testPath of resolvedTests) {
+        const relativePath = path.normalize(path.relative(fullPath, testPath));
+        const dirName = path.dirname(relativePath);
+        const parentDirectory = rootDirectory.createDirectory(dirName);
+        parentDirectory.addChild(new QLTestFile(testPath, path.basename(testPath)));
+      }
 
+      rootDirectory.finish();
+    }
     return rootDirectory;
   }
 

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

@@ -2,20 +2,27 @@ import 'vscode-test';
 import 'mocha';
 import { Uri, WorkspaceFolder } from 'vscode';
 import { expect } from 'chai';
+import * as fs from 'fs-extra';
+import * as sinon from 'sinon';
 
 import { QLTestDiscovery } from '../../qltest-discovery';
 
 describe('qltest-discovery', () => {
   describe('discoverTests', () => {
-    it('should run discovery', async () => {
-      const baseUri = Uri.parse('file:/a/b');
-      const baseDir = baseUri.fsPath;
-      const cDir = Uri.parse('file:/a/b/c').fsPath;
-      const dFile = Uri.parse('file:/a/b/c/d.ql').fsPath;
-      const eFile = Uri.parse('file:/a/b/c/e.ql').fsPath;
-      const hDir = Uri.parse('file:/a/b/c/f/g/h').fsPath;
-      const iFile = Uri.parse('file:/a/b/c/f/g/h/i.ql').fsPath;
-      const qlTestDiscover = new QLTestDiscovery(
+    let sandbox: sinon.SinonSandbox;
+
+    const baseUri = Uri.parse('file:/a/b');
+    const baseDir = baseUri.fsPath;
+    const cDir = Uri.parse('file:/a/b/c').fsPath;
+    const dFile = Uri.parse('file:/a/b/c/d.ql').fsPath;
+    const eFile = Uri.parse('file:/a/b/c/e.ql').fsPath;
+    const hDir = Uri.parse('file:/a/b/c/f/g/h').fsPath;
+    const iFile = Uri.parse('file:/a/b/c/f/g/h/i.ql').fsPath;
+    let qlTestDiscover: QLTestDiscovery;
+
+    beforeEach(() => {
+      sandbox = sinon.createSandbox();
+      qlTestDiscover = new QLTestDiscovery(
         {
           uri: baseUri,
           name: 'My tests'
@@ -31,6 +38,15 @@ describe('qltest-discovery', () => {
         } as any
       );
 
+    });
+
+    afterEach(() => {
+      sandbox.restore();
+    });
+
+    it('should run discovery', async () => {
+      sandbox.stub(fs, 'pathExists').resolves(true);
+
       const result = await (qlTestDiscover as any).discover();
       expect(result.watchPath).to.eq(baseDir);
       expect(result.testDirectory.path).to.eq(baseDir);
@@ -56,5 +72,15 @@ describe('qltest-discovery', () => {
       expect(children[0].path).to.eq(iFile);
       expect(children[0].name).to.eq('i.ql');
     });
+
+    it('should avoid discovery if a folder does not exist', async () => {
+      sandbox.stub(fs, 'pathExists').resolves(false);
+      const result = await (qlTestDiscover as any).discover();
+      expect(result.watchPath).to.eq(baseDir);
+      expect(result.testDirectory.path).to.eq(baseDir);
+      expect(result.testDirectory.name).to.eq('My tests');
+
+      expect(result.testDirectory.children.length).to.eq(0);
+    });
   });
 });