fix: make all unit tests cross-platform for Windows (#39)

SebTardif · SebTardif · commit 2d8227986c25 · 2026-06-04T12:59:35.000-07:00
Replace hardcoded forward-slash paths with path.join() in test assertions so path separators match on all platforms. Use process.platform for real-filesystem PATH discovery tests instead of hardcoding 'linux'. Skip the permission-based test on Windows where Unix file mode is not enforced. Fixes #39 Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
diff --git a/test/unit/binary.test.ts b/test/unit/binary.test.ts
@@ -219,7 +219,7 @@ test("resolveManagedInstallPaths uses cargo-dist style archive names", () => {
 
   assert.equal(paths.archiveFileName, "patchloom-aarch64-apple-darwin.tar.xz");
   assert.equal(paths.checksumFileName, "patchloom-aarch64-apple-darwin.tar.xz.sha256");
-  assert.match(paths.binaryPath, /managed-bin\/patchloom$/);
+  assert.equal(paths.binaryPath, path.join("/managed/install", "0.1.0", "managed-bin", "patchloom"));
 });
 
 test("buildManagedInstallReleaseAssets builds archive and checksum urls", () => {
@@ -327,11 +327,11 @@ test("resolveManagedInstallTransactionPaths keeps staged files separate from the
   assert.ok(target);
   const paths = resolveManagedInstallTransactionPaths("/managed/install", "0.1.0", target);
 
-  assert.equal(paths.archivePath, "/managed/install/0.1.0/patchloom-aarch64-apple-darwin.tar.xz");
-  assert.equal(paths.stagedArchivePath, "/managed/install/0.1.0/.staging/patchloom-aarch64-apple-darwin.tar.xz");
-  assert.equal(paths.stagedChecksumPath, "/managed/install/0.1.0/.staging/patchloom-aarch64-apple-darwin.tar.xz.sha256");
-  assert.equal(paths.stagedBinaryPath, "/managed/install/0.1.0/.staging/managed-bin/patchloom");
-  assert.equal(paths.backupBinaryPath, "/managed/install/0.1.0/managed-bin/patchloom.bak");
+  assert.equal(paths.archivePath, path.join("/managed/install", "0.1.0", "patchloom-aarch64-apple-darwin.tar.xz"));
+  assert.equal(paths.stagedArchivePath, path.join("/managed/install", "0.1.0", ".staging", "patchloom-aarch64-apple-darwin.tar.xz"));
+  assert.equal(paths.stagedChecksumPath, path.join("/managed/install", "0.1.0", ".staging", "patchloom-aarch64-apple-darwin.tar.xz.sha256"));
+  assert.equal(paths.stagedBinaryPath, path.join("/managed/install", "0.1.0", ".staging", "managed-bin", "patchloom"));
+  assert.equal(paths.backupBinaryPath, `${path.join("/managed/install", "0.1.0", "managed-bin", "patchloom")}.bak`);
 });
 
 test("inspectManagedInstallStatus includes the last managed install failure for diagnostics", async () => {
@@ -353,7 +353,7 @@ test("inspectManagedInstallStatus includes the last managed install failure for
 
     assert.deepEqual(status, {
       exists: false,
-      binaryPath: "/managed/install/0.1.0/managed-bin/patchloom",
+      binaryPath: path.join("/managed/install", "0.1.0", "managed-bin", "patchloom"),
       version: "0.1.0",
       target,
       failure: {
@@ -381,7 +381,7 @@ test("clearManagedInstallStaging removes the entire staging directory", async ()
   });
 
   assert.deepEqual(operations, [
-    "rmdir /managed/install/0.1.0/.staging"
+    `rmdir ${paths.stagingRoot}`
   ]);
 });
 
@@ -420,12 +420,12 @@ test("promoteManagedInstallBinary replaces the live binary and clears stale back
   });
 
   assert.deepEqual(operations, [
-    "mkdir /managed/install/0.1.0/managed-bin",
-    "remove /managed/install/0.1.0/managed-bin/patchloom.bak",
-    "rename /managed/install/0.1.0/managed-bin/patchloom -> /managed/install/0.1.0/managed-bin/patchloom.bak",
-    "rename /managed/install/0.1.0/.staging/managed-bin/patchloom -> /managed/install/0.1.0/managed-bin/patchloom",
-    "remove /managed/install/0.1.0/managed-bin/patchloom.bak",
-    `remove /managed/storage/${PATCHLOOM_MANAGED_INSTALL_FAILURE_FILE}`
+    `mkdir ${path.dirname(paths.binaryPath)}`,
+    `remove ${paths.backupBinaryPath}`,
+    `rename ${paths.binaryPath} -> ${paths.backupBinaryPath}`,
+    `rename ${paths.stagedBinaryPath} -> ${paths.binaryPath}`,
+    `remove ${paths.backupBinaryPath}`,
+    `remove ${path.join("/managed/storage", PATCHLOOM_MANAGED_INSTALL_FAILURE_FILE)}`
   ]);
 
   const status = await inspectManagedInstallStatus({
@@ -481,13 +481,14 @@ test("promoteManagedInstallBinary restores the previous binary when replacement
   );
 
   try {
+    const failurePath = path.join("/managed/storage", PATCHLOOM_MANAGED_INSTALL_FAILURE_FILE);
     assert.deepEqual(operations, [
-      "mkdir /managed/install/0.1.0/managed-bin",
-      "rename /managed/install/0.1.0/managed-bin/patchloom -> /managed/install/0.1.0/managed-bin/patchloom.bak",
-      "rename /managed/install/0.1.0/.staging/managed-bin/patchloom -> /managed/install/0.1.0/managed-bin/patchloom",
-      "rename /managed/install/0.1.0/managed-bin/patchloom.bak -> /managed/install/0.1.0/managed-bin/patchloom",
-      "mkdir /managed/storage",
-      `write /managed/storage/${PATCHLOOM_MANAGED_INSTALL_FAILURE_FILE} => {\n  \"stage\": \"replace\",\n  \"reason\": \"replace-failed\",\n  \"message\": \"Failed to replace managed Patchloom binary (simulated rename failure).\"\n}`
+      `mkdir ${path.dirname(paths.binaryPath)}`,
+      `rename ${paths.binaryPath} -> ${paths.backupBinaryPath}`,
+      `rename ${paths.stagedBinaryPath} -> ${paths.binaryPath}`,
+      `rename ${paths.backupBinaryPath} -> ${paths.binaryPath}`,
+      `mkdir ${path.dirname(failurePath)}`,
+      `write ${failurePath} => {\n  \"stage\": \"replace\",\n  \"reason\": \"replace-failed\",\n  \"message\": \"Failed to replace managed Patchloom binary (simulated rename failure).\"\n}`
     ]);
 
     const status = await inspectManagedInstallStatus({
@@ -510,16 +511,17 @@ test("promoteManagedInstallBinary restores the previous binary when replacement
 test("inspectManagedInstallStatus reports discovered managed binaries", async () => {
   const target = detectManagedInstallTarget("darwin", "arm64");
   assert.ok(target);
+  const expectedBinaryPath = path.join("/managed/install", "0.1.0", "managed-bin", "patchloom");
   const status = await inspectManagedInstallStatus({
     installRoot: "/managed/install",
     version: "v0.1.0",
     target,
-    fileExists: async (filePath) => filePath.endsWith("/0.1.0/managed-bin/patchloom")
+    fileExists: async (filePath) => filePath === expectedBinaryPath
   });
 
   assert.deepEqual(status, {
     exists: true,
-    binaryPath: "/managed/install/0.1.0/managed-bin/patchloom",
+    binaryPath: expectedBinaryPath,
     version: "0.1.0",
     target
   });
@@ -531,7 +533,7 @@ test("loadManagedInstallFailure reads persisted failure diagnostics from storage
   const failure = await loadManagedInstallFailure({
     storageRoot: "/managed/storage",
     readFile: async (filePath) => {
-      assert.equal(filePath, `/managed/storage/${PATCHLOOM_MANAGED_INSTALL_FAILURE_FILE}`);
+      assert.equal(filePath, path.join("/managed/storage", PATCHLOOM_MANAGED_INSTALL_FAILURE_FILE));
       return JSON.stringify({
         stage: "verify",
         reason: "checksum-mismatch",
@@ -572,10 +574,11 @@ test("persistManagedInstallFailure and clearManagedInstallFailureRecord update t
     }
   });
 
+  const failurePath = path.join("/managed/storage", PATCHLOOM_MANAGED_INSTALL_FAILURE_FILE);
   assert.deepEqual(writes, [
-    "mkdir /managed/storage",
-    `write /managed/storage/${PATCHLOOM_MANAGED_INSTALL_FAILURE_FILE} => {\n  \"stage\": \"extract\",\n  \"reason\": \"extract-failed\",\n  \"message\": \"Archive extraction failed.\"\n}`,
-    `remove /managed/storage/${PATCHLOOM_MANAGED_INSTALL_FAILURE_FILE}`
+    `mkdir ${path.dirname(failurePath)}`,
+    `write ${failurePath} => {\n  \"stage\": \"extract\",\n  \"reason\": \"extract-failed\",\n  \"message\": \"Archive extraction failed.\"\n}`,
+    `remove ${failurePath}`
   ]);
   assert.equal(await loadManagedInstallFailure({}), undefined);
 });
@@ -630,21 +633,22 @@ test("normalizeReleaseVersion removes a leading v", () => {
 });
 
 test("resolvePatchloomStatusWithInputs falls back to a managed install when present", async () => {
+  const expectedBinaryPath = path.join("/managed/install", "0.1.0", "managed-bin", "patchloom");
   const status = await resolvePatchloomStatusWithInputs({
     configuredPath: "",
     pathValue: "/usr/local/bin:/bin",
     platform: "darwin",
     arch: "arm64",
     managedInstallRoot: "/managed/install",
     managedInstallVersion: "0.1.0",
-    managedFileExists: async (filePath) => filePath.endsWith("/0.1.0/managed-bin/patchloom"),
-    canExecute: async (candidate) => candidate === "/managed/install/0.1.0/managed-bin/patchloom",
+    managedFileExists: async (filePath) => filePath === expectedBinaryPath,
+    canExecute: async (candidate) => candidate === expectedBinaryPath,
     getVersion: async () => "patchloom 0.1.0"
   });
 
   assert.equal(status.ready, true);
   assert.equal(status.source, "managed");
-  assert.equal(status.binaryPath, "/managed/install/0.1.0/managed-bin/patchloom");
+  assert.equal(status.binaryPath, expectedBinaryPath);
   assert.equal(status.managedInstall?.exists, true);
 });
 
diff --git a/test/unit/binaryDiscovery.test.ts b/test/unit/binaryDiscovery.test.ts
@@ -25,17 +25,17 @@ test("findOnPath discovers a real executable in a temp directory", async () => {
     const fakeBinary = path.join(dir, "patchloom");
     await fs.writeFile(fakeBinary, "#!/bin/sh\necho patchloom 0.1.0\n", { mode: 0o755 });
 
-    const found = await findOnPath(dir, "linux");
+    const found = await findOnPath(dir, process.platform);
     assert.equal(found, fakeBinary);
   });
 });
 
-test("findOnPath skips non-executable files", async () => {
+test("findOnPath skips non-executable files", { skip: process.platform === "win32" ? "Windows does not enforce Unix file permissions" : undefined }, async () => {
   await withTempDir(async (dir) => {
     const fakeBinary = path.join(dir, "patchloom");
     await fs.writeFile(fakeBinary, "#!/bin/sh\necho patchloom 0.1.0\n", { mode: 0o644 });
 
-    const found = await findOnPath(dir, "linux");
+    const found = await findOnPath(dir, process.platform);
     assert.equal(found, undefined);
   });
 });
@@ -50,7 +50,8 @@ test("findOnPath searches multiple PATH directories in order", async () => {
     await fs.writeFile(path.join(firstDir, "patchloom"), "#!/bin/sh\necho first\n", { mode: 0o755 });
     await fs.writeFile(path.join(secondDir, "patchloom"), "#!/bin/sh\necho second\n", { mode: 0o755 });
 
-    const found = await findOnPath(`${firstDir}:${secondDir}`, "linux");
+    const pathSep = process.platform === "win32" ? ";" : ":";
+    const found = await findOnPath(`${firstDir}${pathSep}${secondDir}`, process.platform);
     assert.equal(found, path.join(firstDir, "patchloom"), "should find the first match in PATH order");
   });
 });
@@ -66,7 +67,8 @@ test("findOnPath deduplicates PATH entries", async () => {
     await fs.writeFile(fakeBinary, "#!/bin/sh\necho v1\n", { mode: 0o755 });
     let checkCount = 0;
 
-    const found = await findOnPath(`${dir}:${dir}:${dir}`, "linux", async (candidate) => {
+    const pathSep = process.platform === "win32" ? ";" : ":";
+    const found = await findOnPath(`${dir}${pathSep}${dir}${pathSep}${dir}`, process.platform, async (candidate) => {
       checkCount++;
       try {
         await fs.access(candidate, fs.constants.X_OK);
diff --git a/test/unit/initializeProject.test.ts b/test/unit/initializeProject.test.ts
@@ -1,4 +1,5 @@
 import assert from "node:assert/strict";
+import * as path from "node:path";
 import test from "node:test";
 import { MINIMUM_SUPPORTED_PATCHLOOM_VERSION } from "../../src/binary/patchloom.js";
 import { classifyAgentsFile, generateAgentRules } from "../../src/commands/initializeProject.js";
@@ -290,7 +291,7 @@ test("inspectMcpTargets reports configured targets", async () => {
     workspaceFolderPath: "/workspace/demo",
     homeDir: "/Users/demo",
     readFile: async (filePath) => {
-      if (filePath.endsWith(".vscode/mcp.json")) {
+      if (filePath.endsWith(path.join(".vscode", "mcp.json"))) {
         return '{"servers":{"patchloom":{"command":"patchloom","args":["mcp-server"]}}}';
       }
       return undefined;
@@ -317,7 +318,7 @@ test("configureMcpTargets creates or updates only the selected target kinds", as
     includeKinds: ["cursor-workspace"],
     patchloomPathSetting: "/custom/patchloom",
     readFile: async (filePath) => {
-      if (filePath.endsWith(".cursor/mcp.json")) {
+      if (filePath.endsWith(path.join(".cursor", "mcp.json"))) {
         return '{"servers":{"other":{"command":"other"}}}';
       }
       return undefined;
@@ -328,9 +329,9 @@ test("configureMcpTargets creates or updates only the selected target kinds", as
   });
 
   assert.equal(results.length, 1);
-  const cursorPath = "/workspace/demo/.cursor/mcp.json";
+  const cursorPath = path.join("/workspace/demo", ".cursor", "mcp.json");
   assert.equal(writes.has(cursorPath), true);
-  assert.equal(writes.has("/workspace/demo/.vscode/mcp.json"), false);
+  assert.equal(writes.has(path.join("/workspace/demo", ".vscode", "mcp.json")), false);
   assert.match(writes.get(cursorPath) ?? "", /patchloom/);
   assert.match(writes.get(cursorPath) ?? "", /mcp-server/);
   assert.match(writes.get(cursorPath) ?? "", /other/);
