Re-factored and tested source-roots provider

src/configuration.test.ts

@@ -2,27 +2,27 @@ import { ConfigurationTarget, WorkspaceConfiguration } from "vscode";
 import { getPantsExecutable } from "./configuration";
 
 test("getPantsExecutable should default to `pants` without a valid config", () => {
-    expect(getPantsExecutable(config(""))).toEqual("pants");
-    expect(getPantsExecutable(config(undefined))).toEqual("pants");
+  expect(getPantsExecutable(config(""))).toEqual("pants");
+  expect(getPantsExecutable(config(undefined))).toEqual("pants");
 });
 
 test("getPantsExecutable should use the user-specified settings when available", () => {
-    expect(getPantsExecutable(config("./pants_from_sources"))).toEqual("./pants_from_sources");
+  expect(getPantsExecutable(config("./pants_from_sources"))).toEqual("./pants_from_sources");
 });
 
 function config(executable: string | undefined): WorkspaceConfiguration {
-    return {
-        get: (key: string) => {
-            return executable;
-        },
-        has: (key: string) => {
-            throw new Error("Method not implemented.");
-        },
-        inspect: (key: string) => {
-            throw new Error("Method not implemented.");
-        },
-        update: (key: string, value: any, target?: ConfigurationTarget | boolean) => {
-            throw new Error("Method not implemented.");
-        },
-    };
-}
+  return {
+    get: (key: string) => {
+      return executable;
+    },
+    has: (key: string) => {
+      throw new Error("Method not implemented.");
+    },
+    inspect: (key: string) => {
+      throw new Error("Method not implemented.");
+    },
+    update: (key: string, value: any, target?: ConfigurationTarget | boolean) => {
+      throw new Error("Method not implemented.");
+    },
+  };
+}

src/configuration.ts

@@ -1,18 +1,20 @@
 import { WorkspaceConfiguration, workspace } from "vscode";
 
-const namespace = "suspenders";
+export const namespace = "suspenders";
 
 /**
  * Returns the path to the Pants executable, as configured in the user's settings.
  * If no valid executable is configured (e.g. empty string), returns "pants".
- * 
+ *
  * @param config The user's WorkspaceConfiguration. Defaults to the global vscode configuration.
  * @returns The path to the Pants executable.
  */
-export function getPantsExecutable(config: WorkspaceConfiguration = workspace.getConfiguration(namespace)): string {
-    const pantsExecutable = config.get<string>("executable");
-    if (pantsExecutable) {
-        return pantsExecutable;
-    }
-    return "pants";
-}
+export function getPantsExecutable(
+  config: WorkspaceConfiguration = workspace.getConfiguration(namespace)
+): string {
+  const pantsExecutable = config.get<string>("executable");
+  if (pantsExecutable) {
+    return pantsExecutable;
+  }
+  return "pants";
+}

src/pants/runner.ts

@@ -9,7 +9,10 @@ export class Pants {
    *
    * @param buildRoot The root of the Pants build, which is the directory containing the pants.toml file.
    */
-  constructor(readonly buildRoot: string, readonly exe: string = getPantsExecutable()) { }
+  constructor(
+    readonly buildRoot: string,
+    readonly exe: string = getPantsExecutable()
+  ) {}
 
   /**
    * Run a Pants goal with any optional scoped options (e.g. global options or scoped goal options).
@@ -19,7 +22,7 @@ export class Pants {
    * @param scopedOptions A map of scoped options to run, which will be placed betwee the Pants executable and goals.
    * @returns
    */
-  async execute(goals: GoalArg[], target: string, scopedOptions?: Options): Promise<string> {
+  async execute(goals: GoalArg[], target?: string, scopedOptions?: Options): Promise<string> {
     // Ensure there is at least one goal
     if (goals.length === 0) {
       throw new Error("No goals specified");
@@ -33,7 +36,9 @@ export class Pants {
     for (const goal of goals) {
       args.push(...this.buildGoalArg(goal));
     }
-    args.push(target);
+    if (target) {
+      args.push(target);
+    }
 
     // Run the command and return the output
     const command = args.join(" ");

src/treeviews/source-roots-provider.test.ts

@@ -0,0 +1,59 @@
+import { Pants } from "../pants";
+import { SourceRoot, SourceRootsProvider, listSourceRoots } from "./source-roots-provider";
+import { execSync } from "child_process";
+
+vi.mock("child_process");
+
+test("listSourceRoots should return an empty list if Pants does not return any source roots", async () => {
+  const stdout = "";
+  vi.mocked(execSync).mockReturnValue(stdout);
+
+  const runner = new Pants("any/path/to/workspace");
+  const roots = await listSourceRoots(runner);
+  expect(roots).toEqual([]);
+});
+
+test("listSourceRoots should return a list of source roots if Pants returns source roots", async () => {
+  const stdout = "src/helloworld\nsrc/goodbyeworld\n";
+  vi.mocked(execSync).mockReturnValue(stdout);
+
+  const runner = new Pants("any/path/to/workspace");
+  const roots = await listSourceRoots(runner);
+  expect(roots).toEqual(["src/helloworld", "src/goodbyeworld"]);
+});
+
+test("refresh should fire the onDidChangeTreeData event", () => {
+  const provider = new SourceRootsProvider();
+  let eventFired = false;
+  provider.onDidChangeTreeData((e) => {
+    eventFired = true;
+  });
+  provider.refresh();
+  expect(eventFired).toBe(true);
+});
+
+test("getChildren should return an empty list if there is no rootPath", async () => {
+  const provider = new SourceRootsProvider();
+  expect(await provider.getChildren()).toEqual([]);
+
+  const provider1 = new SourceRootsProvider("");
+  expect(await provider1.getChildren()).toEqual([]);
+});
+
+test("getChildren should return an empty list if Pants does not return any source roots", async () => {
+  const stdout = "";
+  vi.mocked(execSync).mockReturnValue(stdout);
+
+  const provider = new SourceRootsProvider("any/path/to/workspace");
+  const children = await provider.getChildren();
+  expect(children).toEqual([]);
+});
+
+test("getChildren should return a list of source roots if Pants returns source roots", async () => {
+  const stdout = "src/helloworld\nsrc/goodbyeworld\n";
+  vi.mocked(execSync).mockReturnValue(stdout);
+
+  const provider = new SourceRootsProvider("any/path/to/workspace");
+  const children = await provider.getChildren();
+  expect(children).toEqual([new SourceRoot("src/helloworld"), new SourceRoot("src/goodbyeworld")]);
+});

src/treeviews/source-roots-provider.ts

@@ -1,38 +1,75 @@
-import * as vscode from "vscode";
-import * as proc from "child_process";
+import { EventEmitter, TreeDataProvider, TreeItem, TreeItemCollapsibleState } from "vscode";
+import { Pants } from "../pants";
 
-export class SourceRootsProvider implements vscode.TreeDataProvider<SourceRoot> {
-  private _onDidChangeTreeData: vscode.EventEmitter<SourceRoot | undefined | void> =
-    new vscode.EventEmitter<SourceRoot | undefined | void>();
-  readonly onDidChangeTreeData: vscode.Event<SourceRoot | undefined | void> =
-    this._onDidChangeTreeData.event;
+/**
+ * A TreeDataProvider that provides a list of source roots in the workspace.
+ * The source roots are discovered by running `pants roots` in the workspace root.
+ * For each source root, a SourceRoot tree item is created, and displayed in the tree view.
+ * At the moment, there is no nesting of source roots - they are all displayed at the root level, fully qualified.
+ *
+ * TODO: Add nesting of source roots.
+ */
+export class SourceRootsProvider implements TreeDataProvider<SourceRoot> {
+  private runner: Pants;
 
-  constructor(private rootPath: string | undefined) {}
+  private _onDidChangeTreeData = new EventEmitter<SourceRoot | undefined | void>();
+  readonly onDidChangeTreeData = this._onDidChangeTreeData.event;
 
+  /**
+   * The constructor for the SourceRootsProvider.
+   *
+   * @param rootPath The path to the workspace root.
+   * @param blah A dummy parameter to test the tree view.
+   */
+  constructor(private rootPath?: string) {
+    // TODO: Should we throw if there is no rootpath?
+    this.runner = new Pants(rootPath ?? "");
+  }
+
+  /**
+   * Refresh the tree view.
+   */
   refresh(): void {
     this._onDidChangeTreeData.fire();
   }
 
-  getTreeItem(element: SourceRoot): vscode.TreeItem | Thenable<vscode.TreeItem> {
+  getTreeItem(element: SourceRoot): TreeItem | Thenable<TreeItem> {
     return element;
   }
 
-  getChildren(element?: SourceRoot | undefined): vscode.ProviderResult<SourceRoot[]> {
+  /**
+   * Get the children of the given element, or the root elements if no element is provided.
+   *
+   * @param element The optional element to get the children of.
+   * @returns A list of {@link SourceRoot}.
+   */
+  async getChildren(element?: SourceRoot): Promise<SourceRoot[]> {
     if (!this.rootPath) {
-      vscode.window.showInformationMessage("No pants.toml in empty workspace");
-      return Promise.resolve([]);
+      return [];
     }
 
-    const executable = "pants";
-    const stdout = proc.execSync(`${executable} roots`, { cwd: this.rootPath }).toString().trim();
-    const roots = stdout.split("\n");
-    const sourceRoots = roots.map((r) => new SourceRoot(r));
-    return Promise.resolve(sourceRoots);
+    const roots = await listSourceRoots(this.runner);
+    return roots.map((r) => new SourceRoot(r));
   }
 }
 
-export class SourceRoot extends vscode.TreeItem {
+/**
+ * A tree item that represents a Pants source root.
+ * https://www.pantsbuild.org/docs/source-roots
+ */
+export class SourceRoot extends TreeItem {
   constructor(label: string) {
-    super(label, vscode.TreeItemCollapsibleState.None);
+    super(label, TreeItemCollapsibleState.None);
   }
 }
+
+/**
+ * Run the pants command which lists the source roots in the workspace.
+ *
+ * @param runner The Pants runner instance used to discover the source roots.
+ * @returns A list of strings representing the source roots in the workspace (e.g. ["src/helloworld", "src/goodbyeworld"]). If there are no source roots, an empty list is returned.
+ */
+export async function listSourceRoots(runner: Pants): Promise<string[]> {
+  const result = await runner.execute([{ goal: "roots" }], "");
+  return result === "" ? [] : result.split("\n");
+}

tests/setup-vitest.ts

@@ -1,13 +1,35 @@
+import { after, afterEach } from "node:test";
 import { vi } from "vitest";
 
+// TODO: Why isn't vi.importActual working? Throwing errors, but partial mocking is desired here
+
 vi.mock("vscode", () => {
-    return {
-        workspace: {
-            getConfiguration: () => {
-                return {
-                    get: () => "",
-                };
-            },
-        },
-    };
-});
+  return {
+    EventEmitter: class {
+      constructor(private listeners: Function[] = []) {}
+
+      fire() {
+        this.listeners.forEach((fn) => fn());
+      }
+
+      get event() {
+        return (fn: Function) => {
+          this.listeners.push(fn);
+        };
+      }
+    },
+    TreeItemCollapsibleState: {
+      None: 0,
+      Collapsed: 1,
+      Expanded: 2,
+    },
+    TreeItem: class {},
+    workspace: {
+      getConfiguration: () => {
+        return {
+          get: () => "",
+        };
+      },
+    },
+  };
+});

tsconfig.json

@@ -3,25 +3,16 @@
     "module": "commonjs",
     "target": "ES2022",
     "outDir": "dist",
-    "lib": [
-      "ES2022"
-    ],
+    "lib": ["ES2022"],
     "sourceMap": true,
     "rootDir": "src",
-    "strict": true, /* enable all strict type-checking options */
-    "types": [
-      "vitest/globals"
-    ]
+    "strict": true /* enable all strict type-checking options */,
+    "types": ["vitest/globals"]
     /* Additional Checks */
     // "noImplicitReturns": true, /* Report error when not all code paths in function return a value. */
     // "noFallthroughCasesInSwitch": true, /* Report errors for fallthrough cases in switch statement. */
     // "noUnusedParameters": true,  /* Report errors on unused parameters. */
   },
-  "include": [
-    "src/**/*.js",
-    "src/**/*.ts"
-  ],
-  "exclude": [
-    "node_modules/**"
-  ]
-}
+  "include": ["src/**/*.js", "src/**/*.ts"],
+  "exclude": ["node_modules/**"]
+}

vitest.config.ts

@@ -1,10 +1,10 @@
-
-import { defineConfig } from 'vitest/config'
+import { defineConfig } from "vitest/config";
 
 export default defineConfig({
-    test: {
-        globals: true,
-        include: ["src/**/*.{test,spec}.{js,ts}"],
-        setupFiles: "./tests/setup-vitest.ts",
-    },
-})
+  test: {
+    globals: true,
+    include: ["src/**/*.{test,spec}.{js,ts}"],
+    mockReset: true,
+    setupFiles: "./tests/setup-vitest.ts",
+  },
+});