Skip to content

Commit 2bdb26b

Browse files
HyukjinKwonYikun
andcommitted
[SPARK-35101][INFRA] Add GitHub status check in PR instead of a comment
### What changes were proposed in this pull request? TL;DR: now it shows green yellow read status of tests instead of relying on a comment in a PR, **see HyukjinKwon#41 for an example**. This PR proposes the GitHub status checks instead of a comment that link to the build (from forked repository) in PRs. This is how it works: 1. **forked repo**: "Build and test" workflow is triggered when you create a branch to create a PR which uses your resources in GitHub Actions. 1. **main repo**: "Notify test workflow" (previously created a comment) now creates a in-progress status (yellow status) as a GitHub Actions check to your current PR. 1. **main repo**: "Update build status workflow" regularly (every 15 mins) checks open PRs, and updates the status of GitHub Actions checks at PRs according to the status of workflows in the forked repositories (status sync). **NOTE** that creating/updating statuses in the PRs is only allowed from the main repo. That's why the flow is as above. ### Why are the changes needed? The GitHub status shows a green although the tests are running, which is confusing. ### Does this PR introduce _any_ user-facing change? No, dev-only. ### How was this patch tested? Manually tested at: - HyukjinKwon#41 - HyukjinKwon#42 - HyukjinKwon#43 - HyukjinKwon#37 **queued**: <img width="861" alt="Screen Shot 2021-04-16 at 10 56 03 AM" src="https://user-images.githubusercontent.com/6477701/114960831-c9a73080-9ea2-11eb-8442-ddf3f6008a45.png"> **in progress**: <img width="871" alt="Screen Shot 2021-04-16 at 12 14 39 PM" src="https://user-images.githubusercontent.com/6477701/114966359-59ea7300-9ead-11eb-98cb-1e63323980ad.png"> **passed**: ![Screen Shot 2021-04-16 at 2 04 07 PM](https://user-images.githubusercontent.com/6477701/114974045-a12c3000-9ebc-11eb-9be5-653393a863e6.png) **failure**: ![Screen Shot 2021-04-16 at 10 46 10 PM](https://user-images.githubusercontent.com/6477701/115033584-90ec7300-9f05-11eb-8f2e-0fc2ef986a70.png) Closes #32193 from HyukjinKwon/update-checks-pr-poc. Lead-authored-by: HyukjinKwon <gurwls223@apache.org> Co-authored-by: Hyukjin Kwon <gurwls223@apache.org> Co-authored-by: Yikun Jiang <yikunkero@gmail.com> Signed-off-by: HyukjinKwon <gurwls223@apache.org>
1 parent 94849af commit 2bdb26b

File tree

3 files changed

+171
-19
lines changed

3 files changed

+171
-19
lines changed

.github/workflows/labeler.yml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,18 @@
1717
# under the License.
1818
#
1919

20-
name: "Pull Request Labeler"
20+
# Intentionally has a general name.
21+
# because the test status check created in GitHub Actions
22+
# currently randomly picks any associated workflow.
23+
# So, the name was changed to make sense in that context too.
24+
# See also https://github.saobby.my.eu.orgmunity/t/specify-check-suite-when-creating-a-checkrun/118380/10
25+
26+
name: "On pull requests"
2127
on: pull_request_target
2228

2329
jobs:
2430
label:
31+
name: Label pull requests
2532
runs-on: ubuntu-latest
2633
steps:
2734
# In order to get back the negated matches like in the old config,
Lines changed: 68 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,35 @@
1-
name: Notify test workflow
1+
#
2+
# Licensed to the Apache Software Foundation (ASF) under one
3+
# or more contributor license agreements. See the NOTICE file
4+
# distributed with this work for additional information
5+
# regarding copyright ownership. The ASF licenses this file
6+
# to you under the Apache License, Version 2.0 (the
7+
# "License"); you may not use this file except in compliance
8+
# with the License. You may obtain a copy of the License at
9+
#
10+
# http://www.apache.org/licenses/LICENSE-2.0
11+
#
12+
# Unless required by applicable law or agreed to in writing,
13+
# software distributed under the License is distributed on an
14+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
# KIND, either express or implied. See the License for the
16+
# specific language governing permissions and limitations
17+
# under the License.
18+
#
19+
20+
# Intentionally has a general name.
21+
# because the test status check created in GitHub Actions
22+
# currently randomly picks any associated workflow.
23+
# So, the name was changed to make sense in that context too.
24+
# See also https://github.saobby.my.eu.orgmunity/t/specify-check-suite-when-creating-a-checkrun/118380/10
25+
name: On pull request update
226
on:
327
pull_request_target:
428
types: [opened, reopened, synchronize]
529

630
jobs:
731
notify:
32+
name: Notify test workflow
833
runs-on: ubuntu-20.04
934
steps:
1035
- name: "Notify test workflow"
@@ -13,28 +38,53 @@ jobs:
1338
with:
1439
github-token: ${{ secrets.GITHUB_TOKEN }}
1540
script: |
16-
const endpoint = "GET /repos/:owner/:repo/actions/workflows/:id/runs?&branch=:branch"
41+
const endpoint = 'GET /repos/:owner/:repo/actions/workflows/:id/runs?&branch=:branch'
42+
43+
// TODO: Should use pull_request.user and pull_request.user.repos_url?
44+
// If a different person creates a commit to another forked repo,
45+
// it wouldn't be able to detect.
1746
const params = {
1847
owner: context.payload.pull_request.head.repo.owner.login,
1948
repo: context.payload.pull_request.head.repo.name,
20-
id: "build_and_test.yml",
49+
id: 'build_and_test.yml',
2150
branch: context.payload.pull_request.head.ref,
2251
}
2352
53+
console.log('Ref: ' + context.payload.pull_request.head.ref)
54+
console.log('SHA: ' + context.payload.pull_request.head.sha)
55+
56+
// Wait 3 seconds to make sure the fork repository triggered a workflow.
57+
await new Promise(r => setTimeout(r, 3000))
58+
2459
const runs = await github.request(endpoint, params)
25-
var runID = runs.data.workflow_runs[0].id
26-
27-
var msg = "**[Test build #" + runID + "]"
28-
+ "(https://github.com/" + context.payload.pull_request.head.repo.full_name
29-
+ "/actions/runs/" + runID + ")** "
30-
+ "for PR " + context.issue.number
31-
+ " at commit [`" + context.payload.pull_request.head.sha.substring(0, 7) + "`]"
32-
+ "(https://github.com/" + context.payload.pull_request.head.repo.full_name
33-
+ "/commit/" + context.payload.pull_request.head.sha + ")."
34-
35-
github.issues.createComment({
36-
issue_number: context.issue.number,
37-
owner: context.payload.repository.owner.login,
38-
repo: context.payload.repository.name,
39-
body: msg
60+
const runID = runs.data.workflow_runs[0].id
61+
// TODO: If no workflows were found, it's likely GitHub Actions was not enabled
62+
63+
if (runs.data.workflow_runs[0].head_sha != context.payload.pull_request.head.sha) {
64+
throw new Error('There was a new unsynced commit pushed. Please retrigger the workflow.');
65+
}
66+
67+
const runUrl = 'https://github.com/'
68+
+ context.payload.pull_request.head.repo.full_name
69+
+ '/actions/runs/'
70+
+ runID
71+
72+
const name = 'Build and test'
73+
const head_sha = context.payload.pull_request.head.sha
74+
const status = 'queued'
75+
76+
github.checks.create({
77+
...context.repo,
78+
name,
79+
head_sha,
80+
status,
81+
output: {
82+
title: 'Test results',
83+
summary: runUrl,
84+
text: JSON.stringify({
85+
owner: context.payload.pull_request.head.repo.owner.login,
86+
repo: context.payload.pull_request.head.repo.name,
87+
run_id: runID
88+
})
89+
}
4090
})
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
#
2+
# Licensed to the Apache Software Foundation (ASF) under one
3+
# or more contributor license agreements. See the NOTICE file
4+
# distributed with this work for additional information
5+
# regarding copyright ownership. The ASF licenses this file
6+
# to you under the Apache License, Version 2.0 (the
7+
# "License"); you may not use this file except in compliance
8+
# with the License. You may obtain a copy of the License at
9+
#
10+
# http://www.apache.org/licenses/LICENSE-2.0
11+
#
12+
# Unless required by applicable law or agreed to in writing,
13+
# software distributed under the License is distributed on an
14+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
# KIND, either express or implied. See the License for the
16+
# specific language governing permissions and limitations
17+
# under the License.
18+
#
19+
20+
name: Update build status workflow
21+
22+
on:
23+
schedule:
24+
- cron: "*/15 * * * *"
25+
26+
jobs:
27+
update:
28+
name: Update build status
29+
runs-on: ubuntu-20.04
30+
steps:
31+
- name: "Update build status"
32+
uses: actions/github-script@v3
33+
with:
34+
github-token: ${{ secrets.GITHUB_TOKEN }}
35+
script: |
36+
const endpoint = 'GET /repos/:owner/:repo/pulls?state=:state'
37+
const params = {
38+
owner: context.repo.owner,
39+
repo: context.repo.repo,
40+
state: 'open'
41+
}
42+
43+
// See https://docs.github.com/en/graphql/reference/enums#mergestatestatus
44+
const maybeReady = ['behind', 'clean', 'draft', 'has_hooks', 'unknown', 'unstable'];
45+
46+
// Iterate open PRs
47+
for await (const prs of github.paginate.iterator(endpoint,params)) {
48+
// Each page
49+
for await (const pr of prs.data) {
50+
console.log('SHA: ' + pr.head.sha)
51+
console.log(' Mergeable status: ' + pr.mergeable_state)
52+
if (pr.mergeable_state == null || maybeReady.includes(pr.mergeable_state)) {
53+
const checkRuns = await github.request('GET /repos/{owner}/{repo}/commits/{ref}/check-runs', {
54+
owner: context.repo.owner,
55+
repo: context.repo.repo,
56+
ref: pr.head.sha
57+
})
58+
59+
// Iterator GitHub Checks in the PR
60+
for await (const cr of checkRuns.data.check_runs) {
61+
if (cr.name == 'Build and test') {
62+
// text contains parameters to make request in JSON.
63+
const params = JSON.parse(cr.output.text)
64+
65+
// Get the workflow run in the forked repository
66+
const run = await github.request('GET /repos/{owner}/{repo}/actions/runs/{run_id}', params)
67+
68+
// Keep syncing the status of the checks
69+
if (run.data.status == 'completed') {
70+
console.log(' Run ' + cr.id + ': set status (' + run.data.status + ') and conclusion (' + run.data.conclusion + ')')
71+
const response = await github.request('PATCH /repos/{owner}/{repo}/check-runs/{check_run_id}', {
72+
owner: context.repo.owner,
73+
repo: context.repo.repo,
74+
check_run_id: cr.id,
75+
output: cr.output,
76+
status: run.data.status,
77+
conclusion: run.data.conclusion
78+
})
79+
} else {
80+
console.log(' Run ' + cr.id + ': set status (' + run.data.status + ')')
81+
const response = await github.request('PATCH /repos/{owner}/{repo}/check-runs/{check_run_id}', {
82+
owner: context.repo.owner,
83+
repo: context.repo.repo,
84+
check_run_id: cr.id,
85+
output: cr.output,
86+
status: run.data.status,
87+
})
88+
}
89+
90+
break
91+
}
92+
}
93+
}
94+
}
95+
}

0 commit comments

Comments
 (0)