Add skip-duplicate-actions action

[niccoloraspa]

Closes: #1100

Description

This PR improves the test workflow by skipping duplicate actions.
This is done using skip-duplicate-actions.

The improvements made:

1. Avoid running the test workflow twice on pull_request and push events

⚠️ IMPORTANT
Test will ALWAYS run on pull_requests events in any case.

This means that:

If Alice is working on branch A and commits some changes to the code, then the test workflow will be triggered.
If Alice waits for the test workflow to be over (the expected behaviour) and then opens a PR to merge the branch to main, the same test workflow will run again.
This is good because you are sure that any new PR code is always tested, and it is necessary to run a pull_request workflow to let codecov post the report in the comment.

The "downside" of this:

If Alice is working on branch A and commits some changes to the code, then the test workflow will be triggered.
If Alice doesn't wait for the test workflow to be over and opens the PR immediately, two duplicated jobs will run (one for the push event and one for the pull_request event).
This is the only case (which I am aware of) in which there will be duplicated runs of the test workflow.

The good part:

• Any following commits to the PR will only RUN once. The workflow triggered by the push event will be skipped.
• When the PR gets merged, the test workflow is skipped again.

2. Run the workflow only when go files are edited

This is a nice optimization to avoid running tests when it's not needed.
If Alice commits don't change any: *.go *.mod and *.sum, then the test workflow will be skipped.

This is controlled by https://github.com/nikever/osmosis/blob/fix/remove-duplicate-ci-trigger/.github/workflows/test.yml#L26

Am I missing any relevant scenario where we would like to run tests?

In pull_requests tests will always run.

3. Cancel outdated runs

This is another optimization, which is slightly more "aggressive" than the other.

Let me know if you want to keep it:

This is controlled by https://github.com/nikever/osmosis/blob/fix/remove-duplicate-ci-trigger/.github/workflows/test.yml#L23

If Alice is working on branch A and commits some changes to the code, then the test workflow will be triggered.
While the workflow is running, if Alice commits another change to the same branch, the previous test workflow will be cancelled in favour of the new test workflow.

Before merging

I have run multiple tests in my local repository to test different scenarios, but CIs are always tricky, and it would be great if someone from the dev team could confirm that the workflow behaves as expected.

---

For contributor use:

• Targeted PR against correct branch (see CONTRIBUTING.md)
• Updated relevant documentation (docs/) or specification (x/<module>/spec/)
• Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
• Re-reviewed Files changed in the Github PR explorer

[niccoloraspa]

And (as before) with PRs coming from remote forks such as this one now the test workflow is not triggered twice since there are no push events.

I will update the CHANGELOG.md so we can check the behaviour.

[niccoloraspa]

Even if I have modified only the CHANGELOG.md and not any *.go files the workflow still runs as expected.

This might be unnecessary but I think it's good practise.
It is better to be safe and always avoid merging possibly untested code.

[codecov-commenter]

Codecov Report

Merging #1114 (2168fef) into main (dbe2e1e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1114   +/-   ##
=======================================
  Coverage   20.94%   20.94%           
=======================================
  Files         199      199           
  Lines       25429    25429           
=======================================
  Hits         5326     5326           
  Misses      19140    19140           
  Partials      963      963           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dbe2e1e...2168fef. Read the comment docs.

[p0mvn]

In 1) you mentioned a case where it is possible to have 2 duplicate jobs on open PRs.

I see that we have concurrent_skipping: 'same_content'. Does this not help mitigate that duplication?

I'm assuming the above setting is overwritten by do_not_skip: '["pull_request",...] ?

Thanks for such a detailed description btw!

[ValarDragon]

Awesome! Thanks for adding this and the extremely detailed description

[niccoloraspa]

I see that we have concurrent_skipping: 'same_content'. Does this not help mitigate that duplication?

I'm assuming the above setting is overwritten by do_not_skip: '["pull_request",...] ?

It doesn't help unfortunately. You can skip jobs that are triggered simultaneously.
In the case of scenario 1, the events happen sequentually (First you push, then you open the PR).

The solution would be to cancel the first push triggered workflow when a corresponding pull_request triggered workflow runs.

However this is not super simple to do but I don't think this would be necessary since:

1. Ideally, you should wait for the push triggered workflow to complete, before opening a PR

2. Both workflow runs only PR opens and it takes some time to review the PR. Remember that you should always run PR workflow in any case. The time delta increase it's tolerable.

[ValarDragon]

I think the duplicate push run when opening PR isn't worth worrying about

The big value add was improving the situation for every subsequent push, as typically we fix some merge conflict before wanting to hit merge, and thats the flow breaking latency, which this PR solves!