From 385d50eb607b0da48c68c8343df206a455cfb94b Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Sun, 5 Jul 2026 23:00:05 -0700 Subject: [PATCH] test(github): tighten owner-repo cache-churn and origin-mock hygiene Round-2 review follow-ups on the branch's own test edits: - gh-utils WSL cache-separation test used a non-matching "not found" error, so the upstream miss took the transient (uncached) path and the "cached hit" call silently re-probed; the git-process count pin had also been dropped. Use the real "No such remote" wording so the stable negative-cache path is exercised, and restore toHaveBeenCalledTimes(4) to guard against git-spawn churn. - Reset getOriginOwnerRepoMock in client.test.ts beforeEach blocks so a stale queued once-value cannot leak between getRepoUpstream tests. Co-Authored-By: Claude Co-authored-by: Orca --- src/main/github/client.test.ts | 3 +++ src/main/github/gh-utils.test.ts | 12 +++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index 3022a69d98d..83a667627ca 100644 --- a/src/main/github/client.test.ts +++ b/src/main/github/client.test.ts @@ -169,6 +169,7 @@ describe('getPRForBranch', () => { execFileAsyncMock.mockReset() ghExecFileAsyncMock.mockReset() getOwnerRepoMock.mockReset() + getOriginOwnerRepoMock.mockReset() getIssueOwnerRepoMock.mockReset() getOwnerRepoForRemoteMock.mockReset() resolvePRRepositoryCandidatesMock.mockReset() @@ -3107,6 +3108,7 @@ describe('updatePRState', () => { beforeEach(() => { ghExecFileAsyncMock.mockReset() getOwnerRepoMock.mockReset() + getOriginOwnerRepoMock.mockReset() ghRepoExecOptionsMock.mockClear() githubRepoContextMock.mockClear() acquireMock.mockReset() @@ -3167,6 +3169,7 @@ describe('GitHub GraphQL rate-limit guard', () => { execFileAsyncMock.mockReset() ghExecFileAsyncMock.mockReset() getOwnerRepoMock.mockReset() + getOriginOwnerRepoMock.mockReset() getIssueOwnerRepoMock.mockReset() getOwnerRepoForRemoteMock.mockReset() resolvePRRepositoryCandidatesMock.mockReset() diff --git a/src/main/github/gh-utils.test.ts b/src/main/github/gh-utils.test.ts index 0ecf9e6cd5c..55b951de4bb 100644 --- a/src/main/github/gh-utils.test.ts +++ b/src/main/github/gh-utils.test.ts @@ -221,11 +221,15 @@ describe('github owner/repo resolution', () => { }) it('keeps local host and local WSL owner/repo cache entries separate for the same path', async () => { - // upstream-first: each getOwnerRepo call tries upstream then origin + // upstream-first: each getOwnerRepo call tries upstream then origin. + // Why: use the real "No such remote" wording so the upstream miss takes the + // stable-missing negative-cache path (isStableMissingGitRemoteError). That + // lets the third call hit cache with zero probes, which the call-count pin + // below asserts — guarding against git-process churn (issue #6381 class). gitExecFileAsyncMock - .mockRejectedValueOnce(new Error('not found')) + .mockRejectedValueOnce(new Error("fatal: No such remote 'upstream'")) .mockResolvedValueOnce({ stdout: 'git@github.com:host/orca.git\n' }) - .mockRejectedValueOnce(new Error('not found')) + .mockRejectedValueOnce(new Error("fatal: No such remote 'upstream'")) .mockResolvedValueOnce({ stdout: 'git@github.com:wsl/orca.git\n' }) await expect(getOwnerRepo('/repo')).resolves.toEqual({ owner: 'host', repo: 'orca' }) @@ -253,6 +257,8 @@ describe('github owner/repo resolution', () => { cwd: '/repo', wslDistro: 'Ubuntu' }) + // The third (cached) lookup must not re-probe: 2 remotes x 2 distinct runtimes. + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(4) }) it('prunes expired distinct owner/repo cache entries on later lookups', async () => {