Skip to content

add manual trigger for extended tests in pull requests #14331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

add manual trigger for extended tests in pull requests #14331

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1856d57
draft
buraksenn Jan 27, 2025
f7752ab
Merge branch 'main' into try-manual-triger-for-extended-tests
buraksenn Jan 28, 2025
5130040
give pr write permission
buraksenn Jan 28, 2025
0790b2c
Merge branch 'main' into try-manual-triger-for-extended-tests
buraksenn Jan 29, 2025
bb918d4
explciit dynamic ref for fix
buraksenn Jan 30, 2025
6a44a1f
Merge branch 'main' into try-manual-triger-for-extended-tests
buraksenn Jan 30, 2025
95bb489
Merge remote-tracking branch 'apache/main' into try-manual-triger-for…
alamb Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 91 additions & 0 deletions .github/workflows/extended.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,46 @@ on:
push:
branches:
- main
issue_comment:
types: [created]

permissions:
pull-requests: write

jobs:
# Check issue comment and notify that extended tests are running
check_issue_comment:
name: Check issue comment
runs-on: ubuntu-latest
if: github.event.issue.pull_request && github.event.comment.body == 'run extended tests'
steps:
- uses: actions/github-script@v7
with:
github-token: ${{secrets.GITHUB_TOKEN}}
script: |
github.rest.issues.createComment({
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we can create a check in the PR to show that the running the tests is in progress,

we can use https://octokit.github.io/rest.js/v21/#checks

to create a check if the comment is available,

and when the pipeline runs, we can update the check using the update the check

octokit.rest.checks.update({
        owner,
repo,
check_run_id,
output.summary,
output.annotations[].path,
output.annotations[].start_line,
output.annotations[].end_line,
output.annotations[].annotation_level,
output.annotations[].message,
output.images[].alt,
output.images[].image_url,
actions[].label,
actions[].description,
actions[].identifier
      })

see in the docs that I have linked the "Update a check run" under the checks api

wdyt @alamb ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Using other actions in an apache repo is problematic as I previously found out. However, it's not hard to have an action post a comment and then edit it to show progress.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried it in my fork, see this PR:

It seems to have worked great

Screenshot 2025-01-29 at 12 26 18 PM

Here is the workflow run:
https://github.com/alamb/datafusion/actions/runs/13036912907

I am going to wait to see what happens on success and then I will purposely introduce a failure in extended tests and see what happens

issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: "Running extended tests..."
})

# Check crate compiles and base cargo check passes
linux-build-lib:
name: linux build test
runs-on: ubuntu-latest
container:
image: amd64/rust
if: |
github.event_name == 'push' ||
(github.event_name == 'issue_comment' && github.event.issue.pull_request && github.event.comment.body == 'run extended tests')
steps:
- uses: actions/checkout@v4
with:
# Check out the pull request branch if triggered by a comment
ref: ${{ github.event_name == 'issue_comment' && github.event.issue.pull_request.head.ref || github.ref }}
Copy link
Contributor

Choose a reason for hiding this comment

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

A fully formatted ref should work. Probably there are other ways to fetch the same info as well.

Suggested change
ref: ${{ github.event_name == 'issue_comment' && github.event.issue.pull_request.head.ref || github.ref }}
ref: ${{ github.event_name == 'issue_comment' && format('refs/pull/{0}/head', github.event.issue.number) || github.ref }}

Test PR
Main branch workflow state
Checkout commit matches PR head

Copy link
Contributor

@alamb alamb Mar 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks @danila-b 🙏

🤔

danila-b#3 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently it did run them, it's just not visible in GitHub UI. I have just double checked it with a bit different implementation on the same branch.

Unfortunately, it seems that issue_comment event trigger is associated with default branch, and hence it does not add anything to the checks or UI of the PR itself. I can take a look if there is a better way to launch check associated with PR during the weekend, but from the top of my head, there are a couple of options:

  1. Use workflow_dispatch event and trigger extended tests for the branch from the GitHub actions UI tab instead of doing so by comment on the PR
  2. Use 3rd party action to set check statuses when the run is triggered by a comment
  3. Maybe slash command 3rd party action can also work for triggering commands a bit better since it uses dispatch event under the hood as well

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried messing around with this again this morning and didn't have much luck

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you again for the help -- I am out of my league in terms of github actions.

Copy link
Contributor

@danila-b danila-b Mar 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@alamb I gave it a stab implementing this here without any 3rd party actions: #15101
It should dispatch extended tests on a PR comment, and also create and update the check marker on the PR.

Testing this is a bit of a pain since I don't know any better way than to just merge the workflow to the main branch of the fork and run several test PRs. At least seems to work properly from a quick glance, will test a bit more during the weekend.

Copy link
Contributor

Choose a reason for hiding this comment

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

Testing this is a bit of a pain since I don't know any better way than to just merge the workflow to the main branch of the fork and run several test PRs. At least seems to work properly from a quick glance, will test a bit more during the weekend.

Thank you! I'll try and check this out shortly

submodules: true
fetch-depth: 1
- name: Setup Rust toolchain
uses: ./.github/actions/setup-builder
with:
Expand All @@ -57,9 +87,14 @@ jobs:
runs-on: ubuntu-latest
container:
image: amd64/rust
if: |
github.event_name == 'push' ||
(github.event_name == 'issue_comment' && github.event.issue.pull_request && github.event.comment.body == 'run extended tests')
steps:
- uses: actions/checkout@v4
with:
# Check out the pull request branch if triggered by a comment
ref: ${{ github.event_name == 'issue_comment' && github.event.issue.pull_request.head.ref || github.ref }}
submodules: true
fetch-depth: 1
- name: Setup Rust toolchain
Expand All @@ -77,9 +112,14 @@ jobs:
runs-on: ubuntu-latest
container:
image: amd64/rust
if: |
github.event_name == 'push' ||
(github.event_name == 'issue_comment' && github.event.issue.pull_request && github.event.comment.body == 'run extended tests')
steps:
- uses: actions/checkout@v4
with:
# Check out the pull request branch if triggered by a comment
ref: ${{ github.event_name == 'issue_comment' && github.event.issue.pull_request.head.ref || github.ref }}
submodules: true
fetch-depth: 1
- name: Setup Rust toolchain
Expand All @@ -96,9 +136,14 @@ jobs:
runs-on: ubuntu-latest
container:
image: amd64/rust
if: |
github.event_name == 'push' ||
(github.event_name == 'issue_comment' && github.event.issue.pull_request && github.event.comment.body == 'run extended tests')
steps:
- uses: actions/checkout@v4
with:
# Check out the pull request branch if triggered by a comment
ref: ${{ github.event_name == 'issue_comment' && github.event.issue.pull_request.head.ref || github.ref }}
submodules: true
fetch-depth: 1
- name: Setup Rust toolchain
Expand All @@ -107,3 +152,49 @@ jobs:
rust-version: stable
- name: Run sqllogictest
run: cargo test --profile release-nonlto --test sqllogictests -- --include-sqlite

notify_if_run_on_pr_success:
name: Notify
runs-on: ubuntu-latest
needs:
[
linux-test-extended,
hash-collisions,
sqllogictest-sqlite,
check_issue_comment,
]
if: success()
steps:
- uses: actions/github-script@v7
with:
github-token: ${{secrets.GITHUB_TOKEN}}
script: |
github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: "extended test suite ran successfully on this PR."
})

notify_if_run_on_pr_failure:
name: Notify
runs-on: ubuntu-latest
needs:
[
linux-test-extended,
hash-collisions,
sqllogictest-sqlite,
check_issue_comment,
]
if: failure()
steps:
- uses: actions/github-script@v7
with:
github-token: ${{secrets.GITHUB_TOKEN}}
script: |
github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: "extended test suite failed on this PR."
})
Loading