Repository: oliverschwendener/ueli
Pull request #1291 (2024-12-09 16:10:30)

refactor(TerminalRegistry): moved operating system specific terminals to module bootstrap
In order to make testing easier this PR moves the operating system specific terminals to the bootstrap of the module.


Patch:
diff --git a/src/main/Core/Terminal/TerminalModule.ts b/src/main/Core/Terminal/TerminalModule.ts
index 3d402b55..9b05c501 100644
--- a/src/main/Core/Terminal/TerminalModule.ts
+++ b/src/main/Core/Terminal/TerminalModule.ts
@@ -1,6 +1,8 @@
 import type { Dependencies } from "@Core/Dependencies";
 import type { DependencyRegistry } from "@Core/DependencyRegistry";
+import type { OperatingSystem } from "@common/Core";
 import type { Terminal } from "@common/Core/Terminal";
+import type { Terminal as TerminalContract } from "./Contract";
 import { TerminalRegistry } from "./TerminalRegistry";
 import { CommandPrompt, Iterm, MacOsTerminal, Powershell, PowershellCore, Wsl } from "./Terminals";
 
@@ -9,7 +11,7 @@ export class TerminalModule {
         const ipcMain = dependencyRegistry.get("IpcMain");
         const assetPathResolver = dependencyRegistry.get("AssetPathResolver");
 
-        const terminalRegistry = new TerminalRegistry(dependencyRegistry.get("OperatingSystem"), {
+        const terminals: Record<OperatingSystem, () => TerminalContract[]> = {
             Linux: () => [],
             macOS: () => [
                 new MacOsTerminal(dependencyRegistry.get("AppleScriptUtility")),
@@ -21,7 +23,9 @@ export class TerminalModule {
                 new PowershellCore(dependencyRegistry.get("CommandlineUtility")),
                 new Wsl(dependencyRegistry.get("CommandlineUtility")),
             ],
-        });
+        };
+
+        const terminalRegistry = new TerminalRegistry(terminals[dependencyRegistry.get("OperatingSystem")]());
 
         dependencyRegistry.register("TerminalRegistry", terminalRegistry);
 
diff --git a/src/main/Core/Terminal/TerminalRegistry.ts b/src/main/Core/Terminal/TerminalRegistry.ts
index 94f8f16a..4d939578 100644
--- a/src/main/Core/Terminal/TerminalRegistry.ts
+++ b/src/main/Core/Terminal/TerminalRegistry.ts
@@ -1,18 +1,14 @@
-import type { OperatingSystem } from "@common/Core";
 import type { Terminal, TerminalRegistry as TerminalRegistryInterface } from "./Contract";
 
 export class TerminalRegistry implements TerminalRegistryInterface {
-    public constructor(
-        private readonly operatingSystem: OperatingSystem,
-        private readonly terminals: Record<OperatingSystem, () => Terminal[]>,
-    ) {}
+    public constructor(private readonly terminals: Terminal[]) {}
 
     public getAll(): Terminal[] {
-        return this.terminals[this.operatingSystem]();
+        return this.terminals;
     }
 
     public getById(terminalId: string): Terminal {
-        const terminal: Terminal | undefined = this.getAll().find((t) => t.terminalId === terminalId);
+        const terminal = this.getAll().find((t) => t.terminalId === terminalId);
 
         if (terminal) {
             return terminal;


Test patch:
diff --git a/src/main/Core/Terminal/TerminalRegistry.test.ts b/src/main/Core/Terminal/TerminalRegistry.test.ts
new file mode 100644
index 00000000..10006b52
--- /dev/null
+++ b/src/main/Core/Terminal/TerminalRegistry.test.ts
@@ -0,0 +1,25 @@
+import { describe, expect, it } from "vitest";
+import type { Terminal } from "./Contract";
+import { TerminalRegistry } from "./TerminalRegistry";
+
+describe(TerminalRegistry, () => {
+    describe(TerminalRegistry.prototype.getAll, () => {
+        it("should return all terminals ", () => {
+            const terminals = [<Terminal>{ terminalId: "terminal1" }, <Terminal>{ terminalId: "terminal2" }];
+            expect(new TerminalRegistry(terminals).getAll()).toBe(terminals);
+        });
+    });
+
+    describe(TerminalRegistry.prototype.getById, () => {
+        it("should throw an error if the terminal with the given id does not exist", () => {
+            expect(() => new TerminalRegistry([]).getById("terminal1")).toThrowError(
+                "Unable to find terminal with id terminal1",
+            );
+        });
+
+        it("should return the terminal if it's found", () => {
+            const terminals = [<Terminal>{ terminalId: "terminal1" }, <Terminal>{ terminalId: "terminal2" }];
+            expect(new TerminalRegistry(terminals).getById("terminal1")).toBe(terminals[0]);
+        });
+    });
+});