Skip to content
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.

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

skip_after_successful_duplicate not working as expected #193

Closed
kpturner opened this issue Feb 8, 2022 · 10 comments
Closed

skip_after_successful_duplicate not working as expected #193

kpturner opened this issue Feb 8, 2022 · 10 comments

Comments

@kpturner
Copy link

kpturner commented Feb 8, 2022

I am finding that if I have a workflow with two jobs - one a pre-check job that uses this action, and one that gets skipped if it is deemed superflous, then I am finding that the "skip_after_successful_duplicate" is returning true even if the previous job failed. For example

    - name: Concurrency checks
      id: skip_check
      uses: fkirc/skip-duplicate-actions@master
      with:
        concurrent_skipping: 'same_content_newer'
        cancel_others: 'true'
        skip_after_successful_duplicate: 'true'
        do_not_skip: '["workflow_dispatch", "schedule"]'   

Workflow run 1
pre-job runs successfully
test-job fails

Workflow run 2
pre-job runs successfully. It reports that the same files were run successfully in Workflow run 1
test-job is skipped

Can't work out why. Unless if is checking a specific job rather than the success of the entire workflow

Might better be illustrated with screen shots:

Workflow 1
image

If I click on the link given for the successful run I get to here, where the run was clearly not successful
image

@paescuj
Copy link
Collaborator

paescuj commented Feb 8, 2022

This happens because you've set concurrent_skipping to "same_content" "same_content_newer", not because of skip_after_successful_duplicate.

This is a known issue with the "same_content" option and has been addressed with a newer "same_content_newer" option (see #124 and skip-concurrent-workflow-runs).

@kpturner
Copy link
Author

kpturner commented Feb 8, 2022

Firstly many thanks for the quick reply. However in this case my config for "concurrent_skipping" is already "same_content_newer"

    - name: Concurrency checks
      id: skip_check
      uses: fkirc/skip-duplicate-actions@master
      with:
        concurrent_skipping: 'same_content_newer'
        cancel_others: 'true'
        skip_after_successful_duplicate: 'true'
        do_not_skip: '["workflow_dispatch", "schedule"]'   

@kpturner
Copy link
Author

kpturner commented Feb 8, 2022

Is this because the two runs were running concurrently (well, one started about 15 minutes before the second) and the check was done BEFORE the slightly older run failed?

@paescuj
Copy link
Collaborator

paescuj commented Feb 8, 2022

Is this because the two runs were running concurrently (well, one started about 15 minutes before the second) and the check was done BEFORE the slightly older run failed?

Exactly! In your case, you might want to remove the concurrent_skipping option altogether.

@kpturner
Copy link
Author

kpturner commented Feb 8, 2022

It sounds like it. The trouble is, these tests are slow and expensive so avoiding unnecessary runs are kinda important. However, in the above scenario, where the runs are configured as "checks", the second run is all you see in the PR checks section. So it looks like it passes when in fact it doesn't :(

I might be able to softcode the concurrent_skipping option if I can somehow find out whether or not a PR has passed all the checks previously or not.

@paescuj
Copy link
Collaborator

paescuj commented Feb 8, 2022

Do you mind sharing your workflow configuration? Do you really need to trigger checks concurrently?

@kpturner
Copy link
Author

kpturner commented Feb 8, 2022

I am afraid I cannot - sorry. But the concurrent workflow situation is a very simple one. We have a workflow that runs when a pull request is approved. If two people approve a PR at the same time (or thereabouts) then workflow will run twice. We were using this action to try to skip one of them, but of course that all goes wrong if one of them is skipped THEN the original one fails.

@paescuj
Copy link
Collaborator

paescuj commented Feb 9, 2022

Okay, thanks! As you can see skip-duplicate-actions is working as intended. However, it is a limitation of GitHub that only the result of the latest check run is displayed on the pull request.

You might be able to prevent concurrent runs in the first place. I don't know if this already helps, but one idea might be to only trigger a run on approving pull request reviews, see https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#running-a-workflow-when-a-pull-request-is-approved.

Another idea would be to add an additional job which waits for the older concurrent run to finish and then fail/pass based on it. For this you should be able to use the skipped_by output of skip-duplicate-actions.

@kpturner
Copy link
Author

kpturner commented Feb 9, 2022

Ok thanks - I will investigate those options, although we already employ the technique described in the link. The trouble is the PR becomes "approved" as soon as the approver clicks the button regardless of the outcome of the checks.

@kpturner kpturner closed this as completed Feb 9, 2022
@paescuj
Copy link
Collaborator

paescuj commented Feb 9, 2022

Ok thanks - I will investigate those options, although we already employ the technique described in the link. The trouble is the PR becomes "approved" as soon as the approver clicks the button regardless of the outcome of the checks.

I see!

If you need some help, I'm happy to work out a solution for a small sponsorship. Feel free to ping me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants