diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..efbcb6ce --- /dev/null +++ b/.gitattributes @@ -0,0 +1,25 @@ +# Every text file is stored and checked out with LF, on Windows too. +# +# Three tests read a source file as text and assert on its contents +# (tests/ipc/accountLoginFlow.test.ts, tests/security-boundaries.test.ts, +# tests/renderer-dom/moddbVisibilityPrompt.test.tsx). Under a CRLF checkout the +# first of those fails, which is how the Windows CI job found this; the other +# two pass only because neither happens to assert across a line ending. +* text=auto eol=lf + +# Byte-exact fixtures. These are crafted archives and compressed streams, and +# the tests assert on declared sizes, offsets and digests inside them, so a +# single rewritten line ending changes the answer. tests/fixtures/not-a-zip.bin +# is the one that actually needs saying: it holds no NUL byte, so the rule above +# would classify it as text on its own. +*.zip binary +*.bin binary +*.lzma1 binary +*.lzma2 binary + +# Assets. Git detects these as binary by itself; the markers keep that true if +# one is ever regenerated in a format that looks textual. +*.png binary +*.jpg binary +*.ico binary +*.icns binary diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fbf08f60..acfd8dd1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,8 +32,12 @@ jobs: - run: npm run lint:ci - run: npm run format:check - test: - runs-on: ubuntu-latest + test-matrix: + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 @@ -43,6 +47,22 @@ jobs: - run: npm ci - run: npm run test:coverage + # `dev` branch protection requires a context literally named `test`, and a + # matrixed job can't produce one: it reports `test (ubuntu-latest)` and + # `test (windows-latest)` instead. This job keeps the required name alive. + # `always()` makes it run even when a leg fails (without it the gate would be + # skipped, and protection counts a skipped required job as satisfied), and + # the explicit comparison fails the gate unless every leg succeeded, so a + # failed, cancelled or skipped matrix turns it red. + test: + needs: [test-matrix] + if: always() + runs-on: ubuntu-latest + steps: + - run: | + echo "test-matrix result: ${{ needs.test-matrix.result }}" + [ "${{ needs.test-matrix.result }}" = "success" ] + # Informational quality scan: never blocks a merge, so a SonarCloud outage # or configuration change can't turn the whole run red. sonarcloud: diff --git a/src/ipc/handlers/gameHandlers.ts b/src/ipc/handlers/gameHandlers.ts index af58e508..0541c67d 100644 --- a/src/ipc/handlers/gameHandlers.ts +++ b/src/ipc/handlers/gameHandlers.ts @@ -1,5 +1,6 @@ import { ipcMain } from "electron" import { spawn } from "node:child_process" +import type { ChildProcessWithoutNullStreams } from "node:child_process" import fse from "fs-extra" import { join } from "node:path" import os from "node:os" @@ -123,7 +124,21 @@ function realGameProcess(): GameProcess { resolve(outcome) } - const externalApp = spawn(request.command, request.args, { env: { ...request.env }, cwd: request.cwd, shell: false, windowsHide: true }) + let externalApp: ChildProcessWithoutNullStreams + try { + externalApp = spawn(request.command, request.args, { env: { ...request.env }, cwd: request.cwd, shell: false, windowsHide: true }) + } catch (err) { + // spawn reports ENOENT, EACCES, EAGAIN, EMFILE and ENFILE through an "error" + // event and THROWS everything else, which is not a distinction the caller can + // do anything with. Windows answers a file that is not a real executable with + // UNKNOWN, so a truncated or quarantined Vintagestory.exe lands here rather + // than on the event below; without this the whole handler rejects and the + // launch stops being a reason the player is told. + logMessage("error", `[back] [ipc] [ipc/handlers/gameHandlers.ts] [EXECUTE_GAME] Error running Vintage Story.`) + logMessage("verbose", `[back] [ipc] [ipc/handlers/gameHandlers.ts] [EXECUTE_GAME] ${getErrorMessage(err)}`) + settle({ started: false, error: getErrorMessage(err) }) + return + } externalApp.stdout.resume() @@ -330,6 +345,10 @@ function realProcessProbe(): ProcessProbe { let stdout = "" let settled = false + // Declared before settle so that every exit from this executor, the spawn + // throw included, goes through settle. clearTimeout ignores undefined, so + // settling before the timer exists is safe. + let timer: ReturnType | undefined = undefined const settle = (outcome: ProcessProbeOutcome): void => { if (settled) return @@ -338,9 +357,19 @@ function realProcessProbe(): ProcessProbe { resolve(outcome) } - const externalApp = spawn(request.command, request.args, { shell: false, windowsHide: true }) + let externalApp: ChildProcessWithoutNullStreams + try { + externalApp = spawn(request.command, request.args, { shell: false, windowsHide: true }) + } catch (err) { + // Same throw-instead-of-emit split as EXECUTE_GAME's spawn above, settled + // the way the "error" event below settles it. + logMessage("error", `[back] [ipc] [gameHandlers.ts] [LOOK_FOR_A_GAME_VERSION] Error looking for the Vintage Story version.`) + logMessage("verbose", `[back] [ipc] [gameHandlers.ts] [LOOK_FOR_A_GAME_VERSION] ${getErrorMessage(err)}`) + settle({ ok: false, stdout, error: getErrorMessage(err) }) + return + } - const timer = setTimeout(() => { + timer = setTimeout(() => { logMessage("error", `[back] [ipc] [gameHandlers.ts] [LOOK_FOR_A_GAME_VERSION] Timed out waiting for Vintage Story to report its version.`) externalApp.kill() settle({ ok: false, stdout, error: "Timed out waiting for a response." }) diff --git a/tests/ipc/accountLoginFlow.test.ts b/tests/ipc/accountLoginFlow.test.ts index 9227952f..f56fbf6a 100644 --- a/tests/ipc/accountLoginFlow.test.ts +++ b/tests/ipc/accountLoginFlow.test.ts @@ -18,7 +18,10 @@ import { describe, it } from "vitest" * union with no member asking for another request, so the domain cannot lead the * handler into a third pass either. */ -const HANDLER_SOURCE = readFileSync(resolve(__dirname, "../../src/ipc/handlers/accountHandlers.ts"), "utf8") +// Normalized to LF: on Windows, git checks this file out with CRLF line +// endings, and every "\n"-based search below (and in the case-slicing test) +// assumes LF. +const HANDLER_SOURCE = readFileSync(resolve(__dirname, "../../src/ipc/handlers/accountHandlers.ts"), "utf8").replace(/\r\n/g, "\n") function countOccurrences(haystack: string, needle: string): number { return haystack.split(needle).length - 1 diff --git a/tests/ipc/accountStore.test.ts b/tests/ipc/accountStore.test.ts index e2e24d65..6e921804 100644 --- a/tests/ipc/accountStore.test.ts +++ b/tests/ipc/accountStore.test.ts @@ -134,7 +134,9 @@ describe("saveAccountSecrets", () => { assert.equal(JSON.parse(raw).version, 2) }) - it("keeps the file readable only by its owner", async () => { + // chmod on Windows only toggles the read-only attribute; it cannot produce + // the POSIX 0o600 this reads back off disk. + it.skipIf(process.platform === "win32")("keeps the file readable only by its owner", async () => { const store = await loadStore() await store.saveAccountSecrets("uid-a", ACCOUNT_A) diff --git a/tests/ipc/configHandlers.test.ts b/tests/ipc/configHandlers.test.ts index 7d74031d..91e00106 100644 --- a/tests/ipc/configHandlers.test.ts +++ b/tests/ipc/configHandlers.test.ts @@ -157,7 +157,9 @@ describe("SAVE_CONFIG", () => { assert.equal(reread.defaultInstallationsFolder, join(appDataFolder, "RiftLauncherInstallations")) }) - it("reports write-failed when the config file cannot be written", async () => { + // chmod 0o500 does not stop a write on Windows: NTFS enforces read-only + // through the file attribute, not POSIX write bits on the containing folder. + it.skipIf(process.platform === "win32")("reports write-failed when the config file cannot be written", async () => { const event = await createTrustedEvent() // Make the userData folder itself read-only so the temp-file write inside diff --git a/tests/ipc/extraction.test.ts b/tests/ipc/extraction.test.ts index 0d6489ff..981bf8b8 100644 --- a/tests/ipc/extraction.test.ts +++ b/tests/ipc/extraction.test.ts @@ -124,9 +124,13 @@ describe("runExtraction on a gzipped tar", () => { await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, sevenZipBin }) + // The listing is what says the wrapping "vintagestory" folder was stepped + // into rather than copied along: it names every entry, so an extra folder + // could not hide in it. Asking the filesystem whether "vintagestory" + // exists cannot say that on Windows, where it resolves to the + // "Vintagestory" file next to it. assert.deepEqual(readdirSync(workspacePath("target")).sort(), ["Vintagestory", "assets"]) assert.equal(readFileSync(workspacePath("target", "Vintagestory"), "utf8"), "elf") - assert.equal(existsSync(workspacePath("target", "vintagestory")), false) }) it("keeps a zero byte marker as a zero byte file", async () => { @@ -221,7 +225,12 @@ describe("runExtraction on a zip", () => { assert.equal(statSync(workspacePath("target", "vintagestory", "assets", "version-1.22.6.txt")).size, 0) }) - it("coalesces 7-Zip progress and emits one terminal 100", async () => { + // Two 7-Zip processes over a 2000 file archive run past the 5 s default on a + // Windows runner, where spawning and scanning cost far more than on Linux. + // The coalescing itself has nothing platform-specific in it, so the ubuntu + // run covers it; and PR #274 replaces this test outright, dropping 7-Zip for + // a yauzl reader that needs no process at all. + it.skipIf(process.platform === "win32")("coalesces 7-Zip progress and emits one terminal 100", async () => { const source = workspacePath("many-files") mkdirSync(source, { recursive: true }) for (let index = 0; index < 2_000; index++) writeFileSync(join(source, `file-${index}.bin`), Buffer.alloc(2_048, index % 251)) diff --git a/tests/ipc/gameHandlers.test.ts b/tests/ipc/gameHandlers.test.ts index f48bd579..4208e151 100644 --- a/tests/ipc/gameHandlers.test.ts +++ b/tests/ipc/gameHandlers.test.ts @@ -43,6 +43,31 @@ vi.mock("@src/ipc/accountStore", () => ({ adoptLegacySingleAccountSecrets: vi.fn(async () => false) })) +/** + * Makes the next spawn throw the way Windows throws, on demand. + * + * `child_process.spawn` reports ENOENT, EACCES, EAGAIN, EMFILE and ENFILE + * through an "error" event and throws every other failure synchronously. Linux + * answers this file's fixtures with EACCES and so only ever takes the event + * path, while Windows answers a file that is not a real executable with + * UNKNOWN and takes the throw path, which used to reject the whole handler. + * Every other test here keeps the real spawn: the flag is off unless a test + * turns it on. + */ +const spawnThrow = vi.hoisted(() => ({ next: false })) + +vi.mock("node:child_process", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + spawn: (...args: Parameters): ReturnType => { + if (!spawnThrow.next) return actual.spawn(...args) + spawnThrow.next = false + throw Object.assign(new Error("spawn UNKNOWN"), { code: "UNKNOWN", errno: -4094, syscall: "spawn" }) + } + } +}) + // Real implementation, wrapped, so the crash-safety guarantee stays covered by // atomicJsonFile.test.ts and this file only asserts that the settings write // goes through the shared adapter rather than a bare fse.writeJSON. @@ -57,6 +82,17 @@ type LookForAGameVersionHandler = (event: IpcMainInvokeEvent, path: unknown) => /** The key the game writes after prompting the player, which the launcher has never seen. */ const GAME_REFRESHED_KEY = "game-session-key" +/** + * The game binary's file name on the host these tests run on. + * + * buildGameLaunchPlan, and detectInstalledGameVersion with it, looks for + * `Vintagestory.exe` on Windows and the native `Vintagestory` on Linux, so a + * fixture that hard-codes either name is only a game folder on one of the two. + * On the other, the handler finds nothing and every test below gets + * `no-executable` back instead of the outcome it was written for. + */ +const GAME_EXECUTABLE = process.platform === "win32" ? "Vintagestory.exe" : "Vintagestory" + let temporaryRoot: string let managedFolder: string let versionsFolder: string @@ -105,6 +141,11 @@ function writeConfig(config: Partial): void { } beforeEach(async () => { + // vi.restoreAllMocks() in afterEach does not reach a vi.hoisted object, so a + // test that turns this on and fails before the spawn consumes it would leave + // it on for the next test, whose real spawn would then throw. + spawnThrow.next = false + temporaryRoot = mkdtempSync(join(tmpdir(), "game-handlers-")) managedFolder = join(temporaryRoot, "Installations") versionsFolder = join(temporaryRoot, "Versions") @@ -166,7 +207,10 @@ describe("EXECUTE_GAME", () => { assert.deepEqual(result, { ok: false, reason: "invalid-request" }) }) - it("resolves no-executable when the version folder cannot be listed", async () => { + // chmod 0o000 cannot make a folder unlistable on Windows: NTFS has no POSIX + // mode bits, so readdir succeeds there and the readdir-failure arm this + // covers is unreachable. + it.skipIf(process.platform === "win32")("resolves no-executable when the version folder cannot be listed", async () => { const gameVersionFolder = join(versionsFolder, "1.20.0") const installationFolder = join(managedFolder, "Main") mkdirSync(gameVersionFolder, { recursive: true }) @@ -190,7 +234,7 @@ describe("EXECUTE_GAME", () => { mkdirSync(installationFolder, { recursive: true }) const realTarget = join(temporaryRoot, "real-binary") writeFileSync(realTarget, "", "utf-8") - symlinkSync(realTarget, join(gameVersionFolder, "Vintagestory")) + symlinkSync(realTarget, join(gameVersionFolder, GAME_EXECUTABLE)) writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"] }) const event = await createTrustedEvent() @@ -227,7 +271,7 @@ describe("EXECUTE_GAME", () => { // That's what exercises the account-less branch of // "if (account && accountSecrets)" and gameProcessOutcomeToResult's // `started: false` arm. - const executablePath = join(gameVersionFolder, "Vintagestory") + const executablePath = join(gameVersionFolder, GAME_EXECUTABLE) writeFileSync(executablePath, "not a real binary", { mode: 0o644 }) writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"] }) @@ -236,12 +280,37 @@ describe("EXECUTE_GAME", () => { assert.deepEqual(result, { ok: false, reason: "launch-failed" }) }) + /** + * A spawn that throws instead of emitting is still a launch that did not + * happen, and this handler's whole contract is that a launch a player can + * fail to complete comes back as a reason rather than as an exception. It + * only shows up on Windows, where a Vintagestory.exe that is not a valid + * executable (a truncated download, a file antivirus emptied) is answered + * with UNKNOWN rather than with one of the five codes spawn reports through + * an event. Both spawns in this file were written for the event alone. + */ + it("resolves launch-failed when the spawn throws instead of emitting an error", async () => { + const gameVersionFolder = join(versionsFolder, "1.20.0") + const installationFolder = join(managedFolder, "Main") + mkdirSync(gameVersionFolder, { recursive: true }) + mkdirSync(installationFolder, { recursive: true }) + writeFileSync(join(gameVersionFolder, GAME_EXECUTABLE), "not a real binary", { mode: 0o644 }) + writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"] }) + + spawnThrow.next = true + const event = await createTrustedEvent() + const result = await executeGameHandler()(event, { version: "1.20.0", path: gameVersionFolder }, baseInstallation({ path: installationFolder })) + + assert.deepEqual(result, { ok: false, reason: "launch-failed" }) + assert.equal(spawnThrow.next, false, "the throwing spawn is the one this test ran") + }) + it("resolves launch-failed after successfully writing the account session first", async () => { const gameVersionFolder = join(versionsFolder, "1.20.0") const installationFolder = join(managedFolder, "Main") mkdirSync(gameVersionFolder, { recursive: true }) mkdirSync(installationFolder, { recursive: true }) - const executablePath = join(gameVersionFolder, "Vintagestory") + const executablePath = join(gameVersionFolder, GAME_EXECUTABLE) writeFileSync(executablePath, "not a real binary", { mode: 0o644 }) writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], @@ -263,7 +332,10 @@ describe("EXECUTE_GAME", () => { assert.deepEqual(vi.mocked(writeJsonAtomic).mock.calls.filter((call) => call[0] === join(installationFolder, "clientsettings.json")).length, 1) }) - it("resolves session-write-failed when the account session cannot be written into clientsettings.json", async () => { + // chmod 0o500 on the installation folder does not stop the write on Windows, + // which gates writes on the file's own read-only attribute rather than on + // POSIX write bits of the folder containing it. + it.skipIf(process.platform === "win32")("resolves session-write-failed when the account session cannot be written into clientsettings.json", async () => { const gameVersionFolder = join(versionsFolder, "1.20.0") const installationFolder = join(managedFolder, "Main") mkdirSync(gameVersionFolder, { recursive: true }) @@ -271,7 +343,7 @@ describe("EXECUTE_GAME", () => { // A real, non-symlink "Vintagestory" file is what buildGameLaunchPlan and // assertExecutable both need to see, on Linux, to hand back a plan instead // of a launchPlanFailureResult/invalidExecutableResult. - writeFileSync(join(gameVersionFolder, "Vintagestory"), "", "utf-8") + writeFileSync(join(gameVersionFolder, GAME_EXECUTABLE), "", "utf-8") writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], accounts: [{ email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false }], @@ -304,7 +376,7 @@ describe("EXECUTE_GAME", () => { const installationFolder = join(managedFolder, "Main") mkdirSync(gameVersionFolder, { recursive: true }) mkdirSync(installationFolder, { recursive: true }) - writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) + writeFileSync(join(gameVersionFolder, GAME_EXECUTABLE), "not a real binary", { mode: 0o644 }) writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], accounts: [{ email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false }], @@ -338,7 +410,7 @@ describe("EXECUTE_GAME", () => { const installationFolder = join(managedFolder, "Main") mkdirSync(gameVersionFolder, { recursive: true }) mkdirSync(installationFolder, { recursive: true }) - writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) + writeFileSync(join(gameVersionFolder, GAME_EXECUTABLE), "not a real binary", { mode: 0o644 }) writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], accounts: [{ email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false }], @@ -363,12 +435,15 @@ describe("EXECUTE_GAME", () => { assert.equal(settings.stringSettings.sessionkey, "our-own-stale-key", "our own session, even a stale one, is left exactly as it was") }) - it("resolves session-write-failed when a foreign session cannot be cleared", async () => { + // Same reason as the write test above: the read-only folder that blocks + // writeJsonAtomic's rename on a POSIX filesystem does not block it on NTFS, + // which has no such mode bits. + it.skipIf(process.platform === "win32")("resolves session-write-failed when a foreign session cannot be cleared", async () => { const gameVersionFolder = join(versionsFolder, "1.20.0") const installationFolder = join(managedFolder, "Main") mkdirSync(gameVersionFolder, { recursive: true }) mkdirSync(installationFolder, { recursive: true }) - writeFileSync(join(gameVersionFolder, "Vintagestory"), "", "utf-8") + writeFileSync(join(gameVersionFolder, GAME_EXECUTABLE), "", "utf-8") writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], accounts: [{ email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false }], @@ -408,7 +483,7 @@ describe("EXECUTE_GAME", () => { const installationFolder = join(managedFolder, "Main") mkdirSync(gameVersionFolder, { recursive: true }) mkdirSync(installationFolder, { recursive: true }) - writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) + writeFileSync(join(gameVersionFolder, GAME_EXECUTABLE), "not a real binary", { mode: 0o644 }) writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], accounts: [{ email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false }], @@ -448,7 +523,7 @@ describe("EXECUTE_GAME", () => { const installationFolder = join(managedFolder, "Main") mkdirSync(gameVersionFolder, { recursive: true }) mkdirSync(installationFolder, { recursive: true }) - writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) + writeFileSync(join(gameVersionFolder, GAME_EXECUTABLE), "not a real binary", { mode: 0o644 }) writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], accounts: [{ email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false }], @@ -493,7 +568,7 @@ describe("EXECUTE_GAME", () => { const installationFolder = join(managedFolder, "Main") mkdirSync(gameVersionFolder, { recursive: true }) mkdirSync(installationFolder, { recursive: true }) - writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) + writeFileSync(join(gameVersionFolder, GAME_EXECUTABLE), "not a real binary", { mode: 0o644 }) writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], accounts: [ @@ -529,7 +604,7 @@ describe("EXECUTE_GAME", () => { const installationFolder = join(managedFolder, "Main") mkdirSync(gameVersionFolder, { recursive: true }) mkdirSync(installationFolder, { recursive: true }) - writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) + writeFileSync(join(gameVersionFolder, GAME_EXECUTABLE), "not a real binary", { mode: 0o644 }) writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], accounts: [ @@ -576,7 +651,7 @@ describe("EXECUTE_GAME", () => { const installationFolder = join(managedFolder, "Main") mkdirSync(gameVersionFolder, { recursive: true }) mkdirSync(installationFolder, { recursive: true }) - writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) + writeFileSync(join(gameVersionFolder, GAME_EXECUTABLE), "not a real binary", { mode: 0o644 }) writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], accounts: [ @@ -640,7 +715,7 @@ describe("LOOK_FOR_A_GAME_VERSION", () => { // A directory named like the Linux candidate: pickExecutable matches it by // name alone, so the probe is attempted, and assertExecutable's own // isFile() check is what refuses it -- no process ever spawns. - mkdirSync(join(folder, "Vintagestory"), { recursive: true }) + mkdirSync(join(folder, GAME_EXECUTABLE), { recursive: true }) writeConfig({ gameVersions: [{ version: "1.20.0", path: folder }] as unknown as ConfigType["gameVersions"] }) const event = await createTrustedEvent() @@ -648,6 +723,23 @@ describe("LOOK_FOR_A_GAME_VERSION", () => { assert.deepEqual(result, { exists: false }) }) + // The probe's own spawn has the same throw-instead-of-emit gap EXECUTE_GAME's + // does, and reaching it needs a candidate that gets past assertExecutable, so + // this one is a real file rather than a directory. + it("reports not found when the probe's spawn throws instead of emitting an error", async () => { + const folder = join(versionsFolder, "throwing-probe") + mkdirSync(folder, { recursive: true }) + writeFileSync(join(folder, GAME_EXECUTABLE), "not a real binary", { mode: 0o644 }) + writeConfig({ gameVersions: [{ version: "1.20.0", path: folder }] as unknown as ConfigType["gameVersions"] }) + + spawnThrow.next = true + const event = await createTrustedEvent() + const result = await lookForAGameVersionHandler()(event, folder) + + assert.deepEqual(result, { exists: false }) + assert.equal(spawnThrow.next, false, "the throwing spawn is the one this test ran") + }) + it("reports not found when only the mono fallback candidate (Vintagestory.exe) is present and fails its probe", async () => { const folder = join(versionsFolder, "mono-fallback") // No "Vintagestory" (the native candidate checked first), so diff --git a/tests/ipc/modsHandlers.test.ts b/tests/ipc/modsHandlers.test.ts index f7d7f111..269b1005 100644 --- a/tests/ipc/modsHandlers.test.ts +++ b/tests/ipc/modsHandlers.test.ts @@ -162,7 +162,10 @@ describe("EXPORT_MODPACK", () => { assert.equal(vi.mocked(dialog.showSaveDialog).mock.calls.length, 0) }) - it("returns success: false when the picked destination cannot be written to (permission denied)", async () => { + // chmod 0o500 on the directory does not stop the rename on Windows, which + // only checks the file's own read-only attribute, not POSIX write bits on + // the containing folder. + it.skipIf(process.platform === "win32")("returns success: false when the picked destination cannot be written to (permission denied)", async () => { // Pre-seed an existing file, then take away write permission on its // DIRECTORY. writeJsonAtomic writes the temp file and renames it over the // destination rather than truncating it in place, so a read-only target diff --git a/tests/ipc/pathsHandlers.test.ts b/tests/ipc/pathsHandlers.test.ts index bf0722d5..c92edd98 100644 --- a/tests/ipc/pathsHandlers.test.ts +++ b/tests/ipc/pathsHandlers.test.ts @@ -259,7 +259,11 @@ describe("DOWNLOAD_ON_PATH / EXTRACT_ON_PATH / RUN_INSTALLER / COMPRESS_ON_PATH: }) }) -describe("CHANGE_PERMS: assertion throws", () => { +// CHANGE_PERMS returns false on anything that is not Linux before it looks at +// its arguments at all (pathsHandlers.ts: `if (os.platform() !== "linux") return +// false`), since POSIX mode bits are the only thing it has to apply. On Windows +// nothing here can throw, so these two cover the Linux arm only. +describe.skipIf(process.platform === "win32")("CHANGE_PERMS: assertion throws", () => { it("throws on an empty paths array", async () => { const event = await createTrustedEvent() await assert.rejects(() => handler(IPC_CHANNELS.PATHS_MANAGER.CHANGE_PERMS)(event, [], 0o644), /Invalid permissions paths/) @@ -832,7 +836,9 @@ describe("COMPRESS_ON_PATH: runTrackedWorker via a fake worker", () => { }) }) -describe("CHANGE_PERMS: runTrackedWorker via a fake worker", () => { +// Same Linux-only early return: on Windows the handler resolves false without +// ever starting a worker, so the fake worker this waits for never arrives. +describe.skipIf(process.platform === "win32")("CHANGE_PERMS: runTrackedWorker via a fake worker", () => { it("resolves true once the worker finishes", async () => { const event = await createTrustedEvent() const workerPromise = nextTrackedWorker() @@ -997,7 +1003,10 @@ describe("before-quit: the limiters stop admitting work", () => { }) describe("RUN_INSTALLER", () => { - it("resolves not-windows on a non-Windows host, before any worker or spawn", async () => { + // This exercises the real, unstubbed process.platform !== "win32" branch + // (see the file header): on an actual Windows host that branch can't fire, + // so RUN_INSTALLER proceeds into the win32 arm this file doesn't cover. + it.skipIf(process.platform === "win32")("resolves not-windows on a non-Windows host, before any worker or spawn", async () => { // assertManagedPath for the installer path requires it to exist (it is // not called with { allowMissing: true }), so the not-windows check -- // which comes after it -- still needs a real file to reach. diff --git a/tests/ipc/permissions.test.ts b/tests/ipc/permissions.test.ts index 2f79fb51..fc19f4a1 100644 --- a/tests/ipc/permissions.test.ts +++ b/tests/ipc/permissions.test.ts @@ -79,7 +79,11 @@ afterEach(() => { }) describe("changePermissions", () => { - it("applies the mode to the root, its files and everything nested under it", () => { + // chmod on Windows only toggles the read-only attribute; it cannot produce + // POSIX mode bits like 0o755 or 0o600, so the mode a real tree ends up with + // has nothing to do with what changePermissions asked for. These read the + // mode back off disk, so they only mean anything on a POSIX filesystem. + it.skipIf(process.platform === "win32")("applies the mode to the root, its files and everything nested under it", () => { changePermissions({ paths: [installation], perms: 0o755 }) assert.equal(modeOf(installation), 0o755) @@ -88,14 +92,14 @@ describe("changePermissions", () => { assert.equal(modeOf(installation, "assets", "version.txt"), 0o755) }) - it("applies the mode to a single file given directly", () => { + it.skipIf(process.platform === "win32")("applies the mode to a single file given directly", () => { changePermissions({ paths: [join(installation, "Vintagestory")], perms: 0o750 }) assert.equal(modeOf(installation, "Vintagestory"), 0o750) assert.equal(modeOf(installation, "assets", "version.txt"), 0o600) }) - it("walks every root it is given", () => { + it.skipIf(process.platform === "win32")("walks every root it is given", () => { const second = workspacePath("data") mkdirSync(second) writeFileSync(join(second, "clientsettings.json"), "{}", { mode: 0o600 }) @@ -106,19 +110,19 @@ describe("changePermissions", () => { assert.equal(modeOf(second, "clientsettings.json"), 0o755) }) - it("skips a path that is not there", () => { + it.skipIf(process.platform === "win32")("skips a path that is not there", () => { assert.doesNotThrow(() => changePermissions({ paths: [workspacePath("never-installed"), installation], perms: 0o755 })) assert.equal(modeOf(installation, "Vintagestory"), 0o755) }) - it("does nothing at all when given no paths", () => { + it.skipIf(process.platform === "win32")("does nothing at all when given no paths", () => { changePermissions({ paths: [], perms: 0o755 }) assert.equal(modeOf(installation, "Vintagestory"), 0o600) }) - it("refuses a symbolic link rather than applying the mode to what it points at", () => { + it.skipIf(process.platform === "win32")("refuses a symbolic link rather than applying the mode to what it points at", () => { const outsider = workspacePath("outsider.txt") writeFileSync(outsider, "not the launcher's file", { mode: 0o600 }) symlinkSync(outsider, join(installation, "shortcut"))