add manual trigger for extended tests in pull requests #14331
Conversation
github-actions
bot
added
the
development-process
|
I took a look at the updates, I think they should work. |
|
Thanks @buraksenn When testing such PRs I normally put them on the |
Thanks @alamb. As you've said I've started testing it in my fork will update here with test result |
buraksenn
changed the title
buraksenn
changed the title
| with: | ||
| github-token: ${{secrets.GITHUB_TOKEN}} | ||
| script: | | ||
| github.rest.issues.createComment({ |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I tried it in my fork, see this PR:
It seems to have worked great
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
@edmondop I may be misunderstanding you, but I think once the |
There was a problem hiding this comment.
This workflow looks great @buraksenn -- thank you and @Omega359 and @edmondop
I just want to test the negative case before approving it
The only other thing I think would be useful is to document how this work, but we can do that as a follow on.
Perhaps we can extend the text introduced in
| with: | ||
| github-token: ${{secrets.GITHUB_TOKEN}} | ||
| script: | | ||
| github.rest.issues.createComment({ |
There was a problem hiding this comment.
I tried it in my fork, see this PR:
It seems to have worked great
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
|
I looked more at what happened with this PR:
Strangely, the workflow seems to have run the extended tests using Here is the workflow: https://github.com/alamb/datafusion/actions/runs/13058528763 Follow to a test Seems to have checked out I may be misreading the job info Possibly related is that the extended tests are not reported on the main PR: |
|
It seems, I've missed that sorry, let me try to fix and then write test steps clearly @alamb |
buraksenn
force-pushed
the
try-manual-triger-for-extended-tests
branch
from
50502dd to
bb918d4
Compare
Which issue does this PR close?
Closes #14319
Rationale for this change
Details in #14319 but main idea is to run extended tests on some PR before merging to verify their integrity before they are merged to main.
What changes are included in this PR?
With this change
run extended testswill trigger extended tests on PR.Are these changes tested?
Pushed these changes to my fork and opened a test PR.
One failed example because of build issues in main:
Successful extended test:
Running:
Action ( https://github.com/buraksenn/datafusion/actions/runs/13020123049):
Finished:
Notification:
