Skip to content

Adds site-with-errors test back, only checks PRs daily#156

Merged
lindseywild merged 2 commits intomainfrom
lw/adds-back-site-with-errors-test
Mar 13, 2026
Merged

Adds site-with-errors test back, only checks PRs daily#156
lindseywild merged 2 commits intomainfrom
lw/adds-back-site-with-errors-test

Conversation

@lindseywild
Copy link
Contributor

@lindseywild lindseywild commented Mar 13, 2026

Adds back the site-with-errors tests to run on every PR, but will only check issues.

PR creation validation will be run at a daily cadence so we can add enough of a timeout to ensure the Copilot PRs are created (sometimes they take several minutes). Also adds a check in the workflow if the test does fail, so we can address it. Current site-with-errors test added back passes, excluding the PR check 🎉

I'll have to run the test-copilot-pr-creation.yml workflow once this PR is merged but can make any follow-up changes necessary!

name: Test Copilot PR creation
on:
schedule:
- cron: "0 6 * * *" # Every day at 06:00 UTC
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Random time; we can change to whenever.

@@ -0,0 +1,136 @@
name: Test Copilot PR creation
Copy link
Contributor Author

@lindseywild lindseywild Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entire workflow is the same as test.yml, except it adds a PR boolean (see later comment)

env:
GITHUB_TOKEN: ${{ secrets.GH_TOKEN }}
CACHE_PATH: ${{ steps.cache_key.outputs.cache_key }}
WAIT_FOR_PULL_REQUESTS: "true"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only major change in this file.

* Repeatedly calls `fn` until `predicate(result)` returns `true`, or until the timeout is exceeded.
* Errors thrown by `fn` (e.g. HTTP 404) are swallowed while polling continues.
*/
async function pollUntil<T>(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically just waiting until the PR is found, with a timeout of 15 minutes; that may be a bit too long, but hopefully it doesn't actually take this long for the PR to be created! We can experiment with lower timeouts if needed, but since this runs once a day I think it's probably fine.


describe.runIf(!!process.env.GITHUB_TOKEN)('—', () => {
let octokit: Octokit
describe.runIf(!!process.env.GITHUB_TOKEN)('issues', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will only run against issues in the existing CI test, which are guaranteed -- but if they fail, then clearly there's real issue 😅

@lindseywild lindseywild marked this pull request as ready for review March 13, 2026 13:53
@lindseywild lindseywild requested a review from a team as a code owner March 13, 2026 13:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Re-enables the site-with-errors integration test suite and introduces a scheduled workflow intended to validate Copilot PR creation separately from per-PR checks, reducing PR CI latency while still monitoring PR-creation reliability.

Changes:

  • Unskips tests/site-with-errors.test.ts, adds polling helper, and gates PR validation behind WAIT_FOR_PULL_REQUESTS.
  • Adds a scheduled workflow to run the PR-creation validation daily (with cleanup and failure notification).
  • Adds a reusable workflow to open an issue when a workflow fails.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
tests/site-with-errors.test.ts Re-enables the test, factors Octokit creation, adds polling, and splits issue vs PR assertions via env gating.
.github/workflows/test-copilot-pr-creation.yml New daily/dispatch workflow to run scan + tests with PR polling enabled and cleanup/failure reporting.
.github/workflows/report-failure.yml New reusable workflow intended to file an issue on failure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

const OctokitWithThrottling = Octokit.plugin(throttling)

describe.skip('site-with-errors', () => {
const WAIT_FOR_PULL_REQUESTS = !!process.env.WAIT_FOR_PULL_REQUESTS
Comment on lines +22 to +33
const deadline = Date.now() + timeoutMs
let lastError: unknown
while (true) {
try {
const result = await fn()
if (predicate(result)) return result
} catch (error) {
lastError = error
}
if (Date.now() >= deadline) {
throw lastError ?? new Error(`Timed out after ${timeoutMs}ms waiting for condition`)
}
branch="$(gh pr view "$URL" --json headRefName -q .headRefName || true)"
if [[ -n "$branch" ]]; then
echo "Deleting branch: $branch"
gh api -X DELETE "repos/${{ env.TESTING_REPOSITORY }}/git/refs/heads/$branch" || echo "Failed to delete branch: $branch"
- uses: actions/github-script@v8
with:
script: |
github.rest.issues.create({ owner: context.repo.owner, repo: context.repo.repo, title: "`${{ github.workflow }}` workflow failed", body: "The workflow, `${{ github.workflow }}`, failed to run. See [Action run output](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}) for more information. cc: @github/accessibility-reviewers." })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like a legit issue, right? specifically about awaiting the issue creation?

notify-of-failures:
if: always() && github.ref_name == 'main' && needs.test_pull_request_creation.result == 'failure'
needs: [test_pull_request_creation]
uses: github/accessibility-scorecard/.github/workflows/report-failure.yml@main
while (true) {
try {
const result = await fn()
if (predicate(result)) return result
Copy link
Contributor

@abdulahmad307 abdulahmad307 Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does a while loop respect the await call? (I don't think so, but I could be wrong). Should we add a time check? or we can use setInterval with a timeout of like 1 second.
actually 🤔 I'm prob wrong. I think it should be respecting it. but I do think this might be simpler if we use an interval instead of the while(true) loop?

would be something like this (I omitted the try/catch just to keep the example short):

return new Promise((resolve, reject) => {
  setInterval(async () => {
    const result = await fn()
    if (predicate(result)) {
      resolve(result)
      return;
    }

    if (Date.now() >= deadline) {
      reject(lastError ?? new Error(`Timed out after ${timeoutMs}ms waiting for condition`))
    }
  }, 30 * 1000)
})

Copy link
Contributor

@abdulahmad307 abdulahmad307 Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although, idk if we have to clear the interval when we resolve/reject the promise with a clearInterval(intervalId. 🤷 I suppose either way is fine.

(to close the loop on this, we would have to clear the interval. I just tested and the interval keeps going even when the promise is resolved)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it does! It will still wait until the fn() resolves. The deadline var will handle a timeout if things are taking too long.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops sorry I missed this originally!!

Copy link
Contributor

@abdulahmad307 abdulahmad307 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont see anything major to be worried about. 🚀

@lindseywild lindseywild merged commit b826e26 into main Mar 13, 2026
6 checks passed
@lindseywild lindseywild deleted the lw/adds-back-site-with-errors-test branch March 13, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants