Skip to content

Commit

Permalink
Merge pull request #29 from uclahs-cds/nwiltsie-slash-command-workflow
Browse files Browse the repository at this point in the history
Allow Nextflow tests workflow to self-fix with /fix-tests comment
  • Loading branch information
nwiltsie authored Apr 2, 2024
2 parents 48de7af + e7c93c7 commit da23bc9
Show file tree
Hide file tree
Showing 6 changed files with 493 additions and 31 deletions.
352 changes: 347 additions & 5 deletions .github/workflows/nextflow-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,100 @@ name: 'Nextflow tests'
on:
workflow_call:

concurrency:
group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.sha }}
cancel-in-progress: true

# Inspired by https://blog.aspect.dev/github-actions-dynamic-matrix
jobs:
discover:
runs-on: ubuntu-latest
steps:
- uses: actions/github-script@v7
name: Parse event
id: parse-event
with:
script: |
/*
We need to determine if the incoming event was from a pull_request
or an issue_comment
*/
let apply_fixes = context.eventName == 'issue_comment'
let pr = {}
let abort_early = false
if (apply_fixes) {
// Actions bot ID - see https://api.github.com/users/github-actions%5Bbot%5D
let bot_id = 41898282
let found_review = false
// Make sure that changes are actually requested for this PR
for await (const response of github.paginate.iterator(
github.rest.pulls.listReviews,
{
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: context.payload.issue.number
}
)) {
if (found_review) break
for (const review of response.data) {
if (review.user.id == bot_id && review.state == "CHANGES_REQUESTED") {
found_review = true
break
}
}
}
if (found_review) {
// React to the original comment
await github.rest.reactions.createForIssueComment({
owner: context.repo.owner,
repo: context.repo.repo,
comment_id: context.payload.comment.id,
content: "rocket"
})
} else {
// Post a comment and fail
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.payload.issue.number,
body: `*Bleep bloop, I am a robot.*
You [requested](${ context.payload.comment.html_url }) that I fix the tests, but I can only do so after posting a comment _saying_ that I can do so.
`
})
// Mark that everything should stop
abort_early = true
}
// Get the pull request context
pr = (await github.rest.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: context.payload.issue.number
})).data
} else {
pr = context.payload.pull_request
}
core.setOutput('abort_early', abort_early)
core.setOutput('apply_fixes', apply_fixes)
core.setOutput('checkout_ref', `refs/pull/${pr.number}/merge`)
core.setOutput('base_ref', pr.base.ref)
core.setOutput('branch_ref', pr.head.ref)
core.setOutput('review_sha', pr.head.sha)
core.setOutput('pr_number', pr.number)
- uses: actions/checkout@v4
with:
ref: ${{ steps.parse-event.outputs.checkout_ref }}

- id: listfiles
shell: python
Expand All @@ -30,11 +118,18 @@ jobs:
outputs:
testfiles: ${{ steps.listfiles.outputs.testfiles }}
num_tests: ${{ steps.listfiles.outputs.num_tests }}
apply_fixes: ${{ steps.parse-event.outputs.apply_fixes }}
checkout_ref: ${{ steps.parse-event.outputs.checkout_ref }}
base_ref: ${{ steps.parse-event.outputs.base_ref }}
branch_ref: ${{ steps.parse-event.outputs.branch_ref }}
review_sha: ${{ steps.parse-event.outputs.review_sha }}
pr_number: ${{ steps.parse-event.outputs.pr_number }}
abort_early: ${{ steps.parse-event.outputs.abort_early }}

run:
runs-on: ubuntu-latest
needs: discover
if: ${{ fromJSON(needs.discover.outputs.num_tests) }}
if: ${{ fromJSON(needs.discover.outputs.num_tests) && needs.discover.outputs.abort_early == 'false' }}

strategy:
fail-fast: false
Expand All @@ -44,6 +139,7 @@ jobs:
steps:
- uses: actions/checkout@v4
with:
ref: ${{ needs.discover.outputs.checkout_ref }}
submodules: true

- uses: docker/login-action@v3
Expand Down Expand Up @@ -73,11 +169,257 @@ jobs:

summary:
runs-on: ubuntu-latest
needs: run
if: ${{ !cancelled() }}
needs: [run, discover]
if: ${{ !cancelled() && needs.discover.result == 'success' && needs.discover.outputs.abort_early == 'false' }}

steps:
- uses: actions/github-script@v7
if: ${{ needs.run.result != 'success' && needs.run.result != 'skipped' }}
id: status
name: Determine required steps
with:
script: |
let needs = ${{ toJSON(needs) }}
let tests_failed = needs.run.result != 'success' &&
needs.run.result != 'skipped'
let should_comment = tests_failed &&
(needs.discover.outputs.apply_fixes != 'true')
let should_fix = tests_failed &&
(needs.discover.outputs.apply_fixes == 'true')
// Find the most recent open review from the bot, as well as all of
// the comments
let review_id = -1
let comments = []
/*
As far as I can tell, this is fixed... I wish I could figure out
how to infer this from the token. The users.getAuthenticated()
endpoint throws "HttpError: Resource not accessible by integration",
and GITHUB_ACTOR / GITHUB_ACTOR_ID / GITHUB_TRIGGERING_ACTOR refer
to the real user (not the bot).
*/
let bot_id = 41898282
for await (const response of github.paginate.iterator(
github.rest.pulls.listReviews,
{
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: needs.discover.outputs.pr_number
}
)) {
for (const review of response.data) {
if (review.user.id == bot_id) {
comments.push(review.body)
console.log(`Prior review: ${review.body}`)
if (review.state == "CHANGES_REQUESTED") {
// Save the newest review ID
review_id = review.id
} else if (review.state == "DISMISSED") {
// Dismissals implicitly dismiss all prior reviews
review_id = -1
}
}
}
}
if (!tests_failed && review_id != -1) {
// There is an open review but the problems are resolved
await github.rest.pulls.dismissReview({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: needs.discover.outputs.pr_number,
review_id: review_id,
message: "All tests are passing!"
})
}
core.setOutput('tests-failed', tests_failed)
core.setOutput('should-comment', should_comment)
core.setOutput('should-fix', should_fix)
core.setOutput('comments', comments)
- if: ${{ steps.status.outputs.tests-failed == 'true' }}
uses: actions/checkout@v4
with:
ref: ${{ needs.discover.outputs.checkout_ref }}
# By default the depth is 1, which only fetches the current commit
# with no history. Instead fetch all history so we can compare it to
# the base branch. There doesn't seem to be any middle ground ("get
# everything between the two commits").
fetch-depth: 0
# If we're going to make commits, use a different token
token: ${{ steps.status.outputs.should-fix == 'true' && secrets.UCLAHS_CDS_REPO_READ_TOKEN || secrets.GITHUB_TOKEN }}

- if: ${{ steps.status.outputs.tests-failed == 'true' }}
uses: actions/download-artifact@v4
with:
merge-multiple: true

- if: ${{ steps.status.outputs.tests-failed == 'true' }}
id: check-fixability
name: Confirm tests can be fixed
uses: actions/github-script@v7
with:
script: |
const file_patterns = ['configtest*.json', '**/configtest*.json']
// Get the list of all changed test files
const updated_tests = (await exec.getExecOutput(
'git',
['ls-files', '--modified', '--', ...file_patterns],
)).stdout.trim().split(/\r?\n/)
let branch_sha = ${{ toJSON(needs.discover.outputs.review_sha) }}
/*
If any of those files were changed between the branch tip and the
merge commit, then they must have been changed in the main branch.
*/
try {
await exec.exec('git',
[
'diff-tree',
'HEAD',
branch_sha,
'--exit-code',
'--name-status',
'--',
...updated_tests
]
)
} catch (error) {
let prefix = "This is embarrassing..."
let comments = ${{ toJSON(steps.status.outputs.comments) }}
if (!comments.length || !comments[comments.length-1].includes(prefix)) {
let base_ref = ${{ toJSON(needs.discover.outputs.base_ref) }}
let branch_ref = ${{ toJSON(needs.discover.outputs.branch_ref) }}
github.rest.pulls.createReview({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: ${{ toJSON(needs.discover.outputs.pr_number) }},
commit_id: ${{ toJSON(needs.discover.outputs.review_sha) }},
event: "REQUEST_CHANGES",
body: `*Bleep bloop, I am a robot.*
${prefix} the \`${ base_ref }\` branch has test changes that haven't been merged into _this_ branch, and I can't handle that. Please fix that and then I can fix the tests for you!
\`\`\`console
cd /path/to/your/repository/
git checkout ${ branch_ref }
git fetch origin
git merge origin/${ base_ref }
git push origin
\`\`\``
})
} else {
console.log("Review state not changed")
}
core.setFailed("Need to merge tests")
}
- if: ${{ steps.status.outputs.should-fix == 'true' }}
id: pushchanges
uses: stefanzweifel/git-auto-commit-action@v5
with:
commit_message: Autofix Nextflow configuration regression tests
branch: ${{ needs.discover.outputs.branch_ref }}
file_pattern: '**/configtest*.json'

- if: ${{ steps.status.outputs.should-fix == 'true' }}
name: Add comment about reviewing
uses: actions/github-script@v7
with:
script: core.setFailed('Tests failed!')
script: |
const timers = require('node:timers/promises')
let commit_id = ${{ toJSON(steps.pushchanges.outputs.commit_hash) }}
let review_details = {
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: ${{ toJSON(needs.discover.outputs.pr_number) }},
commit_id: commit_id,
event: "COMMENT",
body: `*Bleep bloop, I am a robot.*
I have updated all of the failing tests for you with ${ commit_id }. You must review my work before merging this pull request!`
}
// We need to trigger GitHub to actually compute the changes
// https://docs.github.com/en/rest/guides/using-the-rest-api-to-interact-with-your-git-database
for (let i=0; i<10; i++) {
if (i != 0) {
await timers.setTimeout(3000)
console.log(`Waited ${i*3} seconds...`)
}
let updated_pr = (await github.rest.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: ${{ toJSON(needs.discover.outputs.pr_number) }}
})).data
// The SHA must change and the new commit must have settled
if ((updated_pr.head.sha != ${{ toJSON(needs.discover.outputs.review_sha) }})
&&
(updated_pr.mergeable != null)) {
break
}
}
await github.rest.pulls.createReview(review_details)
- if: ${{ steps.status.outputs.should-comment == 'true' }}
id: createreview
name: Post review
uses: actions/github-script@v7
with:
script: |
const fsPromises = require('fs/promises')
// Get the list of all annotation files
const annotation_files = (await exec.getExecOutput(
'git',
['ls-files', '--others', '--exclude-standard', '--', '*.prnote'],
)).stdout.trim().split(/\r?\n/)
let prefix = "Alas, some of the Nextflow configuration tests failed"
let comments = ${{ toJSON(steps.status.outputs.comments) }}
if (!comments.length || !comments[comments.length-1].includes(prefix)) {
let body = `*Bleep bloop, I am a robot.*\n\n${ prefix }!`
const notes = await Promise.all(annotation_files.map(filename =>
fsPromises.readFile(filename, {encoding: 'utf-8'})
))
let suffix = `If the above changes are surprising, **stop** and determine what happened.
If the above changes are expected, there are two ways to fix this:
1. **Automatically**: Post a comment starting with "/fix-tests" (without the quotes) and I will update the tests for you (you must review my work afterwards).
2. **Manually**: Follow [these steps](https://uclahs-cds.atlassian.net/wiki/spaces/BOUTROSLAB/pages/254115842/Nextflow+Configuration+Regression+Tests#Resolving-Pull-Request-Check-Failures) on Confluence.`
github.rest.pulls.createReview({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: ${{ toJSON(needs.discover.outputs.pr_number) }},
commit_id: ${{ toJSON(needs.discover.outputs.review_sha) }},
event: "REQUEST_CHANGES",
body: [body, ...notes, suffix].join("\n")
})
} else {
console.log("Still can't fix anything")
}
// Make sure the job fails regardless
core.setFailed('Tests failed!')
Loading

0 comments on commit da23bc9

Please sign in to comment.