Address pull request comments

Author: aeisenberg

extensions/ql-vscode/src/databases/database-fetcher.ts

@@ -298,7 +298,6 @@ export async function importLocalDatabase(
   try {
     const origin: DatabaseOrigin = {
       type: databaseUrl.endsWith(".testproj") ? "testproj" : "archive",
-      // TODO validate that archive origins can use a file path, not a URI
       path: Uri.parse(databaseUrl).fsPath,
     };
     const item = await fetchDatabaseToWorkspaceStorage(
@@ -371,7 +370,7 @@ async function fetchDatabaseToWorkspaceStorage(
 
   if (isFile(databaseUrl)) {
     if (origin.type === "testproj") {
-      await copyDatabase(origin.path, unzipPath, progress);
+      await copyDatabase(databaseUrl, unzipPath, progress);
     } else {
       await readAndUnzip(databaseUrl, unzipPath, cli, progress);
     }
@@ -458,12 +457,12 @@ function validateUrl(databaseUrl: string) {
 
 /**
  * Copies a database folder from the file system into the workspace storage.
- * @param scrDir the original location of the database
+ * @param scrDirURL the original location of the database as a URL string
  * @param destDir the location to copy the database to. This should be a folder in the workspace storage.
  * @param progress callback to send progress messages to
  */
 async function copyDatabase(
-  scrDir: string,
+  srcDirURL: string,
   destDir: string,
   progress?: ProgressCallback,
 ) {
@@ -473,7 +472,7 @@ async function copyDatabase(
     message: `Copying database ${basename(destDir)} into the workspace`,
   });
   await ensureDir(destDir);
-  await copy(scrDir, destDir);
+  await copy(Uri.parse(srcDirURL).fsPath, destDir);
 }
 
 async function readAndUnzip(

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

@@ -750,7 +750,6 @@ export class DatabaseUI extends DisposableObject {
     return withProgress(
       async (progress) => {
         try {
-          // Assume user has selected an archive if the file has a .zip extension
           if (!uri.path.endsWith(".testproj")) {
             throw new Error(
               "Please select a valid test database to import. Test databases end with `.testproj`.",

extensions/ql-vscode/src/databases/local-databases/database-manager.ts

@@ -221,10 +221,15 @@ export class DatabaseManager extends DisposableObject {
       "codeql-database.yml",
     );
 
-    // TODO add error handling if one does not exist.
-    const originStat = await stat(originDbYml);
-    const importedStat = await stat(importedDbYml);
-    return originStat.mtimeMs > importedStat.mtimeMs;
+    try {
+      const originStat = await stat(originDbYml);
+      const importedStat = await stat(importedDbYml);
+      return originStat.mtimeMs > importedStat.mtimeMs;
+    } catch (e) {
+      // If either of the files does not exist, we assume the origin is newer.
+      // This shouldn't happen unless the user manually deleted one of the files.
+      return true;
+    }
   }
 
   /**

extensions/ql-vscode/src/query-server/query-runner.ts

@@ -63,15 +63,15 @@ export interface CoreQueryRun {
 export type CoreCompletedQuery = CoreQueryResults &
   Omit<CoreQueryRun, "evaluate">;
 
-type OnQueryRunStargingListener = (dbPath: Uri) => Promise<void>;
+type OnQueryRunStartingListener = (dbPath: Uri) => Promise<void>;
 export class QueryRunner {
   constructor(public readonly qs: QueryServerClient) {}
 
   // Event handlers that get notified whenever a query is about to start running.
   // Can't use vscode EventEmitters since they are not asynchronous.
-  private readonly onQueryRunStartingListeners: OnQueryRunStargingListener[] =
+  private readonly onQueryRunStartingListeners: OnQueryRunStartingListener[] =
     [];
-  public onQueryRunStarting(listener: OnQueryRunStargingListener) {
+  public onQueryRunStarting(listener: OnQueryRunStartingListener) {
     this.onQueryRunStartingListeners.push(listener);
   }
 

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

@@ -12,7 +12,7 @@ import fetch from "node-fetch";
 import { renameSync } from "fs";
 import { unzipToDirectoryConcurrently } from "../../../src/common/unzip-concurrently";
 import { platform } from "os";
-import { wait } from "./utils";
+import { sleep } from "../../../src/common/time";
 
 beforeAll(async () => {
   // ensure the test database is downloaded
@@ -42,7 +42,7 @@ beforeAll(async () => {
     // On Windows, wait a few seconds to make sure all background processes
     // release their lock on the files before renaming the directory.
     if (platform() === "win32") {
-      await wait(3000);
+      await sleep(3000);
     }
     renameSync(join(dbParentDir, "db"), testprojLoc);
     console.log("Unzip completed.");

extensions/ql-vscode/test/vscode-tests/cli-integration/utils.ts

@@ -7,7 +7,3 @@ import { join } from "path";
 export function getDataFolderFilePath(path: string): string {
   return join(workspace.workspaceFolders![0].uri.fsPath, path);
 }
-
-export async function wait(ms = 1000) {
-  return new Promise((resolve) => setTimeout(resolve, ms));
-}