upsidr / merge-gatekeeper

Get better merge control
MIT License
85 stars 14 forks source link

Merge Gatekeeper considering previous failed runs and failing unexpectedly #25

Closed thiagomajesk closed 2 years ago

thiagomajesk commented 2 years ago

Hi! Just caught this weird behavior today using this action where Merge Gatekeeper is considering previously failed runs and it keeps failing. Look at the log outputs:

Start processing validator: merge-gatekeeper....
Finish validator: merge-gatekeeper processing.
Error: validation failed, err: [10](https://github.com/my-organization/my-repo/runs/5407659157?check_suite_focus=true#step:3:10) out of [12](https://github.com/my-organization/my-repo/runs/5407659157?check_suite_focus=true#step:3:12)

  Total job count:     12
    jobs: ["code-quality" "label" "Validate PR title" "Validate PR title" "label" "label" "Validate PR title" "label" "Validate PR title" "code-quality" "label" "Validate PR title"]
  Completed job count: 10
    jobs: ["label" "Validate PR title" "Validate PR title" "label" "label" "Validate PR title" "label" "Validate PR title" "code-quality" "label"]
  Failed job count:    1
    jobs: ["Validate PR title"]

As you can see, the job log displays all previous runs of the "Validate PR title" job, including the failed ones, which prevents the Merge Gatekeeper job to succeed after we already fixed the problem (I'm using this action to validate PR titles: https://github.com/amannn/action-semantic-pull-request).

Steps to reproduce:

This behavior continues even after closing/ reopening or opening a completely new PR unless I change the commit hash

image

PS.: One of the things that solved it for me was changing the commit history so the "job cache" wouldn't take into consideration previous runs of failed jobs.

rytswd commented 2 years ago

Hi @thiagomajesk - sorry for the late response, and thanks for the report! Merge Gatekeeper checks the PR state based on GitHub API response, and thus I would have thought this would be correctly reported if the job has been rerun and succeeded in the second time. If I understand the situation correctly, this is probably due to the fact that the "Validate PR title" job is not directly connected to a commit per se, and thus the commit reference we use for Merge Gatekeeper does not get the latest state of the "Validate PR title". Probably the same can be said for other non-commit based actions, such as user comments, PR description, etc.

We will need to debug this behaviour further, and report once we have an idea for fixing this behaviour. Thinking out loud right now, I'm not too sure if we have a nice solution for this other than adding some extra query logic for failing jobs to ensure the actual reported state matches the latest state. We will look more closely into this - it would be a great help to hear suggestions if you have any!

rytswd commented 2 years ago

This turned out that there was a bug in the status handling, and we have corrected this with #26 . The new version v1.0.2 has been released with the fix. Thank you for the report, and please feel free to reach out to us if you find anything else!

thiagomajesk commented 2 years ago

Thanks for the feedback @rytswd! You beat me to it. I'll do some testing with the new version. Cheers!