canonical / bootstack-actions

Actions for unifying lint, unit and functional tests, and charm and snap releases
0 stars 8 forks source link

Don't re-run func tests on each approval. #19

Closed mkalcok closed 1 year ago

mkalcok commented 1 year ago

I observed that currently each approve in the PR triggers a new run of functional tests even if there are no changes since last approval. I'm not sure if it's possible but it'd be nice if func tests ran only on first "approval" or if there were changes since last approval.

Current flow:

Desired flow:

Of course if there are additional changes made to the PR, func test should be re-ran:

Pjack commented 1 year ago

@agileshaw Is there any sample that we run func test many times without any new commit? I am a little surprised that the func test will be triggered by "Second reviewer gives approval". From the code the behavior doesn't look like this.

I even suspect that we only need to adjust the events then we can avoid unnessary functional tes

agileshaw commented 1 year ago

@Pjack Yes. Taking PR #45 of prometheus-juju-exporter as an example, workflow run #87 and #89 are triggered by opening and updating PR, whereas #90 and #91 are triggered by Robert's and my approval.

Here is the condition code triggering workflows on approval. And the link you shared is for testing inside bootstack-action, not the actual workflow used by other repos.

However, the more I worked on it, the more I believe we should simply remove PR approval as a triggering condition. Instead, the workflows should be triggered only on: PR opens, PR updates, recheck review comment submits. Opening a PR for this approach.