spark-test commented on code in PR #60098: URL: https://github.com/apache/spark/pull/60098#discussion_r4474561884
########## .github/workflows/update_build_status.js: ########## @@ -0,0 +1,393 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +module.exports = async ({github, context, core, env = process.env, log = console}) => { + // Open PRs are read with one GraphQL query per page of PRs. For each PR it returns + // the Build checks on the head commit (statusCheckRollup.commit) and the fork's + // workflow runs for that commit (headRef.target), both as GitHub Actions check + // suites: fork runs are Actions runs, and Build checks written with the GITHUB_TOKEN + // land in Actions suites too. + // + // A PR rarely has more than a few such suites. A fork's default branch can collect + // thousands from scheduled workflows, but suites are listed oldest first and a push + // creates its runs' suites right away, so they are still among the first. Should a + // page ever not hold everything a PR needs, the PR fails loudly rather than being + // synced from partial data. + // + // GitHub aborts a GraphQL request after 10 seconds and allows 60 seconds of GraphQL + // CPU time per minute. A page of 25 PRs takes about 3 seconds, so each page is + // followed by a pause as long as it took. + // https://docs.github.com/en/graphql/overview/rate-limits-and-query-limits-for-the-graphql-api + const GITHUB_ACTIONS_APP_ID = 15368 + const PR_PAGE_SIZE = 25 + + // Quick syncs cover only open PRs updated in the last QUICK_SYNC_LOOKBACK_DAYS. A + // push, comment, review, or label change updates a PR, but a check change does not. + // Fork runs usually finish within hours of a push, and most reruns happen within + // days. A later rerun, or a gap in quick syncs longer than the lookback, is picked + // up by the next full sync, which covers all open PRs. Both are scheduled at the + // schedule in the workflow. + const QUICK_SYNC_LOOKBACK_DAYS = 3 + const fullSync = env.FULL_SYNC == 'true' + const updatedSince = fullSync ? 0 + : Date.now() - QUICK_SYNC_LOOKBACK_DAYS * 24 * 60 * 60 * 1000 + + // A missing Build check points at the fork's build_main.yml run for the PR branch. + // Right after a push that run may not be registered yet, so a PR updated in the last + // NEW_RUN_GRACE_MINUTES without a run is left for a later pass instead of being + // told that no run exists. + const BUILD_WORKFLOW = '.github/workflows/build_main.yml' + const NEW_RUN_GRACE_MINUTES = 5 + const runGraceSince = Date.now() - NEW_RUN_GRACE_MINUTES * 60 * 1000 + + const prPageQuery = ` + query($owner: String!, $repo: String!, $size: Int!, $cursor: String, $appId: Int!) { + repository(owner: $owner, name: $repo) { + pullRequests(states: OPEN, first: $size, after: $cursor, + orderBy: {field: UPDATED_AT, direction: DESC}) { + pageInfo { hasNextPage endCursor } + nodes { + number + updatedAt + headRefOid + statusCheckRollup { + commit { + oid + checkSuites(first: 10, filterBy: {appId: $appId}) { + pageInfo { hasNextPage } + nodes { + checkRuns(first: 10, + filterBy: {checkName: "Build", checkType: LATEST}) { + pageInfo { hasNextPage } + nodes { databaseId status conclusion title summary text } + } + } + } + } + } + headRef { + name + repository { name owner { login } } + target { + oid + ... on Commit { + checkSuites(first: 10, filterBy: {appId: $appId}) { + pageInfo { hasNextPage } + nodes { + status + conclusion + branch { name } + workflowRun { databaseId file { path } } + } + } + } + } + } + } + } + } + }` + + // Yield the open PRs updated since updatedSince one page at a time, most recently + // updated first, pausing after each page as long as it took (see above). A PR + // updated during the pass moves to the front and may be missed, but it is then + // recent enough for the next pass. + async function* listOpenPRs() { + let cursor = null + for (;;) { + const started = Date.now() + const data = await github.graphql(prPageQuery, { + ...context.repo, + size: PR_PAGE_SIZE, + cursor, + appId: GITHUB_ACTIONS_APP_ID + }) + const took = Date.now() - started + const page = data.repository.pullRequests + const recent = page.nodes.filter(node => + Date.parse(node.updatedAt) >= updatedSince) + yield recent + if (recent.length < page.nodes.length || !page.pageInfo.hasNextPage) { + return + } + cursor = page.pageInfo.endCursor + await new Promise(resolve => setTimeout(resolve, took)) + } + } + + // Flatten a PR from a page into its number, updatedAt, head sha, fork owner and repo, + // and: + // - buildChecks: the head commit's Build checks. + // - forkRuns: the fork's workflow runs for the head commit, by run ID, and + // forkRunsTruncated: whether the page held only some of them. + // - buildRun: the fork's newest build_main.yml run for the PR branch, if any. The + // branch matters: build_main.yml skips fork pushes to master ("Sync fork"), so the + // same commit can also carry a skipped run on another branch. + // Statuses and conclusions are lowercased to match the check-runs REST API. + // Return null if the fork branch is gone or not at the PR head commit (the branch + // was just pushed to and the PR has yet to follow). Throw if the page does not hold + // all of the PR's Build checks. + const toPR = (node) => { + const target = node.headRef?.target + const commit = node.statusCheckRollup?.commit + if (target?.oid != node.headRefOid || (commit && commit.oid != node.headRefOid)) { + return null + } + // A commit without any checks or statuses has no rollup. + const suites = commit?.checkSuites.nodes ?? [] + if (commit?.checkSuites.pageInfo.hasNextPage + || suites.some(s => s.checkRuns.pageInfo.hasNextPage)) { + throw new Error('The PR head commit has more check suites or Build checks ' + + 'than a page holds') + } + const lower = (value) => value?.toLowerCase() + const runs = target.checkSuites.nodes.filter(s => s.workflowRun).map(s => ({ + id: s.workflowRun.databaseId, + status: lower(s.status), + conclusion: lower(s.conclusion), + workflow: s.workflowRun.file?.path, + branch: s.branch?.name + })) + return { + number: node.number, + updatedAt: node.updatedAt, + sha: node.headRefOid, + forkOwner: node.headRef.repository.owner.login, + forkRepo: node.headRef.repository.name, + buildChecks: suites.flatMap(s => s.checkRuns.nodes).map(cr => ({ + id: cr.databaseId, + status: lower(cr.status), + conclusion: lower(cr.conclusion), + output: {title: cr.title, summary: cr.summary, text: cr.text} + })), + forkRuns: new Map(runs.map(run => [String(run.id), run])), + forkRunsTruncated: target.checkSuites.pageInfo.hasNextPage, + buildRun: runs.filter(run => run.workflow == BUILD_WORKFLOW + && run.branch == node.headRef.name).pop() + } + } + + // The fork run ID a Build check points at, stored as JSON in its output text, or + // undefined if the text is not that JSON (e.g. a check from an older version, a + // manual run, or another app). + const runIdOf = (cr) => { + try { + return JSON.parse(cr.output.text).run_id + } catch { + return undefined + } + } + + // The Build check state that shows a fork run's state. The check-runs API accepts + // fewer values than a run reports: the other pending statuses (requested, waiting, + // pending) become queued, and the other conclusions (startup_failure, stale, or + // none), which all mean the run produced no test result, become failure. + const checkState = (run) => { + if (run.status != 'completed') { + return {status: run.status == 'in_progress' ? 'in_progress' : 'queued'} + } + const conclusions = ['success', 'failure', 'neutral', 'cancelled', 'timed_out', + 'action_required', 'skipped'] + return { + status: 'completed', + conclusion: conclusions.includes(run.conclusion) ? run.conclusion : 'failure' + } + } + const label = (state) => state.status == 'completed' ? state.conclusion : state.status + const sameState = (a, b) => a.status == b.status + && (a.status != 'completed' || a.conclusion == b.conclusion) + + // PRs whose sync failed on this pass. A failure skips only that PR, which the next + // pass retries, and fails the step at the end so a recurring error stays visible. + const failedPRs = new Set() + + // The log lists only the Build checks a pass creates or updates, and errors, each + // with its PR and head commit. Everything else is counted for a summary at the end. + const count = {prs: 0, skipped: 0, updated: 0, created: 0, unchanged: 0, notApplied: 0} + const prUrl = `https://github.com/${context.repo.owner}/${context.repo.repo}/pull/` + + log.log(fullSync ? 'Full sync: all open PRs' + : 'Quick sync: open PRs updated in the last ' + QUICK_SYNC_LOOKBACK_DAYS + ' days') + for await (const prs of listOpenPRs()) { + for (const node of prs) { + count.prs++ + const ref = prUrl + node.number + ' ' + node.headRefOid.slice(0, 7) + let pr + try { + pr = toPR(node) + } catch (error) { + log.error(ref + ': ' + error.message + '; raise the limits in prPageQuery') + failedPRs.add(node.number) + continue + } + if (!pr) { + count.skipped++ + continue + } + + // Sync every Build check that points at a fork run, not just the first: a commit + // can carry more than one (e.g. left by the notify workflow this one replaced). + // An action_required check, written when no fork run was found, points at none. + let syncableBuildCheck = false + for (const cr of pr.buildChecks) { + if (cr.conclusion == 'action_required') { + continue + } + const runId = runIdOf(cr) + const run = pr.forkRuns.get(String(runId)) + if (!run) { + if (runId && pr.forkRunsTruncated) { + // The run may be among the fork's check suites the page left out. Keep + // the backfill from replacing the check, and fail the PR to get noticed. + log.error(ref + ': fork run ' + runId + ' not in the first fork check ' + + 'suites; raise the limits in prPageQuery') + failedPRs.add(pr.number) + syncableBuildCheck = true + } + // Otherwise the check's output is unusable or its fork run was deleted, so + // it can never be synced; the backfill below replaces it. + continue + } + syncableBuildCheck = true + + // Leave a check alone if it already shows the run's state, as nearly all do. + const state = checkState(run) + if (sameState(cr, state)) { + count.unchanged++ + continue + } + const change = label(cr) + ' -> ' + label(state) + try { + const {data: updated} = await github.rest.checks.update({ + ...context.repo, + check_run_id: cr.id, + output: cr.output, + ...state + }) + // A successful request can leave the state unchanged, notably when trying + // to reopen a completed check after a rerun. + if (sameState(cr, updated)) { + count.unchanged++ + } else { + log.log(ref + ': ' + label(cr) + ' -> ' + label(updated)) + count.updated++ + } + if (!sameState(updated, state)) { + count.notApplied++ + } + } catch (error) { + log.error(ref + ': ' + change + ' failed: ' + error.message) + failedPRs.add(pr.number) + } + } + + // No syncable Build check: none was created yet, only an action_required one + // exists, or the existing one points at a fork run that is gone. Create one + // from the fork's build_main.yml run for the PR branch (see toPR). A PR opened + // from the fork's master only has a skipped run, which has nothing to show. + if (!syncableBuildCheck) { + const run = pr.buildRun?.conclusion == 'skipped' ? undefined : pr.buildRun + if (!run) { + if (!pr.buildRun && pr.forkRunsTruncated) { + // The run may be among the fork's check suites the page left out. + log.error(ref + ': no build_main.yml run in the first fork check ' + + 'suites; raise the limits in prPageQuery') + failedPRs.add(pr.number) + continue + } + // Already told there is no run, or the run may not be registered yet. + if (pr.buildChecks.some(cr => cr.conclusion == 'action_required') + || Date.parse(pr.updatedAt) > runGraceSince) { + count.skipped++ + continue + } + } + + const state = run ? checkState(run) + : {status: 'completed', conclusion: 'action_required'} + const check = {...context.repo, name: 'Build', head_sha: pr.sha, ...state} + const change = 'none -> ' + label(state) + try { + if (run) { + const actions_url = 'https://github.com/' + pr.forkOwner + '/' + pr.forkRepo + + '/actions/runs/' + run.id + await github.rest.checks.create({ + ...check, + output: { + title: 'Test results', + summary: '[See test results](' + actions_url + ')\n\n' + + 'If the tests fail for reasons unrelated to this pull request, ' + + 'please rerun the workflow in your forked repository.\n' + + 'If the failures are related to this pull request, ' + + 'please investigate them and push follow-up changes.', + text: JSON.stringify({ + owner: pr.forkOwner, + repo: pr.forkRepo, + run_id: run.id + }) + }, + details_url: actions_url + }) + } else { + // No build_main.yml run to show for the PR branch: Actions is disabled, + // the PR is from the fork's master, or the commit never triggered a run. + // Tell the contributor how to fix it. + await github.rest.checks.create({ + ...check, + output: { + title: 'Workflow run detection failed', + summary: ` + Unable to detect the workflow run for testing the changes in your PR. + + 1. If you did not enable GitHub Actions in your forked repository, please enable it by clicking the button as shown in the image below. See also [Managing GitHub Actions settings for a repository](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository) for more details. Review Comment: **Nit (P3):** Enabling Actions doesn't start `build_main.yml` for a commit that's already pushed, because that workflow only triggers on `push`. And once this check exists, later passes skip the PR until a run appears for its head commit (lines 314-318). So a contributor who follows item 1, the usual case for a new fork whose workflows are disabled, sees nothing change and isn't told what to do next. The same is true for a commit that never triggered a run (for example, one whose message contains `[skip ci]`), which the comment above lists as a cause. The old text's second item ended with `git push origin YOUR_BRANCH --force`, which happened to start a build, and the rewrite drops it. Consider adding a step for PRs that aren't from the fork's `master`: after fixing the cause, push a new commit to the PR branch to start the build, and the Build check will update on a later pass. Scoped that way, it won't suggest pushing to `master` again, since item 2 already tells those contributors to use another branch. **Verification:** - **Inspection:** The summary of a newly created action_required check tells contributors whose PR is not from the fork's master to push a new commit to the PR branch after fixing the cause, does not tell contributors with a PR from the fork's master to push to master again, and still includes the enable-Actions and fork-master items and the enable-workflows image. ########## .github/workflows/update_build_status.js: ########## @@ -0,0 +1,393 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +module.exports = async ({github, context, core, env = process.env, log = console}) => { + // Open PRs are read with one GraphQL query per page of PRs. For each PR it returns + // the Build checks on the head commit (statusCheckRollup.commit) and the fork's + // workflow runs for that commit (headRef.target), both as GitHub Actions check + // suites: fork runs are Actions runs, and Build checks written with the GITHUB_TOKEN + // land in Actions suites too. + // + // A PR rarely has more than a few such suites. A fork's default branch can collect + // thousands from scheduled workflows, but suites are listed oldest first and a push + // creates its runs' suites right away, so they are still among the first. Should a + // page ever not hold everything a PR needs, the PR fails loudly rather than being + // synced from partial data. + // + // GitHub aborts a GraphQL request after 10 seconds and allows 60 seconds of GraphQL + // CPU time per minute. A page of 25 PRs takes about 3 seconds, so each page is + // followed by a pause as long as it took. + // https://docs.github.com/en/graphql/overview/rate-limits-and-query-limits-for-the-graphql-api + const GITHUB_ACTIONS_APP_ID = 15368 + const PR_PAGE_SIZE = 25 + + // Quick syncs cover only open PRs updated in the last QUICK_SYNC_LOOKBACK_DAYS. A + // push, comment, review, or label change updates a PR, but a check change does not. + // Fork runs usually finish within hours of a push, and most reruns happen within + // days. A later rerun, or a gap in quick syncs longer than the lookback, is picked + // up by the next full sync, which covers all open PRs. Both are scheduled at the + // schedule in the workflow. + const QUICK_SYNC_LOOKBACK_DAYS = 3 + const fullSync = env.FULL_SYNC == 'true' + const updatedSince = fullSync ? 0 + : Date.now() - QUICK_SYNC_LOOKBACK_DAYS * 24 * 60 * 60 * 1000 + + // A missing Build check points at the fork's build_main.yml run for the PR branch. + // Right after a push that run may not be registered yet, so a PR updated in the last + // NEW_RUN_GRACE_MINUTES without a run is left for a later pass instead of being + // told that no run exists. + const BUILD_WORKFLOW = '.github/workflows/build_main.yml' + const NEW_RUN_GRACE_MINUTES = 5 + const runGraceSince = Date.now() - NEW_RUN_GRACE_MINUTES * 60 * 1000 + + const prPageQuery = ` + query($owner: String!, $repo: String!, $size: Int!, $cursor: String, $appId: Int!) { + repository(owner: $owner, name: $repo) { + pullRequests(states: OPEN, first: $size, after: $cursor, + orderBy: {field: UPDATED_AT, direction: DESC}) { + pageInfo { hasNextPage endCursor } + nodes { + number + updatedAt + headRefOid + statusCheckRollup { + commit { + oid + checkSuites(first: 10, filterBy: {appId: $appId}) { + pageInfo { hasNextPage } + nodes { + checkRuns(first: 10, + filterBy: {checkName: "Build", checkType: LATEST}) { + pageInfo { hasNextPage } + nodes { databaseId status conclusion title summary text } + } + } + } + } + } + headRef { + name + repository { name owner { login } } + target { + oid + ... on Commit { + checkSuites(first: 10, filterBy: {appId: $appId}) { + pageInfo { hasNextPage } + nodes { + status + conclusion + branch { name } + workflowRun { databaseId file { path } } + } + } + } + } + } + } + } + } + }` + + // Yield the open PRs updated since updatedSince one page at a time, most recently + // updated first, pausing after each page as long as it took (see above). A PR + // updated during the pass moves to the front and may be missed, but it is then + // recent enough for the next pass. + async function* listOpenPRs() { + let cursor = null + for (;;) { + const started = Date.now() + const data = await github.graphql(prPageQuery, { + ...context.repo, + size: PR_PAGE_SIZE, + cursor, + appId: GITHUB_ACTIONS_APP_ID + }) + const took = Date.now() - started + const page = data.repository.pullRequests + const recent = page.nodes.filter(node => + Date.parse(node.updatedAt) >= updatedSince) + yield recent + if (recent.length < page.nodes.length || !page.pageInfo.hasNextPage) { + return + } + cursor = page.pageInfo.endCursor + await new Promise(resolve => setTimeout(resolve, took)) + } + } + + // Flatten a PR from a page into its number, updatedAt, head sha, fork owner and repo, + // and: + // - buildChecks: the head commit's Build checks. + // - forkRuns: the fork's workflow runs for the head commit, by run ID, and + // forkRunsTruncated: whether the page held only some of them. + // - buildRun: the fork's newest build_main.yml run for the PR branch, if any. The + // branch matters: build_main.yml skips fork pushes to master ("Sync fork"), so the + // same commit can also carry a skipped run on another branch. + // Statuses and conclusions are lowercased to match the check-runs REST API. + // Return null if the fork branch is gone or not at the PR head commit (the branch + // was just pushed to and the PR has yet to follow). Throw if the page does not hold + // all of the PR's Build checks. + const toPR = (node) => { + const target = node.headRef?.target + const commit = node.statusCheckRollup?.commit + if (target?.oid != node.headRefOid || (commit && commit.oid != node.headRefOid)) { + return null + } + // A commit without any checks or statuses has no rollup. + const suites = commit?.checkSuites.nodes ?? [] + if (commit?.checkSuites.pageInfo.hasNextPage + || suites.some(s => s.checkRuns.pageInfo.hasNextPage)) { + throw new Error('The PR head commit has more check suites or Build checks ' + + 'than a page holds') + } + const lower = (value) => value?.toLowerCase() + const runs = target.checkSuites.nodes.filter(s => s.workflowRun).map(s => ({ + id: s.workflowRun.databaseId, + status: lower(s.status), + conclusion: lower(s.conclusion), + workflow: s.workflowRun.file?.path, + branch: s.branch?.name + })) + return { + number: node.number, + updatedAt: node.updatedAt, + sha: node.headRefOid, + forkOwner: node.headRef.repository.owner.login, + forkRepo: node.headRef.repository.name, + buildChecks: suites.flatMap(s => s.checkRuns.nodes).map(cr => ({ + id: cr.databaseId, + status: lower(cr.status), + conclusion: lower(cr.conclusion), + output: {title: cr.title, summary: cr.summary, text: cr.text} + })), + forkRuns: new Map(runs.map(run => [String(run.id), run])), + forkRunsTruncated: target.checkSuites.pageInfo.hasNextPage, + buildRun: runs.filter(run => run.workflow == BUILD_WORKFLOW + && run.branch == node.headRef.name).pop() + } + } + + // The fork run ID a Build check points at, stored as JSON in its output text, or + // undefined if the text is not that JSON (e.g. a check from an older version, a + // manual run, or another app). Review Comment: **Nit (P3):** A Build check from another app can't reach this parser anymore: the query only reads check suites filtered to `GITHUB_ACTIONS_APP_ID` (lines 72 and 119), and `pr.buildChecks` is built from those suites alone. The example comes from the old `parseRunParams` comment, which was accurate when the REST listing returned every app's check runs. ```suggestion // manual run). ``` **Verification:** - **Inspection:** The comment's list of non-JSON sources matches the inputs the app-filtered head-commit query can return. ########## .github/workflows/update_build_status.js: ########## @@ -0,0 +1,393 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +module.exports = async ({github, context, core, env = process.env, log = console}) => { + // Open PRs are read with one GraphQL query per page of PRs. For each PR it returns + // the Build checks on the head commit (statusCheckRollup.commit) and the fork's + // workflow runs for that commit (headRef.target), both as GitHub Actions check + // suites: fork runs are Actions runs, and Build checks written with the GITHUB_TOKEN + // land in Actions suites too. + // + // A PR rarely has more than a few such suites. A fork's default branch can collect + // thousands from scheduled workflows, but suites are listed oldest first and a push + // creates its runs' suites right away, so they are still among the first. Should a + // page ever not hold everything a PR needs, the PR fails loudly rather than being + // synced from partial data. + // + // GitHub aborts a GraphQL request after 10 seconds and allows 60 seconds of GraphQL + // CPU time per minute. A page of 25 PRs takes about 3 seconds, so each page is + // followed by a pause as long as it took. + // https://docs.github.com/en/graphql/overview/rate-limits-and-query-limits-for-the-graphql-api + const GITHUB_ACTIONS_APP_ID = 15368 + const PR_PAGE_SIZE = 25 + + // Quick syncs cover only open PRs updated in the last QUICK_SYNC_LOOKBACK_DAYS. A + // push, comment, review, or label change updates a PR, but a check change does not. + // Fork runs usually finish within hours of a push, and most reruns happen within + // days. A later rerun, or a gap in quick syncs longer than the lookback, is picked + // up by the next full sync, which covers all open PRs. Both are scheduled at the + // schedule in the workflow. + const QUICK_SYNC_LOOKBACK_DAYS = 3 + const fullSync = env.FULL_SYNC == 'true' + const updatedSince = fullSync ? 0 + : Date.now() - QUICK_SYNC_LOOKBACK_DAYS * 24 * 60 * 60 * 1000 + + // A missing Build check points at the fork's build_main.yml run for the PR branch. + // Right after a push that run may not be registered yet, so a PR updated in the last + // NEW_RUN_GRACE_MINUTES without a run is left for a later pass instead of being + // told that no run exists. + const BUILD_WORKFLOW = '.github/workflows/build_main.yml' + const NEW_RUN_GRACE_MINUTES = 5 + const runGraceSince = Date.now() - NEW_RUN_GRACE_MINUTES * 60 * 1000 + + const prPageQuery = ` + query($owner: String!, $repo: String!, $size: Int!, $cursor: String, $appId: Int!) { + repository(owner: $owner, name: $repo) { + pullRequests(states: OPEN, first: $size, after: $cursor, + orderBy: {field: UPDATED_AT, direction: DESC}) { + pageInfo { hasNextPage endCursor } + nodes { + number + updatedAt + headRefOid + statusCheckRollup { + commit { + oid + checkSuites(first: 10, filterBy: {appId: $appId}) { + pageInfo { hasNextPage } + nodes { + checkRuns(first: 10, + filterBy: {checkName: "Build", checkType: LATEST}) { + pageInfo { hasNextPage } + nodes { databaseId status conclusion title summary text } + } + } + } + } + } + headRef { + name + repository { name owner { login } } + target { + oid + ... on Commit { + checkSuites(first: 10, filterBy: {appId: $appId}) { + pageInfo { hasNextPage } + nodes { + status + conclusion + branch { name } + workflowRun { databaseId file { path } } + } + } + } + } + } + } + } + } + }` + + // Yield the open PRs updated since updatedSince one page at a time, most recently + // updated first, pausing after each page as long as it took (see above). A PR + // updated during the pass moves to the front and may be missed, but it is then + // recent enough for the next pass. + async function* listOpenPRs() { + let cursor = null + for (;;) { + const started = Date.now() + const data = await github.graphql(prPageQuery, { + ...context.repo, + size: PR_PAGE_SIZE, + cursor, + appId: GITHUB_ACTIONS_APP_ID + }) + const took = Date.now() - started + const page = data.repository.pullRequests + const recent = page.nodes.filter(node => + Date.parse(node.updatedAt) >= updatedSince) + yield recent + if (recent.length < page.nodes.length || !page.pageInfo.hasNextPage) { + return + } + cursor = page.pageInfo.endCursor + await new Promise(resolve => setTimeout(resolve, took)) + } + } + + // Flatten a PR from a page into its number, updatedAt, head sha, fork owner and repo, + // and: + // - buildChecks: the head commit's Build checks. + // - forkRuns: the fork's workflow runs for the head commit, by run ID, and + // forkRunsTruncated: whether the page held only some of them. + // - buildRun: the fork's newest build_main.yml run for the PR branch, if any. The + // branch matters: build_main.yml skips fork pushes to master ("Sync fork"), so the + // same commit can also carry a skipped run on another branch. + // Statuses and conclusions are lowercased to match the check-runs REST API. + // Return null if the fork branch is gone or not at the PR head commit (the branch + // was just pushed to and the PR has yet to follow). Throw if the page does not hold + // all of the PR's Build checks. + const toPR = (node) => { + const target = node.headRef?.target + const commit = node.statusCheckRollup?.commit + if (target?.oid != node.headRefOid || (commit && commit.oid != node.headRefOid)) { + return null + } + // A commit without any checks or statuses has no rollup. + const suites = commit?.checkSuites.nodes ?? [] + if (commit?.checkSuites.pageInfo.hasNextPage + || suites.some(s => s.checkRuns.pageInfo.hasNextPage)) { + throw new Error('The PR head commit has more check suites or Build checks ' + + 'than a page holds') + } + const lower = (value) => value?.toLowerCase() + const runs = target.checkSuites.nodes.filter(s => s.workflowRun).map(s => ({ + id: s.workflowRun.databaseId, + status: lower(s.status), + conclusion: lower(s.conclusion), + workflow: s.workflowRun.file?.path, + branch: s.branch?.name + })) + return { + number: node.number, + updatedAt: node.updatedAt, + sha: node.headRefOid, + forkOwner: node.headRef.repository.owner.login, + forkRepo: node.headRef.repository.name, + buildChecks: suites.flatMap(s => s.checkRuns.nodes).map(cr => ({ + id: cr.databaseId, + status: lower(cr.status), + conclusion: lower(cr.conclusion), + output: {title: cr.title, summary: cr.summary, text: cr.text} + })), + forkRuns: new Map(runs.map(run => [String(run.id), run])), + forkRunsTruncated: target.checkSuites.pageInfo.hasNextPage, + buildRun: runs.filter(run => run.workflow == BUILD_WORKFLOW + && run.branch == node.headRef.name).pop() + } + } + + // The fork run ID a Build check points at, stored as JSON in its output text, or + // undefined if the text is not that JSON (e.g. a check from an older version, a + // manual run, or another app). + const runIdOf = (cr) => { + try { + return JSON.parse(cr.output.text).run_id + } catch { + return undefined + } + } + + // The Build check state that shows a fork run's state. The check-runs API accepts + // fewer values than a run reports: the other pending statuses (requested, waiting, + // pending) become queued, and the other conclusions (startup_failure, stale, or + // none), which all mean the run produced no test result, become failure. + const checkState = (run) => { + if (run.status != 'completed') { + return {status: run.status == 'in_progress' ? 'in_progress' : 'queued'} + } + const conclusions = ['success', 'failure', 'neutral', 'cancelled', 'timed_out', + 'action_required', 'skipped'] + return { + status: 'completed', + conclusion: conclusions.includes(run.conclusion) ? run.conclusion : 'failure' + } + } + const label = (state) => state.status == 'completed' ? state.conclusion : state.status + const sameState = (a, b) => a.status == b.status + && (a.status != 'completed' || a.conclusion == b.conclusion) + + // PRs whose sync failed on this pass. A failure skips only that PR, which the next + // pass retries, and fails the step at the end so a recurring error stays visible. + const failedPRs = new Set() + + // The log lists only the Build checks a pass creates or updates, and errors, each + // with its PR and head commit. Everything else is counted for a summary at the end. + const count = {prs: 0, skipped: 0, updated: 0, created: 0, unchanged: 0, notApplied: 0} + const prUrl = `https://github.com/${context.repo.owner}/${context.repo.repo}/pull/` + + log.log(fullSync ? 'Full sync: all open PRs' + : 'Quick sync: open PRs updated in the last ' + QUICK_SYNC_LOOKBACK_DAYS + ' days') + for await (const prs of listOpenPRs()) { + for (const node of prs) { + count.prs++ + const ref = prUrl + node.number + ' ' + node.headRefOid.slice(0, 7) + let pr + try { + pr = toPR(node) + } catch (error) { + log.error(ref + ': ' + error.message + '; raise the limits in prPageQuery') + failedPRs.add(node.number) + continue + } + if (!pr) { + count.skipped++ + continue + } + + // Sync every Build check that points at a fork run, not just the first: a commit + // can carry more than one (e.g. left by the notify workflow this one replaced). + // An action_required check, written when no fork run was found, points at none. + let syncableBuildCheck = false + for (const cr of pr.buildChecks) { + if (cr.conclusion == 'action_required') { + continue + } + const runId = runIdOf(cr) + const run = pr.forkRuns.get(String(runId)) + if (!run) { + if (runId && pr.forkRunsTruncated) { + // The run may be among the fork's check suites the page left out. Keep + // the backfill from replacing the check, and fail the PR to get noticed. + log.error(ref + ': fork run ' + runId + ' not in the first fork check ' + + 'suites; raise the limits in prPageQuery') + failedPRs.add(pr.number) + syncableBuildCheck = true + } + // Otherwise the check's output is unusable or its fork run was deleted, so + // it can never be synced; the backfill below replaces it. + continue + } + syncableBuildCheck = true + + // Leave a check alone if it already shows the run's state, as nearly all do. + const state = checkState(run) + if (sameState(cr, state)) { + count.unchanged++ + continue + } + const change = label(cr) + ' -> ' + label(state) + try { + const {data: updated} = await github.rest.checks.update({ + ...context.repo, + check_run_id: cr.id, + output: cr.output, + ...state + }) + // A successful request can leave the state unchanged, notably when trying + // to reopen a completed check after a rerun. + if (sameState(cr, updated)) { + count.unchanged++ + } else { + log.log(ref + ': ' + label(cr) + ' -> ' + label(updated)) + count.updated++ + } + if (!sameState(updated, state)) { + count.notApplied++ + } + } catch (error) { + log.error(ref + ': ' + change + ' failed: ' + error.message) + failedPRs.add(pr.number) + } + } + + // No syncable Build check: none was created yet, only an action_required one + // exists, or the existing one points at a fork run that is gone. Create one + // from the fork's build_main.yml run for the PR branch (see toPR). A PR opened + // from the fork's master only has a skipped run, which has nothing to show. + if (!syncableBuildCheck) { + const run = pr.buildRun?.conclusion == 'skipped' ? undefined : pr.buildRun Review Comment: **Nit (P3):** While a fork-master `build_main.yml` run is still queued or in progress, its conclusion is `undefined`, so this line keeps it as `run`. If a pass reads the PR between the push and GitHub evaluating the job's `if` (usually seconds, longer when Actions is backlogged), the code below creates a `queued` Build check pointing at that run, and the next pass syncs it to `skipped` (lines 270-282). From then on the check points at a known run, so the backfill never runs again for that commit: the PR keeps a `skipped` check with a "See test results" link instead of the `action_required` guidance this PR adds. Only PRs opened from a fork's `master` are affected. One way to close the gap: when the PR's head branch is the fork's `master` and the run hasn't completed yet, skip the PR on this pass, as the 5-minute grace window does. A later pass then sees `skipped` and writes the guidance check. **Verification:** - **Regression:** Simulate a PR opened from a fork's master across two passes, once for each not-yet-completed suite status: queued, in_progress, and one that maps to queued, such as waiting. In the first pass the build_main.yml suite has that status. In the second it is completed and skipped, and the PR was last updated more than the 5-minute grace window earlier. The first pass creates no Build check, and the second creates exactly one action_required Build check. - **Behavior:** A PR from a non-master fork branch whose build run is queued still gets a Build check created in the queued state that points at that run. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
