Triggering extended tests through PR comment: Run extended tests
#15101
Conversation
github-actions
bot
added
the
development-process
|
Looks like it ran checks on the forked branch, I think that is what we want
|
|
Is this ready for review or is there something outstanding for it to be still in draft? |
It should be ready for a review, rebased on the latest main and tested it a bit more. Looks like it works as expected. |
|
When I tested this on my fork, it seems to have run extended tests for my PR without any prompting: |
Hey @alamb, I think the extended test |
|
Thanks @danila-b -- you are totally right. I started a new test and we'll see how that goes |
There was a problem hiding this comment.
I have completed testing in my fork and I agree this works ❤️ - thank you so much @danila-b (see testing PR here alamb#33)
I also think we should add a note to the user docs about this awesome feature: https://datafusion.apache.org/contributor-guide/testing.html#extended-tests
But we can do that as a follow on
THanks again for this great feature and for your patience waiting on review
|
|
||
| jobs: | ||
|
|
||
| run_extended_tests: |
There was a problem hiding this comment.
Can we please add a human summary here of what this does -- something like
| run_extended_tests: | |
| # Starts the extended_tests when someone leaves a `Run extended tests` comment | |
| run_extended_tests: |
It might also be nice to make the comparison case insensitive (so run extended tests would also work)
There was a problem hiding this comment.
Added the comment 👍
There was a problem hiding this comment.
As for the case sensitivity, maybe let's add it in a follow-up PR since changing the triggers will require a whole new round of testing to make sure the condition work as expected again
.github/workflows/extended.yml
Outdated
| @@ -127,4 +145,44 @@ jobs: | |||
| cargo test --features backtrace --profile release-nonlto --test sqllogictests -- --include-sqlite | |||
| cargo clean | |||
|
|
|||
| # If the workflow was triggered by the PR comment we need to manually update check status to display in UI | |||
There was a problem hiding this comment.
Perhaps this can also make a reference to the pr_comment_commands.yml
There was a problem hiding this comment.
Makes sense, will add it to this PR
|
|
||
| name: PR commands | ||
|
|
||
| on: |
There was a problem hiding this comment.
I think take.yml also has a command triggered by a comment. Maybe we can combine them eventually
There was a problem hiding this comment.
Yes indeed, I think they can be combined into one action quite easily in the future
|
|
||
| name: PR commands | ||
|
|
||
| on: |
There was a problem hiding this comment.
I think take.yml also has a command triggered by a comment. Maybe we can combine them eventually
alamb
changed the title
Run extended tests
Happy to help @alamb, I have pushed the comments that you suggested to this PR, it might need another approval to get merged since some actions need to be re-run I think. I don't know what the merge process is but feel free to merge it whenever convenient or get this across the finish line since I will be on vacation for the next couple of weeks 👍 |
|
Thanks again @danila-b ! |
…pache#15101) * Add custom trigger for extended tests * Add workflow dispatch * Try different templating for ref path * Add checkout * Use branch name directly * Add js action to fetch branch name * Post check on PR * Fix owner and repo name * Remove unused vars * Add permissions for check write * Add check updates * Add lighweight test * Uncomment needs * Add comments for new actions
…pache#15101) * Add custom trigger for extended tests * Add workflow dispatch * Try different templating for ref path * Add checkout * Use branch name directly * Add js action to fetch branch name * Post check on PR * Fix owner and repo name * Remove unused vars * Add permissions for check write * Add check updates * Add lighweight test * Uncomment needs * Add comments for new actions
Which issue does this PR close?
extendedtest suite from a PR #14319Rationale for this change
Allows to run extended tests on some PRs if needed, while skipping them by default on other PRs. The user experience of running the extended test suite should be roughly the same (check is shown in the PR actions tab, comment is marked with 🚀 emoji when the tests are triggered for the PR)
What changes are included in this PR?
Adding a new trigger event for extended tests -
workflow_dispatch, and a separate action which is checking all PR comments and issuing this dispatch event for the PR branch when theRun extended testscomment is detected.Are these changes tested?
Only manual tests on the fork (see here), unfortunately I am not aware of any better tests for Github actions.