From 91cbce2e38479ec3470849615003b9e3bbea3f65 Mon Sep 17 00:00:00 2001 From: twaldin Date: Wed, 10 Jun 2026 13:43:06 -0700 Subject: [PATCH] fix(tests): replace mock.module with _depsForTest seam in remote-cmd tests bun's mock.module is process-global and never unmocked; when linux CI's test-file order ran remote-cmd.test.ts before remotes.test.ts, the src/remotes mock leaked and failed 6 remotes tests on main. Swap the ssh/remotes collaborators through a _depsForTest seam (same convention as ssh.ts/_fsForTest) and restore originals in afterAll. --- src/commands/remote.ts | 33 +++++++++++++++++++++++---------- tests/unit/remote-cmd.test.ts | 29 +++++++++++++++-------------- 2 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/commands/remote.ts b/src/commands/remote.ts index fa491c9..018d724 100644 --- a/src/commands/remote.ts +++ b/src/commands/remote.ts @@ -10,6 +10,19 @@ export const _fsForTest: { mkdtempSync: (prefix: string) => string } = { existsSync, mkdtempSync } +// Seam for ssh/remotes collaborators. Tests swap these members instead of +// mock.module('../ssh' | '../remotes'), which is process-global in bun and +// leaks into later test files (remotes.test.ts fails when file order puts +// remote-cmd.test.ts first — observed on linux CI, not macOS). +export const _depsForTest = { + sshExecCheck, + sshExec, + rsyncTo, + addRemoteEntry, + loadRemotes, + removeRemoteEntry, +} + interface AddRemoteArgs { alias: string host: string @@ -54,12 +67,12 @@ export async function addRemote(args: AddRemoteArgs): Promise { identityFile: args.identityFile, } - const probe = sshExecCheck(remote, 'true') + const probe = _depsForTest.sshExecCheck(remote, 'true') if (probe !== true) { throw new Error(`SSH authentication probe failed for ${args.host}.\n${probe.error}`) } - const uname = sshExec(remote, 'uname -m && uname -s') + const uname = _depsForTest.sshExec(remote, 'uname -m && uname -s') if (uname.status !== 0) { throw new Error(`Failed to detect remote architecture: ${uname.stderr || uname.stdout}`) } @@ -81,20 +94,20 @@ export async function addRemote(args: AddRemoteArgs): Promise { const bytes = new Uint8Array(await response.arrayBuffer()) await Bun.write(tempFile, bytes) - const mkdirResult = sshExec(remote, 'mkdir -p ~/.flt/bin') + const mkdirResult = _depsForTest.sshExec(remote, 'mkdir -p ~/.flt/bin') if (mkdirResult.status !== 0) { throw new Error(`Failed to prepare remote binary directory: ${mkdirResult.stderr || mkdirResult.stdout}`) } - rsyncTo(remote, tempFile, '~/.flt/bin/flt', { isDirectory: false }) + _depsForTest.rsyncTo(remote, tempFile, '~/.flt/bin/flt', { isDirectory: false }) - const chmodResult = sshExec(remote, 'chmod +x ~/.flt/bin/flt') + const chmodResult = _depsForTest.sshExec(remote, 'chmod +x ~/.flt/bin/flt') if (chmodResult.status !== 0) { throw new Error(`Failed to finalize remote binary install: ${chmodResult.stderr || chmodResult.stdout}`) } for (const rc of ['~/.bashrc', '~/.zshrc']) { - sshExec( + _depsForTest.sshExec( remote, `grep -qF 'export PATH=$HOME/.flt/bin:$PATH' ${rc} 2>/dev/null || echo 'export PATH=$HOME/.flt/bin:$PATH' >> ${rc}`, ) @@ -103,12 +116,12 @@ export async function addRemote(args: AddRemoteArgs): Promise { const skillsDir = join(process.env.HOME || '', '.flt', 'skills') if (skillsDir && _fsForTest.existsSync(skillsDir)) { - rsyncTo(remote, skillsDir, '~/.flt/skills/', { isDirectory: true }) + _depsForTest.rsyncTo(remote, skillsDir, '~/.flt/skills/', { isDirectory: true }) } else { console.warn(`Warning: local skills directory not found at ${skillsDir}; skipping skills sync.`) } - addRemoteEntry(args.alias, remote) + _depsForTest.addRemoteEntry(args.alias, remote) const user = remote.user ?? process.env.USER ?? 'user' const port = remote.port ?? 22 @@ -117,7 +130,7 @@ export async function addRemote(args: AddRemoteArgs): Promise { } export function listRemotes(): void { - const remotes = loadRemotes() + const remotes = _depsForTest.loadRemotes() const rows = Object.entries(remotes) if (rows.length === 0) { console.log('No remotes configured. Use "flt add remote " to add one.') @@ -142,7 +155,7 @@ export function listRemotes(): void { } export function removeRemote(alias: string): void { - if (removeRemoteEntry(alias)) { + if (_depsForTest.removeRemoteEntry(alias)) { console.log(`Removed remote "${alias}".`) return } diff --git a/tests/unit/remote-cmd.test.ts b/tests/unit/remote-cmd.test.ts index 7617d51..758e59d 100644 --- a/tests/unit/remote-cmd.test.ts +++ b/tests/unit/remote-cmd.test.ts @@ -1,6 +1,12 @@ import { describe, it, expect, mock, beforeEach, afterAll } from 'bun:test' import { join } from 'path' +import { _depsForTest, _fsForTest, addRemote, listRemotes, removeRemote } from '../../src/commands/remote' + +// Collaborators are swapped via the _depsForTest/_fsForTest seams instead of +// mock.module('../../src/ssh' | '../../src/remotes'): bun module mocks are +// process-global and leak into later test files (remotes.test.ts fails when +// file order puts this file first — observed on linux CI). const mockSshExecCheck = mock((_remote: unknown, _cmd: string) => true as true | { error: string }) const mockSshExec = mock((_remote: unknown, _cmd: string) => ({ stdout: '', stderr: '', status: 0 })) const mockRsyncTo = mock((_remote: unknown, _local: string, _remotePath: string, _opts?: unknown) => {}) @@ -12,20 +18,6 @@ const mockRemoveRemote = mock((_alias: string) => true) const mockExistsSync = mock((_path: string) => true) const mockMkdtempSync = mock((_prefix: string) => '/tmp/flt-remote-test') -mock.module('../../src/ssh', () => ({ - sshExecCheck: mockSshExecCheck, - sshExec: mockSshExec, - rsyncTo: mockRsyncTo, -})) - -mock.module('../../src/remotes', () => ({ - addRemote: mockAddRemote, - loadRemotes: mockLoadRemotes, - removeRemote: mockRemoveRemote, -})) - -import { _fsForTest, addRemote, listRemotes, removeRemote } from '../../src/commands/remote' - describe('remote commands', () => { const originalFetch = globalThis.fetch const originalWrite = Bun.write @@ -34,6 +26,7 @@ describe('remote commands', () => { const originalHome = process.env.HOME const originalExistsSync = _fsForTest.existsSync const originalMkdtempSync = _fsForTest.mkdtempSync + const originalDeps = { ..._depsForTest } const logSpy = mock((..._args: unknown[]) => {}) const warnSpy = mock((..._args: unknown[]) => {}) let downloadDir: string @@ -66,6 +59,13 @@ describe('remote commands', () => { _fsForTest.existsSync = mockExistsSync _fsForTest.mkdtempSync = mockMkdtempSync + _depsForTest.sshExecCheck = mockSshExecCheck + _depsForTest.sshExec = mockSshExec + _depsForTest.rsyncTo = mockRsyncTo + _depsForTest.addRemoteEntry = mockAddRemote + _depsForTest.loadRemotes = mockLoadRemotes + _depsForTest.removeRemoteEntry = mockRemoveRemote + globalThis.fetch = mock(async () => new Response(new Uint8Array([1, 2, 3]), { status: 200 })) as unknown as typeof fetch Bun.write = mock(async () => 3) as typeof Bun.write @@ -160,6 +160,7 @@ describe('remote commands', () => { process.env.HOME = originalHome _fsForTest.existsSync = originalExistsSync _fsForTest.mkdtempSync = originalMkdtempSync + Object.assign(_depsForTest, originalDeps) mock.restore() }) })