test(github): cover persist, hydrate, and complete open-list misses
Add store persist/hydrate regressions for terminal branch associations and a server test that a complete empty open list does not query closed PRs. Inline the open-only matcher state so the next repo target can win. Co-authored-by: serkraser <serkraser@gmail.com>
This commit is contained in:
co-authored by
serkraser
parent
5e9e35897f
commit
13cbbc76d7
@@ -440,4 +440,104 @@ describe("GitHub PR status stale terminal associations", () => {
|
||||
expect(useGitHubPrStatusStore.getState().entries[key]?.status?.pr?.number).toBe(12)
|
||||
expect(useGitHubPrStatusStore.getState().entries[key]?.error).toBe("GitHub unavailable")
|
||||
})
|
||||
|
||||
test("does not persist a merged branch association", () => {
|
||||
const merged: GitHubPullRequestStatus = {
|
||||
connected: true,
|
||||
fetchedAt: 1_000,
|
||||
pr: { number: 12, title: "old", url: "u12", state: "merged", draft: false, base: "main", head: "feature" },
|
||||
}
|
||||
const key = getGitHubPrStatusKey("/repo", "feature", "origin")
|
||||
useGitHubPrStatusStore.getState().ensureEntry(key)
|
||||
useGitHubPrStatusStore.getState().setParams(key, params({} as RuntimeAPIs["github"], "feature"))
|
||||
useGitHubPrStatusStore.setState((state) => ({
|
||||
entries: {
|
||||
...state.entries,
|
||||
[key]: {
|
||||
...state.entries[key]!,
|
||||
status: merged,
|
||||
isInitialStatusResolved: true,
|
||||
lastRefreshAt: Date.now(),
|
||||
},
|
||||
},
|
||||
}))
|
||||
|
||||
const persisted = useGitHubPrStatusStore.persist.getOptions().partialize?.(
|
||||
useGitHubPrStatusStore.getState(),
|
||||
) as { entries?: Record<string, unknown> } | undefined
|
||||
expect(persisted?.entries?.[key]).toBeUndefined()
|
||||
})
|
||||
|
||||
test("still persists an open branch association", () => {
|
||||
const open: GitHubPullRequestStatus = {
|
||||
connected: true,
|
||||
fetchedAt: 1_000,
|
||||
pr: { number: 15, title: "new", url: "u15", state: "open", draft: false, base: "main", head: "feature" },
|
||||
}
|
||||
const key = getGitHubPrStatusKey("/repo", "feature", "origin")
|
||||
useGitHubPrStatusStore.getState().ensureEntry(key)
|
||||
useGitHubPrStatusStore.getState().setParams(key, params({} as RuntimeAPIs["github"], "feature"))
|
||||
useGitHubPrStatusStore.setState((state) => ({
|
||||
entries: {
|
||||
...state.entries,
|
||||
[key]: {
|
||||
...state.entries[key]!,
|
||||
status: open,
|
||||
isInitialStatusResolved: true,
|
||||
lastRefreshAt: Date.now(),
|
||||
},
|
||||
},
|
||||
}))
|
||||
|
||||
const persisted = useGitHubPrStatusStore.persist.getOptions().partialize?.(
|
||||
useGitHubPrStatusStore.getState(),
|
||||
) as { entries?: Record<string, { status?: GitHubPullRequestStatus | null }> } | undefined
|
||||
expect(persisted?.entries?.[key]?.status?.pr?.number).toBe(15)
|
||||
})
|
||||
|
||||
test("hydrate strips a legacy persisted merged PR and marks it unresolved", () => {
|
||||
const key = getGitHubPrStatusKey("/repo", "feature", "origin")
|
||||
const hydrated = useGitHubPrStatusStore.persist.getOptions().merge?.(
|
||||
{
|
||||
entries: {
|
||||
[key]: {
|
||||
status: {
|
||||
connected: true,
|
||||
fetchedAt: 1_000,
|
||||
repo: { owner: "acme", repo: "app", url: "https://github.com/acme/app" },
|
||||
pr: { number: 12, title: "old", url: "u12", state: "merged", draft: false, base: "main", head: "feature" },
|
||||
checks: { state: "success", total: 1, success: 1, failure: 0, pending: 0 },
|
||||
canMerge: true,
|
||||
},
|
||||
isInitialStatusResolved: true,
|
||||
lastRefreshAt: Date.now(),
|
||||
lastDiscoveryPollAt: 0,
|
||||
identity: {
|
||||
runtimeKey: "runtime-a",
|
||||
directory: "/repo",
|
||||
branch: "feature",
|
||||
remoteName: "origin",
|
||||
},
|
||||
resolvedRemoteName: "origin",
|
||||
},
|
||||
},
|
||||
},
|
||||
useGitHubPrStatusStore.getState(),
|
||||
) as {
|
||||
entries: Record<string, {
|
||||
status: GitHubPullRequestStatus | null
|
||||
isInitialStatusResolved: boolean
|
||||
}>
|
||||
}
|
||||
|
||||
expect(hydrated.entries[key]?.status?.pr).toBeNull()
|
||||
expect(hydrated.entries[key]?.status?.repo).toEqual({
|
||||
owner: "acme",
|
||||
repo: "app",
|
||||
url: "https://github.com/acme/app",
|
||||
})
|
||||
expect(hydrated.entries[key]?.status?.checks).toBeUndefined()
|
||||
expect(hydrated.entries[key]?.status?.canMerge).toBeUndefined()
|
||||
expect(hydrated.entries[key]?.isInitialStatusResolved).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -514,14 +514,11 @@ const findFirstMatchingPr = async ({ octokit, target, branch, sourceCandidates,
|
||||
|
||||
// Branch status associates the current head with an open PR only. Returning a
|
||||
// closed/merged PR here made the client cache a terminal status that could not
|
||||
// self-heal until a manual forced refresh.
|
||||
const state = 'open';
|
||||
// Shared per-repo list first: one pulls.list answers every branch of the
|
||||
// repo within the TTL. A miss in a complete list is authoritative — skip
|
||||
// the per-branch query fan entirely.
|
||||
// self-heal until a manual forced refresh. A miss also lets the next repo
|
||||
// target run, so an open upstream PR wins over a merged fork PR.
|
||||
let listWasComplete = false;
|
||||
try {
|
||||
const listEntry = await getRepoPulls(octokit, target.repo, state, { force });
|
||||
const listEntry = await getRepoPulls(octokit, target.repo, 'open', { force });
|
||||
const fromList = pickPreferred(listEntry.prs);
|
||||
if (fromList) {
|
||||
return fromList;
|
||||
@@ -539,7 +536,7 @@ const findFirstMatchingPr = async ({ octokit, target, branch, sourceCandidates,
|
||||
const directCandidates = await safeListPulls(octokit, {
|
||||
owner: target.repo.owner,
|
||||
repo: target.repo.repo,
|
||||
state,
|
||||
state: 'open',
|
||||
head: `${owner}:${branch}`,
|
||||
per_page: 100,
|
||||
});
|
||||
|
||||
@@ -86,4 +86,20 @@ describe('findFirstMatchingPr open-only branch status', () => {
|
||||
expect(listMock.mock.calls.every((call) => call[0]?.state === 'open')).toBe(true);
|
||||
expect(listMock.mock.calls.some((call) => call[0]?.state === 'closed')).toBe(false);
|
||||
});
|
||||
|
||||
test('does not query closed PRs when the open list is complete and empty', async () => {
|
||||
listMock.mockImplementation(async () => ({ data: [] }));
|
||||
|
||||
const pr = await findFirstMatchingPr({
|
||||
octokit: { rest: { pulls: { list: listMock } } },
|
||||
target: { repo: { owner: 'acme', repo: 'app' }, remoteName: 'origin' },
|
||||
branch: 'feature',
|
||||
sourceCandidates: [{ repo: { owner: 'acme', repo: 'app' } }],
|
||||
force: true,
|
||||
});
|
||||
|
||||
expect(pr).toBeNull();
|
||||
expect(listMock.mock.calls).toHaveLength(1);
|
||||
expect(listMock.mock.calls[0]?.[0]?.state).toBe('open');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user