Skip empty annotations reads for completed checks (#35524)

* Skip empty annotation requests for completed checks

* test(github): trim redundant cases

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
OrcaWin
2026-10-28 21:32:05 -07:00
committed by GitHub
co-authored by OrcaWin
parent 0699da6143
commit 90742fc22c
2 changed files with 62 additions and 13 deletions
@@ -177,6 +177,52 @@ describe('getPRCheckDetails', () => {
)
})
it.each([
{ status: 'completed', count: 0, calls: 1 },
{ status: 'in_progress', count: 0, calls: 2 },
{ status: 'completed', count: 1, calls: 2 },
{ status: 'completed', count: undefined, calls: 2 }
])(
'only skips authoritative empty annotations for a completed check: %j',
async ({ status, count, calls }) => {
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
if (args[1] === 'repos/acme/widgets/check-runs/88') {
return {
stdout: JSON.stringify({
name: 'verify',
status,
conclusion: status === 'completed' ? 'failure' : null,
output: { title: 'Output retained', summary: 'Details', annotations_count: count }
})
}
}
if (args[1] === 'repos/acme/widgets/check-runs/88/annotations?per_page=20') {
return {
stdout: JSON.stringify(
count === 1
? [
{
path: 'src/index.ts',
start_line: 1,
end_line: 1,
annotation_level: 'failure',
message: 'Retained'
}
]
: []
)
}
}
throw new Error(`Unexpected request: ${args.join(' ')}`)
})
const details = await getPRCheckDetails('/repo-root', { checkRunId: 88 })
expect(details).toMatchObject({ title: 'Output retained', summary: 'Details', status })
expect(details?.annotations).toHaveLength(count === 1 ? 1 : 0)
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(calls)
}
)
it('rejects provider failures so callers can distinguish them from unavailable details', async () => {
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
ghExecFileAsyncMock.mockRejectedValueOnce(new Error('authentication failed'))
@@ -78,6 +78,7 @@ export async function getPRCheckDetails(
return call.result
}
let checkRun: Record<string, unknown> | null = null
let output: Record<string, unknown> | null = null
let annotations: PRCheckRunDetails['annotations'] = []
if (args.checkRunId) {
const { stdout } = await fetchMetadata([
@@ -89,15 +90,21 @@ export async function getPRCheckDetails(
parsed && typeof parsed === 'object' && !Array.isArray(parsed)
? Object.fromEntries(Object.entries(parsed))
: null
try {
const annotationsResult = await fetchMetadata([
'api',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/check-runs/${args.checkRunId}/annotations?per_page=20`
])
annotations = mapCheckAnnotations(JSON.parse(annotationsResult.stdout))
} catch (err) {
rethrowCheckDetailsAbort(controller.signal, err)
console.warn('getPRCheckDetails', 'annotations_failed')
output =
checkRun?.output && typeof checkRun.output === 'object'
? Object.fromEntries(Object.entries(checkRun.output))
: null
if (checkRun?.status !== 'completed' || output?.annotations_count !== 0) {
try {
const annotationsResult = await fetchMetadata([
'api',
`repos/${ownerRepo.owner}/${ownerRepo.repo}/check-runs/${args.checkRunId}/annotations?per_page=20`
])
annotations = mapCheckAnnotations(JSON.parse(annotationsResult.stdout))
} catch (err) {
rethrowCheckDetailsAbort(controller.signal, err)
console.warn('getPRCheckDetails', 'annotations_failed')
}
}
await waitPrevious()
}
@@ -124,10 +131,6 @@ export async function getPRCheckDetails(
await waitPrevious()
}
const output =
checkRun?.output && typeof checkRun.output === 'object'
? Object.fromEntries(Object.entries(checkRun.output))
: null
return {
name: nullableString(checkRun?.name) ?? args.checkName ?? 'Check',
status: nullableString(checkRun?.status),