Skip to content

Commit 4157a17

Browse files
Copilotlindseywild
andcommitted
Fix Copilot PR timing: increase retry limit and make tests resilient to missing PRs
Co-authored-by: lindseywild <35239154+lindseywild@users.noreply.github.com>
1 parent ba9918d commit 4157a17

File tree

3 files changed

+21
-20
lines changed

3 files changed

+21
-20
lines changed

.github/actions/fix/src/retry.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ function sleep(ms: number): Promise<void> {
1616
*/
1717
export async function retry<T>(
1818
fn: () => Promise<T | null | undefined> | T | null | undefined,
19-
maxAttempts = 6,
19+
maxAttempts = 10,
2020
baseDelay = 2000,
2121
attempt = 1,
2222
): Promise<T | undefined> {

tests/site-with-errors.test.ts

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,10 @@ describe('site-with-errors', () => {
1616
})
1717

1818
it('cache has expected results', () => {
19-
const actual = results.map(({issue: {url: issueUrl}, pullRequest, findings}) => {
20-
const pullRequestUrl = pullRequest?.url
19+
const actual = results.map(({issue: {url: issueUrl}, findings}) => {
2120
const {problemUrl, solutionLong, screenshotId, ...finding} = findings[0]
2221
// Check volatile fields for existence only
2322
expect(issueUrl).toBeDefined()
24-
expect(pullRequestUrl).toBeDefined()
2523
expect(problemUrl).toBeDefined()
2624
expect(solutionLong).toBeDefined()
2725
// Check `problemUrl`, ignoring axe version
@@ -145,21 +143,22 @@ describe('site-with-errors', () => {
145143
)
146144
// Fetch pull requests referenced in the findings file
147145
pullRequests = await Promise.all(
148-
results.map(async ({pullRequest}) => {
149-
const pullRequestUrl = pullRequest?.url
150-
expect(pullRequestUrl).toBeDefined()
151-
const {owner, repo, pullNumber} =
152-
/https:\/\/github\.com\/(?<owner>[^/]+)\/(?<repo>[^/]+)\/pull\/(?<pullNumber>\d+)/.exec(
153-
pullRequestUrl!,
154-
)!.groups!
155-
const {data: fetchedPullRequest} = await octokit.request('GET /repos/{owner}/{repo}/pulls/{pull_number}', {
156-
owner,
157-
repo,
158-
pull_number: parseInt(pullNumber, 10),
159-
})
160-
expect(fetchedPullRequest).toBeDefined()
161-
return fetchedPullRequest
162-
}),
146+
results
147+
.filter(({pullRequest}) => !!pullRequest?.url)
148+
.map(async ({pullRequest}) => {
149+
const pullRequestUrl = pullRequest!.url
150+
const {owner, repo, pullNumber} =
151+
/https:\/\/github\.com\/(?<owner>[^/]+)\/(?<repo>[^/]+)\/pull\/(?<pullNumber>\d+)/.exec(
152+
pullRequestUrl,
153+
)!.groups!
154+
const {data: fetchedPullRequest} = await octokit.request('GET /repos/{owner}/{repo}/pulls/{pull_number}', {
155+
owner,
156+
repo,
157+
pull_number: parseInt(pullNumber, 10),
158+
})
159+
expect(fetchedPullRequest).toBeDefined()
160+
return fetchedPullRequest
161+
}),
163162
)
164163
})
165164

@@ -183,6 +182,8 @@ describe('site-with-errors', () => {
183182
})
184183

185184
it('pull requests exist and have expected author, state, and assignee', async () => {
185+
// Verify every result has an associated pull request (not just those that happened to be fetched)
186+
expect(pullRequests).toHaveLength(results.length)
186187
for (const pullRequest of pullRequests) {
187188
expect(pullRequest.user.login).toBe('Copilot')
188189
expect(pullRequest.state).toBe('open')

tests/types.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,5 +26,5 @@ export type PullRequest = {
2626
export type Result = {
2727
findings: Finding[]
2828
issue: Issue
29-
pullRequest: PullRequest
29+
pullRequest?: PullRequest
3030
}

0 commit comments

Comments
 (0)