Merge pull request #1838 from github/charis-nora/run-mrva-against-selected-db

Use currently selected remote DB when Variant Analysis is run

extensions/ql-vscode/src/databases/db-module.ts

@@ -9,6 +9,16 @@ import { DbPanel } from "./ui/db-panel";
 import { DbSelectionDecorationProvider } from "./ui/db-selection-decoration-provider";
 
 export class DbModule extends DisposableObject {
+  public readonly dbManager: DbManager;
+  private readonly dbConfigStore: DbConfigStore;
+
+  constructor(app: App) {
+    super();
+
+    this.dbConfigStore = new DbConfigStore(app);
+    this.dbManager = new DbManager(app, this.dbConfigStore);
+  }
+
   public async initialize(app: App): Promise<void> {
     if (
       app.mode !== AppMode.Development ||
@@ -23,15 +33,13 @@ export class DbModule extends DisposableObject {
 
     void extLogger.log("Initializing database module");
 
-    const dbConfigStore = new DbConfigStore(app);
-    await dbConfigStore.initialize();
+    await this.dbConfigStore.initialize();
 
-    const dbManager = new DbManager(app, dbConfigStore);
-    const dbPanel = new DbPanel(dbManager);
+    const dbPanel = new DbPanel(this.dbManager);
     await dbPanel.initialize();
 
     this.push(dbPanel);
-    this.push(dbConfigStore);
+    this.push(this.dbConfigStore);
 
     const dbSelectionDecorationProvider = new DbSelectionDecorationProvider();
 
@@ -40,7 +48,7 @@ export class DbModule extends DisposableObject {
 }
 
 export async function initializeDbModule(app: App): Promise<DbModule> {
-  const dbModule = new DbModule();
+  const dbModule = new DbModule(app);
   await dbModule.initialize(app);
   return dbModule;
 }

extensions/ql-vscode/src/extension.ts

@@ -622,6 +622,11 @@ async function activateWithInstalledDistribution(
   ctx.subscriptions.push(localQueryResultsView);
 
   void extLogger.log("Initializing variant analysis manager.");
+
+  const app = new ExtensionApp(ctx);
+  const dbModule = await initializeDbModule(app);
+  ctx.subscriptions.push(dbModule);
+
   const variantAnalysisStorageDir = join(
     ctx.globalStorageUri.fsPath,
     "variant-analyses",
@@ -636,6 +641,7 @@ async function activateWithInstalledDistribution(
     cliServer,
     variantAnalysisStorageDir,
     variantAnalysisResultsManager,
+    dbModule.dbManager,
   );
   ctx.subscriptions.push(variantAnalysisManager);
   ctx.subscriptions.push(variantAnalysisResultsManager);
@@ -1580,10 +1586,6 @@ async function activateWithInstalledDistribution(
   void extLogger.log("Reading query history");
   await qhm.readQueryHistory();
 
-  const app = new ExtensionApp(ctx);
-  const dbModule = await initializeDbModule(app);
-  ctx.subscriptions.push(dbModule);
-
   void extLogger.log("Successfully finished extension initialization.");
 
   return {

extensions/ql-vscode/src/remote-queries/repository-selection.ts

@@ -4,9 +4,12 @@ import { extLogger } from "../common";
 import {
   getRemoteRepositoryLists,
   getRemoteRepositoryListsPath,
+  isNewQueryRunExperienceEnabled,
 } from "../config";
 import { OWNER_REGEX, REPO_REGEX } from "../pure/helpers-pure";
 import { UserCancellationException } from "../commandRunner";
+import { DbManager } from "../databases/db-manager";
+import { DbItemKind } from "../databases/db-item";
 
 export interface RepositorySelection {
   repositories?: string[];
@@ -30,7 +33,33 @@ interface RepoList {
  * Gets the repositories or repository lists to run the query against.
  * @returns The user selection.
  */
-export async function getRepositorySelection(): Promise<RepositorySelection> {
+export async function getRepositorySelection(
+  dbManager?: DbManager,
+): Promise<RepositorySelection> {
+  if (isNewQueryRunExperienceEnabled()) {
+    const selectedDbItem = dbManager?.getSelectedDbItem();
+    if (selectedDbItem) {
+      switch (selectedDbItem.kind) {
+        case DbItemKind.LocalDatabase || DbItemKind.LocalList:
+          throw new Error("Local databases and lists are not supported yet.");
+        case DbItemKind.RemoteSystemDefinedList:
+          return { repositoryLists: [selectedDbItem.listName] };
+        case DbItemKind.RemoteUserDefinedList:
+          return {
+            repositories: selectedDbItem.repos.map((repo) => repo.repoFullName),
+          };
+        case DbItemKind.RemoteOwner:
+          return { owners: [selectedDbItem.ownerName] };
+        case DbItemKind.RemoteRepo:
+          return { repositories: [selectedDbItem.repoFullName] };
+      }
+    } else {
+      throw new Error(
+        "Please select a remote database to run the query against.",
+      );
+    }
+  }
+
   const quickPickItems = [
     createCustomRepoQuickPickItem(),
     createAllReposOfOwnerQuickPickItem(),

extensions/ql-vscode/src/remote-queries/run-remote-query.ts

@@ -29,6 +29,7 @@ import {
   RepositorySelection,
 } from "./repository-selection";
 import { Repository } from "./shared/repository";
+import { DbManager } from "../databases/db-manager";
 
 export interface QlPack {
   name: string;
@@ -213,6 +214,7 @@ export async function prepareRemoteQueryRun(
   uri: Uri | undefined,
   progress: ProgressCallback,
   token: CancellationToken,
+  dbManager?: DbManager, // the dbManager is only needed when the newQueryRunExperience is enabled
 ): Promise<PreparedRemoteQuery> {
   if (!(await cliServer.cliConstraints.supportsRemoteQueries())) {
     throw new Error(
@@ -232,7 +234,7 @@ export async function prepareRemoteQueryRun(
     message: "Determining query target language",
   });
 
-  const repoSelection = await getRepositorySelection();
+  const repoSelection = await getRepositorySelection(dbManager);
   if (!isValidSelection(repoSelection)) {
     throw new UserCancellationException("No repositories to query.");
   }

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

@@ -59,6 +59,7 @@ import {
   RepositoriesFilterSortStateWithIds,
 } from "../pure/variant-analysis-filter-sort";
 import { URLSearchParams } from "url";
+import { DbManager } from "../databases/db-manager";
 
 export class VariantAnalysisManager
   extends DisposableObject
@@ -100,6 +101,7 @@ export class VariantAnalysisManager
     private readonly cliServer: CodeQLCliServer,
     private readonly storagePath: string,
     private readonly variantAnalysisResultsManager: VariantAnalysisResultsManager,
+    private readonly dbManager: DbManager,
   ) {
     super();
     this.variantAnalysisMonitor = this.push(
@@ -140,6 +142,7 @@ export class VariantAnalysisManager
       uri,
       progress,
       token,
+      this.dbManager,
     );
 
     const queryName = getQueryName(queryMetadata, queryFile);

extensions/ql-vscode/src/vscode-tests/cli-integration/remote-queries/variant-analysis-manager.test.ts

@@ -56,6 +56,7 @@ import {
   defaultFilterSortState,
   SortKey,
 } from "../../../pure/variant-analysis-filter-sort";
+import { DbManager } from "../../../databases/db-manager";
 
 // up to 3 minutes per test
 jest.setTimeout(3 * 60 * 1000);
@@ -69,6 +70,7 @@ describe("Variant Analysis Manager", () => {
   let cancellationTokenSource: CancellationTokenSource;
   let variantAnalysisManager: VariantAnalysisManager;
   let variantAnalysisResultsManager: VariantAnalysisResultsManager;
+  let dbManager: DbManager;
   let variantAnalysis: VariantAnalysis;
   let scannedRepos: VariantAnalysisScannedRepository[];
 
@@ -107,6 +109,7 @@ describe("Variant Analysis Manager", () => {
       cli,
       storagePath,
       variantAnalysisResultsManager,
+      dbManager,
     );
   });
 

extensions/ql-vscode/src/vscode-tests/no-workspace/remote-queries/repository-selection.test.ts

@@ -4,169 +4,152 @@ import { UserCancellationException } from "../../../commandRunner";
 
 import * as config from "../../../config";
 import { getRepositorySelection } from "../../../remote-queries/repository-selection";
+import { DbManager } from "../../../databases/db-manager";
+import { DbItem, DbItemKind } from "../../../databases/db-item";
 
 describe("repository selection", () => {
-  let quickPickSpy: jest.SpiedFunction<typeof window.showQuickPick>;
-  let showInputBoxSpy: jest.SpiedFunction<typeof window.showInputBox>;
+  describe("newQueryRunExperience true", () => {
+    beforeEach(() => {
+      jest
+        .spyOn(config, "isNewQueryRunExperienceEnabled")
+        .mockReturnValue(true);
+    });
 
-  let getRemoteRepositoryListsSpy: jest.SpiedFunction<
-    typeof config.getRemoteRepositoryLists
-  >;
-  let getRemoteRepositoryListsPathSpy: jest.SpiedFunction<
-    typeof config.getRemoteRepositoryListsPath
-  >;
+    it("should throw error when no database item is selected", async () => {
+      const dbManager = setUpDbManager(undefined);
 
-  let pathExistsStub: jest.SpiedFunction<typeof fs.pathExists>;
-  let fsStatStub: jest.SpiedFunction<typeof fs.stat>;
-  let fsReadFileStub: jest.SpiedFunction<typeof fs.readFile>;
+      await expect(getRepositorySelection(dbManager)).rejects.toThrow(
+        Error("Please select a remote database to run the query against."),
+      );
+    });
 
-  beforeEach(() => {
-    quickPickSpy = jest
-      .spyOn(window, "showQuickPick")
-      .mockResolvedValue(undefined);
-    showInputBoxSpy = jest
-      .spyOn(window, "showInputBox")
-      .mockResolvedValue(undefined);
+    it("should throw error when local database item is selected", async () => {
+      const dbManager = setUpDbManager({
+        kind: DbItemKind.LocalDatabase,
+      } as DbItem);
 
-    getRemoteRepositoryListsSpy = jest
-      .spyOn(config, "getRemoteRepositoryLists")
-      .mockReturnValue(undefined);
-    getRemoteRepositoryListsPathSpy = jest
-      .spyOn(config, "getRemoteRepositoryListsPath")
-      .mockReturnValue(undefined);
+      await expect(getRepositorySelection(dbManager)).rejects.toThrow(
+        Error("Local databases and lists are not supported yet."),
+      );
+    });
 
-    pathExistsStub = jest
-      .spyOn(fs, "pathExists")
-      .mockImplementation(() => false);
-    fsStatStub = jest
-      .spyOn(fs, "stat")
-      .mockRejectedValue(new Error("not found"));
-    fsReadFileStub = jest
-      .spyOn(fs, "readFile")
-      .mockRejectedValue(new Error("not found"));
-  });
+    it("should return correct selection when remote system defined list is selected", async () => {
+      const dbManager = setUpDbManager({
+        kind: DbItemKind.RemoteSystemDefinedList,
+        listName: "top_10",
+      } as DbItem);
 
-  describe("repo lists from settings", () => {
-    it("should allow selection from repo lists from your pre-defined config", async () => {
-      // Fake return values
-      quickPickSpy.mockResolvedValue({
-        repositories: ["foo/bar", "foo/baz"],
-      } as unknown as QuickPickItem);
-      getRemoteRepositoryListsSpy.mockReturnValue({
-        list1: ["foo/bar", "foo/baz"],
-        list2: [],
-      });
+      const repoSelection = await getRepositorySelection(dbManager);
 
-      // Make the function call
-      const repoSelection = await getRepositorySelection();
+      expect(repoSelection.repositoryLists).toEqual(["top_10"]);
+      expect(repoSelection.owners).toBeUndefined();
+      expect(repoSelection.repositories).toBeUndefined();
+    });
+
+    it("should return correct selection when remote user defined list is selected", async () => {
+      const dbManager = setUpDbManager({
+        kind: DbItemKind.RemoteUserDefinedList,
+        repos: [
+          { repoFullName: "owner1/repo1" },
+          { repoFullName: "owner1/repo2" },
+        ],
+      } as DbItem);
+
+      const repoSelection = await getRepositorySelection(dbManager);
 
-      // Check that the return value is correct
       expect(repoSelection.repositoryLists).toBeUndefined();
       expect(repoSelection.owners).toBeUndefined();
-      expect(repoSelection.repositories).toEqual(["foo/bar", "foo/baz"]);
+      expect(repoSelection.repositories).toEqual([
+        "owner1/repo1",
+        "owner1/repo2",
+      ]);
     });
-  });
 
-  describe("system level repo lists", () => {
-    it("should allow selection from repo lists defined at the system level", async () => {
-      // Fake return values
-      quickPickSpy.mockResolvedValue({
-        repositoryList: "top_100",
-      } as unknown as QuickPickItem);
-      getRemoteRepositoryListsSpy.mockReturnValue({
-        list1: ["foo/bar", "foo/baz"],
-        list2: [],
-      });
+    it("should return correct selection when remote owner is selected", async () => {
+      const dbManager = setUpDbManager({
+        kind: DbItemKind.RemoteOwner,
+        ownerName: "owner2",
+      } as DbItem);
 
-      // Make the function call
-      const repoSelection = await getRepositorySelection();
+      const repoSelection = await getRepositorySelection(dbManager);
 
-      // Check that the return value is correct
+      expect(repoSelection.repositoryLists).toBeUndefined();
+      expect(repoSelection.owners).toEqual(["owner2"]);
       expect(repoSelection.repositories).toBeUndefined();
+    });
+
+    it("should return correct selection when remote repo is selected", async () => {
+      const dbManager = setUpDbManager({
+        kind: DbItemKind.RemoteRepo,
+        repoFullName: "owner1/repo2",
+      } as DbItem);
+
+      const repoSelection = await getRepositorySelection(dbManager);
+
+      expect(repoSelection.repositoryLists).toBeUndefined();
       expect(repoSelection.owners).toBeUndefined();
-      expect(repoSelection.repositoryLists).toEqual(["top_100"]);
+      expect(repoSelection.repositories).toEqual(["owner1/repo2"]);
     });
+
+    function setUpDbManager(response: DbItem | undefined): DbManager {
+      return {
+        getSelectedDbItem: jest.fn(() => {
+          return response;
+        }),
+      } as any as DbManager;
+    }
   });
 
-  describe("custom owner", () => {
-    // Test the owner regex in various "good" cases
-    const goodOwners = [
-      "owner",
-      "owner-with-hyphens",
-      "ownerWithNumbers58",
-      "owner_with_underscores",
-      "owner.with.periods.",
-    ];
-    goodOwners.forEach((owner) => {
-      it(`should run on a valid owner that you enter in the text box: ${owner}`, async () => {
+  describe("newQueryRunExperience false", () => {
+    let quickPickSpy: jest.SpiedFunction<typeof window.showQuickPick>;
+    let showInputBoxSpy: jest.SpiedFunction<typeof window.showInputBox>;
+
+    let getRemoteRepositoryListsSpy: jest.SpiedFunction<
+      typeof config.getRemoteRepositoryLists
+    >;
+    let getRemoteRepositoryListsPathSpy: jest.SpiedFunction<
+      typeof config.getRemoteRepositoryListsPath
+    >;
+
+    let pathExistsStub: jest.SpiedFunction<typeof fs.pathExists>;
+    let fsStatStub: jest.SpiedFunction<typeof fs.stat>;
+    let fsReadFileStub: jest.SpiedFunction<typeof fs.readFile>;
+
+    beforeEach(() => {
+      quickPickSpy = jest
+        .spyOn(window, "showQuickPick")
+        .mockResolvedValue(undefined);
+      showInputBoxSpy = jest
+        .spyOn(window, "showInputBox")
+        .mockResolvedValue(undefined);
+
+      getRemoteRepositoryListsSpy = jest
+        .spyOn(config, "getRemoteRepositoryLists")
+        .mockReturnValue(undefined);
+      getRemoteRepositoryListsPathSpy = jest
+        .spyOn(config, "getRemoteRepositoryListsPath")
+        .mockReturnValue(undefined);
+
+      pathExistsStub = jest
+        .spyOn(fs, "pathExists")
+        .mockImplementation(() => false);
+      fsStatStub = jest
+        .spyOn(fs, "stat")
+        .mockRejectedValue(new Error("not found"));
+      fsReadFileStub = jest
+        .spyOn(fs, "readFile")
+        .mockRejectedValue(new Error("not found"));
+    });
+    describe("repo lists from settings", () => {
+      it("should allow selection from repo lists from your pre-defined config", async () => {
         // Fake return values
         quickPickSpy.mockResolvedValue({
-          useAllReposOfOwner: true,
+          repositories: ["foo/bar", "foo/baz"],
         } as unknown as QuickPickItem);
-        getRemoteRepositoryListsSpy.mockReturnValue({}); // no pre-defined repo lists
-        showInputBoxSpy.mockResolvedValue(owner);
-
-        // Make the function call
-        const repoSelection = await getRepositorySelection();
-
-        // Check that the return value is correct
-        expect(repoSelection.repositories).toBeUndefined();
-        expect(repoSelection.repositoryLists).toBeUndefined();
-        expect(repoSelection.owners).toEqual([owner]);
-      });
-    });
-
-    // Test the owner regex in various "bad" cases
-    const badOwners = ["invalid&owner", "owner-with-repo/repo"];
-    badOwners.forEach((owner) => {
-      it(`should show an error message if you enter an invalid owner in the text box: ${owner}`, async () => {
-        // Fake return values
-        quickPickSpy.mockResolvedValue({
-          useAllReposOfOwner: true,
-        } as unknown as QuickPickItem);
-        getRemoteRepositoryListsSpy.mockReturnValue({}); // no pre-defined repo lists
-        showInputBoxSpy.mockResolvedValue(owner);
-
-        // Function call should throw a UserCancellationException
-        await expect(getRepositorySelection()).rejects.toThrow(
-          `Invalid user or organization: ${owner}`,
-        );
-      });
-    });
-
-    it("should be ok for the user to change their mind", async () => {
-      quickPickSpy.mockResolvedValue({
-        useAllReposOfOwner: true,
-      } as unknown as QuickPickItem);
-      getRemoteRepositoryListsSpy.mockReturnValue({});
-
-      // The user pressed escape to cancel the operation
-      showInputBoxSpy.mockResolvedValue(undefined);
-
-      await expect(getRepositorySelection()).rejects.toThrow(
-        "No repositories selected",
-      );
-      await expect(getRepositorySelection()).rejects.toThrow(
-        UserCancellationException,
-      );
-    });
-  });
-
-  describe("custom repo", () => {
-    // Test the repo regex in various "good" cases
-    const goodRepos = [
-      "owner/repo",
-      "owner_with.symbols-/repo.with-symbols_",
-      "ownerWithNumbers58/repoWithNumbers37",
-    ];
-    goodRepos.forEach((repo) => {
-      it(`should run on a valid repo that you enter in the text box: ${repo}`, async () => {
-        // Fake return values
-        quickPickSpy.mockResolvedValue({
-          useCustomRepo: true,
-        } as unknown as QuickPickItem);
-        getRemoteRepositoryListsSpy.mockReturnValue({}); // no pre-defined repo lists
-        showInputBoxSpy.mockResolvedValue(repo);
+        getRemoteRepositoryListsSpy.mockReturnValue({
+          list1: ["foo/bar", "foo/baz"],
+          list2: [],
+        });
 
         // Make the function call
         const repoSelection = await getRepositorySelection();
@@ -174,29 +157,88 @@ describe("repository selection", () => {
         // Check that the return value is correct
         expect(repoSelection.repositoryLists).toBeUndefined();
         expect(repoSelection.owners).toBeUndefined();
-        expect(repoSelection.repositories).toEqual([repo]);
+        expect(repoSelection.repositories).toEqual(["foo/bar", "foo/baz"]);
       });
     });
 
-    // Test the repo regex in various "bad" cases
-    const badRepos = [
-      "invalid*owner/repo",
-      "owner/repo+some&invalid&stuff",
-      "owner-with-no-repo/",
-      "/repo-with-no-owner",
-    ];
-    badRepos.forEach((repo) => {
-      it(`should show an error message if you enter an invalid repo in the text box: ${repo}`, async () => {
+    describe("system level repo lists", () => {
+      it("should allow selection from repo lists defined at the system level", async () => {
         // Fake return values
         quickPickSpy.mockResolvedValue({
-          useCustomRepo: true,
+          repositoryList: "top_100",
         } as unknown as QuickPickItem);
-        getRemoteRepositoryListsSpy.mockReturnValue({}); // no pre-defined repo lists
-        showInputBoxSpy.mockResolvedValue(repo);
+        getRemoteRepositoryListsSpy.mockReturnValue({
+          list1: ["foo/bar", "foo/baz"],
+          list2: [],
+        });
+
+        // Make the function call
+        const repoSelection = await getRepositorySelection();
+
+        // Check that the return value is correct
+        expect(repoSelection.repositories).toBeUndefined();
+        expect(repoSelection.owners).toBeUndefined();
+        expect(repoSelection.repositoryLists).toEqual(["top_100"]);
+      });
+    });
+
+    describe("custom owner", () => {
+      // Test the owner regex in various "good" cases
+      const goodOwners = [
+        "owner",
+        "owner-with-hyphens",
+        "ownerWithNumbers58",
+        "owner_with_underscores",
+        "owner.with.periods.",
+      ];
+      goodOwners.forEach((owner) => {
+        it(`should run on a valid owner that you enter in the text box: ${owner}`, async () => {
+          // Fake return values
+          quickPickSpy.mockResolvedValue({
+            useAllReposOfOwner: true,
+          } as unknown as QuickPickItem);
+          getRemoteRepositoryListsSpy.mockReturnValue({}); // no pre-defined repo lists
+          showInputBoxSpy.mockResolvedValue(owner);
+
+          // Make the function call
+          const repoSelection = await getRepositorySelection();
+
+          // Check that the return value is correct
+          expect(repoSelection.repositories).toBeUndefined();
+          expect(repoSelection.repositoryLists).toBeUndefined();
+          expect(repoSelection.owners).toEqual([owner]);
+        });
+      });
+
+      // Test the owner regex in various "bad" cases
+      const badOwners = ["invalid&owner", "owner-with-repo/repo"];
+      badOwners.forEach((owner) => {
+        it(`should show an error message if you enter an invalid owner in the text box: ${owner}`, async () => {
+          // Fake return values
+          quickPickSpy.mockResolvedValue({
+            useAllReposOfOwner: true,
+          } as unknown as QuickPickItem);
+          getRemoteRepositoryListsSpy.mockReturnValue({}); // no pre-defined repo lists
+          showInputBoxSpy.mockResolvedValue(owner);
+
+          // Function call should throw a UserCancellationException
+          await expect(getRepositorySelection()).rejects.toThrow(
+            `Invalid user or organization: ${owner}`,
+          );
+        });
+      });
+
+      it("should be ok for the user to change their mind", async () => {
+        quickPickSpy.mockResolvedValue({
+          useAllReposOfOwner: true,
+        } as unknown as QuickPickItem);
+        getRemoteRepositoryListsSpy.mockReturnValue({});
+
+        // The user pressed escape to cancel the operation
+        showInputBoxSpy.mockResolvedValue(undefined);
 
-        // Function call should throw a UserCancellationException
         await expect(getRepositorySelection()).rejects.toThrow(
-          "Invalid repository format",
+          "No repositories selected",
         );
         await expect(getRepositorySelection()).rejects.toThrow(
           UserCancellationException,
@@ -204,113 +246,166 @@ describe("repository selection", () => {
       });
     });
 
-    it("should be ok for the user to change their mind", async () => {
-      quickPickSpy.mockResolvedValue({
-        useCustomRepo: true,
-      } as unknown as QuickPickItem);
-      getRemoteRepositoryListsSpy.mockReturnValue({});
+    describe("custom repo", () => {
+      // Test the repo regex in various "good" cases
+      const goodRepos = [
+        "owner/repo",
+        "owner_with.symbols-/repo.with-symbols_",
+        "ownerWithNumbers58/repoWithNumbers37",
+      ];
+      goodRepos.forEach((repo) => {
+        it(`should run on a valid repo that you enter in the text box: ${repo}`, async () => {
+          // Fake return values
+          quickPickSpy.mockResolvedValue({
+            useCustomRepo: true,
+          } as unknown as QuickPickItem);
+          getRemoteRepositoryListsSpy.mockReturnValue({}); // no pre-defined repo lists
+          showInputBoxSpy.mockResolvedValue(repo);
 
-      // The user pressed escape to cancel the operation
-      showInputBoxSpy.mockResolvedValue(undefined);
+          // Make the function call
+          const repoSelection = await getRepositorySelection();
 
-      await expect(getRepositorySelection()).rejects.toThrow(
-        "No repositories selected",
-      );
-      await expect(getRepositorySelection()).rejects.toThrow(
-        UserCancellationException,
-      );
-    });
-  });
-
-  describe("external repository lists file", () => {
-    it("should fail if path does not exist", async () => {
-      const fakeFilePath = "/path/that/does/not/exist.json";
-      getRemoteRepositoryListsPathSpy.mockReturnValue(fakeFilePath);
-      pathExistsStub.mockImplementation(() => false);
-
-      await expect(getRepositorySelection()).rejects.toThrow(
-        `External repository lists file does not exist at ${fakeFilePath}`,
-      );
-    });
-
-    it("should fail if path points to directory", async () => {
-      const fakeFilePath = "/path/to/dir";
-      getRemoteRepositoryListsPathSpy.mockReturnValue(fakeFilePath);
-      pathExistsStub.mockImplementation(() => true);
-      fsStatStub.mockResolvedValue({ isDirectory: () => true } as any);
-
-      await expect(getRepositorySelection()).rejects.toThrow(
-        "External repository lists path should not point to a directory",
-      );
-    });
-
-    it("should fail if file does not have valid JSON", async () => {
-      const fakeFilePath = "/path/to/file.json";
-      getRemoteRepositoryListsPathSpy.mockReturnValue(fakeFilePath);
-      pathExistsStub.mockImplementation(() => true);
-      fsStatStub.mockResolvedValue({ isDirectory: () => false } as any);
-      fsReadFileStub.mockResolvedValue("not-json" as any as Buffer);
-
-      await expect(getRepositorySelection()).rejects.toThrow(
-        "Invalid repository lists file. It should contain valid JSON.",
-      );
-    });
-
-    it("should fail if file contains array", async () => {
-      const fakeFilePath = "/path/to/file.json";
-      getRemoteRepositoryListsPathSpy.mockReturnValue(fakeFilePath);
-      pathExistsStub.mockImplementation(() => true);
-      fsStatStub.mockResolvedValue({ isDirectory: () => false } as any);
-      fsReadFileStub.mockResolvedValue("[]" as any as Buffer);
-
-      await expect(getRepositorySelection()).rejects.toThrow(
-        "Invalid repository lists file. It should be an object mapping names to a list of repositories.",
-      );
-    });
-
-    it("should fail if file does not contain repo lists in the right format", async () => {
-      const fakeFilePath = "/path/to/file.json";
-      getRemoteRepositoryListsPathSpy.mockReturnValue(fakeFilePath);
-      pathExistsStub.mockImplementation(() => true);
-      fsStatStub.mockResolvedValue({ isDirectory: () => false } as any);
-      const repoLists = {
-        list1: "owner1/repo1",
-      };
-      fsReadFileStub.mockResolvedValue(
-        JSON.stringify(repoLists) as any as Buffer,
-      );
-
-      await expect(getRepositorySelection()).rejects.toThrow(
-        "Invalid repository lists file. It should contain an array of repositories for each list.",
-      );
-    });
-
-    it("should get repo lists from file", async () => {
-      const fakeFilePath = "/path/to/file.json";
-      getRemoteRepositoryListsPathSpy.mockReturnValue(fakeFilePath);
-      pathExistsStub.mockImplementation(() => true);
-      fsStatStub.mockResolvedValue({ isDirectory: () => false } as any);
-      const repoLists = {
-        list1: ["owner1/repo1", "owner2/repo2"],
-        list2: ["owner3/repo3"],
-      };
-      fsReadFileStub.mockResolvedValue(
-        JSON.stringify(repoLists) as any as Buffer,
-      );
-      getRemoteRepositoryListsSpy.mockReturnValue({
-        list3: ["onwer4/repo4"],
-        list4: [],
+          // Check that the return value is correct
+          expect(repoSelection.repositoryLists).toBeUndefined();
+          expect(repoSelection.owners).toBeUndefined();
+          expect(repoSelection.repositories).toEqual([repo]);
+        });
       });
 
-      quickPickSpy.mockResolvedValue({
-        repositories: ["owner3/repo3"],
-      } as unknown as QuickPickItem);
+      // Test the repo regex in various "bad" cases
+      const badRepos = [
+        "invalid*owner/repo",
+        "owner/repo+some&invalid&stuff",
+        "owner-with-no-repo/",
+        "/repo-with-no-owner",
+      ];
+      badRepos.forEach((repo) => {
+        it(`should show an error message if you enter an invalid repo in the text box: ${repo}`, async () => {
+          // Fake return values
+          quickPickSpy.mockResolvedValue({
+            useCustomRepo: true,
+          } as unknown as QuickPickItem);
+          getRemoteRepositoryListsSpy.mockReturnValue({}); // no pre-defined repo lists
+          showInputBoxSpy.mockResolvedValue(repo);
 
-      const repoSelection = await getRepositorySelection();
+          // Function call should throw a UserCancellationException
+          await expect(getRepositorySelection()).rejects.toThrow(
+            "Invalid repository format",
+          );
+          await expect(getRepositorySelection()).rejects.toThrow(
+            UserCancellationException,
+          );
+        });
+      });
 
-      expect(repoSelection.repositoryLists).toBeUndefined();
-      expect(repoSelection.owners).toBeUndefined();
-      expect(repoSelection.repositories).toEqual(["owner3/repo3"]);
+      it("should be ok for the user to change their mind", async () => {
+        quickPickSpy.mockResolvedValue({
+          useCustomRepo: true,
+        } as unknown as QuickPickItem);
+        getRemoteRepositoryListsSpy.mockReturnValue({});
+
+        // The user pressed escape to cancel the operation
+        showInputBoxSpy.mockResolvedValue(undefined);
+
+        await expect(getRepositorySelection()).rejects.toThrow(
+          "No repositories selected",
+        );
+        await expect(getRepositorySelection()).rejects.toThrow(
+          UserCancellationException,
+        );
+      });
+    });
+
+    describe("external repository lists file", () => {
+      it("should fail if path does not exist", async () => {
+        const fakeFilePath = "/path/that/does/not/exist.json";
+        getRemoteRepositoryListsPathSpy.mockReturnValue(fakeFilePath);
+        pathExistsStub.mockImplementation(() => false);
+
+        await expect(getRepositorySelection()).rejects.toThrow(
+          `External repository lists file does not exist at ${fakeFilePath}`,
+        );
+      });
+
+      it("should fail if path points to directory", async () => {
+        const fakeFilePath = "/path/to/dir";
+        getRemoteRepositoryListsPathSpy.mockReturnValue(fakeFilePath);
+        pathExistsStub.mockImplementation(() => true);
+        fsStatStub.mockResolvedValue({ isDirectory: () => true } as any);
+
+        await expect(getRepositorySelection()).rejects.toThrow(
+          "External repository lists path should not point to a directory",
+        );
+      });
+
+      it("should fail if file does not have valid JSON", async () => {
+        const fakeFilePath = "/path/to/file.json";
+        getRemoteRepositoryListsPathSpy.mockReturnValue(fakeFilePath);
+        pathExistsStub.mockImplementation(() => true);
+        fsStatStub.mockResolvedValue({ isDirectory: () => false } as any);
+        fsReadFileStub.mockResolvedValue("not-json" as any as Buffer);
+
+        await expect(getRepositorySelection()).rejects.toThrow(
+          "Invalid repository lists file. It should contain valid JSON.",
+        );
+      });
+
+      it("should fail if file contains array", async () => {
+        const fakeFilePath = "/path/to/file.json";
+        getRemoteRepositoryListsPathSpy.mockReturnValue(fakeFilePath);
+        pathExistsStub.mockImplementation(() => true);
+        fsStatStub.mockResolvedValue({ isDirectory: () => false } as any);
+        fsReadFileStub.mockResolvedValue("[]" as any as Buffer);
+
+        await expect(getRepositorySelection()).rejects.toThrow(
+          "Invalid repository lists file. It should be an object mapping names to a list of repositories.",
+        );
+      });
+
+      it("should fail if file does not contain repo lists in the right format", async () => {
+        const fakeFilePath = "/path/to/file.json";
+        getRemoteRepositoryListsPathSpy.mockReturnValue(fakeFilePath);
+        pathExistsStub.mockImplementation(() => true);
+        fsStatStub.mockResolvedValue({ isDirectory: () => false } as any);
+        const repoLists = {
+          list1: "owner1/repo1",
+        };
+        fsReadFileStub.mockResolvedValue(
+          JSON.stringify(repoLists) as any as Buffer,
+        );
+
+        await expect(getRepositorySelection()).rejects.toThrow(
+          "Invalid repository lists file. It should contain an array of repositories for each list.",
+        );
+      });
+
+      it("should get repo lists from file", async () => {
+        const fakeFilePath = "/path/to/file.json";
+        getRemoteRepositoryListsPathSpy.mockReturnValue(fakeFilePath);
+        pathExistsStub.mockImplementation(() => true);
+        fsStatStub.mockResolvedValue({ isDirectory: () => false } as any);
+        const repoLists = {
+          list1: ["owner1/repo1", "owner2/repo2"],
+          list2: ["owner3/repo3"],
+        };
+        fsReadFileStub.mockResolvedValue(
+          JSON.stringify(repoLists) as any as Buffer,
+        );
+        getRemoteRepositoryListsSpy.mockReturnValue({
+          list3: ["onwer4/repo4"],
+          list4: [],
+        });
+
+        quickPickSpy.mockResolvedValue({
+          repositories: ["owner3/repo3"],
+        } as unknown as QuickPickItem);
+
+        const repoSelection = await getRepositorySelection();
+
+        expect(repoSelection.repositoryLists).toBeUndefined();
+        expect(repoSelection.owners).toBeUndefined();
+        expect(repoSelection.repositories).toEqual(["owner3/repo3"]);
+      });
     });
   });
 });