From a63a8944a26e1e488dd363e526fe49843f02e8c6 Mon Sep 17 00:00:00 2001 From: thatben Date: Mon, 21 Apr 2025 13:44:16 +0200 Subject: [PATCH] Fixes process detection for defunct processes in unix environment --- README.md | 7 ++++ src/handlers/processControl.js | 9 +++-- src/handlers/processControl.test.js | 61 +++++++++++++++++++++++------ src/main.js | 1 + src/services/fsService.js | 5 +-- src/services/osService.js | 2 +- src/ui/configMenu.test.js | 5 +-- src/ui/installMenu.js | 15 ++++++- src/ui/installMenu.test.js | 31 +++++++++++++-- 9 files changed, 107 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index b9e2886..f2204d8 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,13 @@ npm install npm start ``` +## Trouble? + +Is the installer crashing? Make sure these packages are installed: +``` +apt-get install fdisk procps +``` + ## License MIT diff --git a/src/handlers/processControl.js b/src/handlers/processControl.js index 583789b..c6b7fbd 100644 --- a/src/handlers/processControl.js +++ b/src/handlers/processControl.js @@ -12,8 +12,9 @@ export class ProcessControl { if (this.os.isWindows()) { return processes.filter((p) => p.name === "codex.exe"); } else { - return processes.filter((p) => - p.name === "codex" && !p.cmd.includes("")); + return processes.filter( + (p) => p.name === "codex" && !p.cmd.includes(""), + ); } }; @@ -37,14 +38,14 @@ export class ProcessControl { this.os.terminateProcess(pid); await this.sleep(); } - } + }; isProcessRunning = async (pid) => { const processes = await this.os.listProcesses(); const p = processes.filter((p) => p.pid == pid); const result = p.length > 0; return result; - } + }; startCodexProcess = async () => { await this.saveCodexConfigFile(); diff --git a/src/handlers/processControl.test.js b/src/handlers/processControl.test.js index 70a6963..705884a 100644 --- a/src/handlers/processControl.test.js +++ b/src/handlers/processControl.test.js @@ -27,12 +27,12 @@ describe("ProcessControl", () => { describe("getCodexProcesses", () => { const processes = [ - { id: 0, name: "a.exe" }, - { id: 1, name: "aaa" }, - { id: 2, name: "codex" }, - { id: 3, name: "codex.exe" }, - { id: 4, name: "notcodex" }, - { id: 5, name: "alsonotcodex.exe" }, + { id: 0, name: "a.exe", cmd: "" }, + { id: 1, name: "codex", cmd: "" }, + { id: 2, name: "codex", cmd: "" }, + { id: 3, name: "codex.exe", cmd: "" }, + { id: 4, name: "notcodex", cmd: "" }, + { id: 5, name: "alsonotcodex.exe", cmd: "" }, ]; beforeEach(() => { @@ -80,7 +80,7 @@ describe("ProcessControl", () => { ); }); - it("stops the first codex process", async () => { + it("calls stopProcess with pid of first codex process", async () => { const pid = 12345; processControl.getCodexProcesses.mockResolvedValue([ { pid: pid }, @@ -88,21 +88,58 @@ describe("ProcessControl", () => { { pid: 222 }, ]); + processControl.stopProcess = vi.fn(); await processControl.stopCodexProcess(); + expect(processControl.stopProcess).toHaveBeenCalledWith(pid); + }); + }); + + describe("stopProcess", () => { + const pid = 234; + beforeEach(() => { + processControl.isProcessRunning = vi.fn(); + }); + + it("stops the process", async () => { + processControl.isProcessRunning.mockResolvedValue(false); + + await processControl.stopProcess(pid); + expect(mockOsService.stopProcess).toHaveBeenCalledWith(pid); }); it("sleeps", async () => { - processControl.getCodexProcesses.mockResolvedValue([ - { pid: 111 }, - { pid: 222 }, - ]); + processControl.isProcessRunning.mockResolvedValue(false); - await processControl.stopCodexProcess(); + await processControl.stopProcess(pid); expect(processControl.sleep).toHaveBeenCalled(); }); + + it("terminates process if it is running after stop", async () => { + processControl.isProcessRunning.mockResolvedValue(true); + + await processControl.stopProcess(pid); + + expect(mockOsService.terminateProcess).toHaveBeenCalledWith(pid); + }); + }); + + describe("isProcessRunning", () => { + const pid = 345; + + it("is true when process is in process list", async () => { + mockOsService.listProcesses.mockResolvedValue([{ pid: pid }]); + + expect(await processControl.isProcessRunning(pid)).toBeTruthy(); + }); + + it("is false when process is not in process list", async () => { + mockOsService.listProcesses.mockResolvedValue([{ pid: pid + 11 }]); + + expect(await processControl.isProcessRunning(pid)).toBeFalsy(); + }); }); describe("startCodexProcess", () => { diff --git a/src/main.js b/src/main.js index 104b0d5..9adb505 100644 --- a/src/main.js +++ b/src/main.js @@ -116,6 +116,7 @@ export async function main() { ); const installMenu = new InstallMenu( uiService, + new MenuLoop(), configService, pathSelector, installer, diff --git a/src/services/fsService.js b/src/services/fsService.js index 879f621..8c91f33 100644 --- a/src/services/fsService.js +++ b/src/services/fsService.js @@ -15,8 +15,7 @@ export class FsService { if (!fs.lstatSync(mount).isFile()) { mountPoints.push(mount); } - } catch { - } + } catch {} } }); }); @@ -24,7 +23,7 @@ export class FsService { if (mountPoints.length < 1) { // In certain containerized environments, the devices don't reveal any // useful mounts. We'll proceed under the assumption that '/' is valid here. - return ['/']; + return ["/"]; } return mountPoints; }; diff --git a/src/services/osService.js b/src/services/osService.js index a42f384..f3b4449 100644 --- a/src/services/osService.js +++ b/src/services/osService.js @@ -32,5 +32,5 @@ export class OsService { terminateProcess = (pid) => { process.kill(pid, "SIGTERM"); - } + }; } diff --git a/src/ui/configMenu.test.js b/src/ui/configMenu.test.js index 59d972d..12bf207 100644 --- a/src/ui/configMenu.test.js +++ b/src/ui/configMenu.test.js @@ -2,10 +2,7 @@ import { describe, beforeEach, it, expect, vi } from "vitest"; import { ConfigMenu } from "./configMenu.js"; import { mockUiService } from "../__mocks__/service.mocks.js"; import { mockConfigService } from "../__mocks__/service.mocks.js"; -import { - mockNumberSelector, - mockMenuLoop, -} from "../__mocks__/utils.mocks.js"; +import { mockNumberSelector, mockMenuLoop } from "../__mocks__/utils.mocks.js"; describe("ConfigMenu", () => { const config = { diff --git a/src/ui/installMenu.js b/src/ui/installMenu.js index f87069d..5b99fee 100644 --- a/src/ui/installMenu.js +++ b/src/ui/installMenu.js @@ -1,13 +1,20 @@ export class InstallMenu { - constructor(uiService, configService, pathSelector, installer) { + constructor(uiService, menuLoop, configService, pathSelector, installer) { this.ui = uiService; + this.loop = menuLoop; this.configService = configService; this.config = configService.get(); this.pathSelector = pathSelector; this.installer = installer; + + this.loop.initialize(this.showMenu); } show = async () => { + await this.loop.showLoop(); + }; + + showMenu = async () => { if (await this.installer.isCodexInstalled()) { await this.showUninstallMenu(); } else { @@ -86,14 +93,18 @@ export class InstallMenu { }; performInstall = async () => { + this.loop.stopLoop(); await this.installer.installCodex(this); }; performUninstall = async () => { + this.loop.stopLoop(); this.installer.uninstallCodex(); }; - doNothing = async () => {}; + doNothing = async () => { + this.loop.stopLoop(); + }; // Progress callbacks from installer module: installStarts = () => { diff --git a/src/ui/installMenu.test.js b/src/ui/installMenu.test.js index 6dd281f..620edcb 100644 --- a/src/ui/installMenu.test.js +++ b/src/ui/installMenu.test.js @@ -2,7 +2,7 @@ import { describe, beforeEach, it, expect, vi } from "vitest"; import { InstallMenu } from "./installMenu.js"; import { mockUiService } from "../__mocks__/service.mocks.js"; import { mockConfigService } from "../__mocks__/service.mocks.js"; -import { mockPathSelector } from "../__mocks__/utils.mocks.js"; +import { mockMenuLoop, mockPathSelector } from "../__mocks__/utils.mocks.js"; import { mockInstaller } from "../__mocks__/handler.mocks.js"; describe("InstallMenu", () => { @@ -17,13 +17,30 @@ describe("InstallMenu", () => { installMenu = new InstallMenu( mockUiService, + mockMenuLoop, mockConfigService, mockPathSelector, mockInstaller, ); }); + describe("constructor", () => { + it("initializes the menu loop with the showMenu function", () => { + expect(mockMenuLoop.initialize).toHaveBeenCalledWith( + installMenu.showMenu, + ); + }); + }); + describe("show", () => { + it("starts the menu loop", async () => { + await installMenu.show(); + + expect(mockMenuLoop.showLoop).toHaveBeenCalled(); + }); + }); + + describe("showMenu", () => { beforeEach(() => { installMenu.showInstallMenu = vi.fn(); installMenu.showUninstallMenu = vi.fn(); @@ -32,7 +49,7 @@ describe("InstallMenu", () => { it("shows uninstall menu when codex is installed", async () => { mockInstaller.isCodexInstalled.mockResolvedValue(true); - await installMenu.show(); + await installMenu.showMenu(); expect(installMenu.showUninstallMenu).toHaveBeenCalled(); }); @@ -40,7 +57,7 @@ describe("InstallMenu", () => { it("shows install menu when codex is not installed", async () => { mockInstaller.uninstallCodex.mockResolvedValue(false); - await installMenu.show(); + await installMenu.showMenu(); expect(installMenu.showInstallMenu).toHaveBeenCalled(); }); @@ -140,12 +157,20 @@ describe("InstallMenu", () => { await installMenu.performInstall(); expect(mockInstaller.installCodex).toHaveBeenCalledWith(installMenu); + expect(mockMenuLoop.stopLoop).toHaveBeenCalled(); }); it("calls installer for deinstallation", async () => { await installMenu.performUninstall(); expect(mockInstaller.uninstallCodex).toHaveBeenCalled(); + expect(mockMenuLoop.stopLoop).toHaveBeenCalled(); + }); + + it("stops the menu loop when nothing is selected", async () => { + await installMenu.doNothing(); + + expect(mockMenuLoop.stopLoop).toHaveBeenCalled(); }); describe("process callback handling", () => {