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

Required status check not fulfilled randomly using this action to set the required check #27

Open
alambike opened this issue Nov 17, 2021 · 6 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@alambike
Copy link

alambike commented Nov 17, 2021

We are using this action to set to neutral a check that is marked as required in the branch protections, when there is no code change in the project.

From time to time this setting fails (but the action completes correctly, without error), and the PRs gets stuck, waiting for that required status check to be fulfilled. Sometimes retrying is enough for the process to work properly.

Would it be possible to add a retry to validate that the check has been set correctly? Maybe as an input parameter, configurable by the user. This could be useful also for cases where the Github API returns an unexpected error, or some rate-limit has been reached.

@alambike alambike changed the title Random required status check not fulfilled using this action to set the required check Required status check not fulfilled randomly using this action to set the required check Nov 17, 2021
@LouisBrunner LouisBrunner added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Dec 1, 2021
@LouisBrunner
Copy link
Owner

Hi @alambike,

I think an optional retry system would be pretty valuable for sure!

@piotrekkr
Copy link

We have same problem. Is there any option to make it more verbose to see what happens inside this action?

@LouisBrunner
Copy link
Owner

@piotrekkr I think you should be able to use ACTIONS_STEP_DEBUG (https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/enabling-debug-logging#enabling-step-debug-logging) to see more logging from the action.

All GitHub API failures should be logged as the message of the failure though, so not sure there is more context to be gained?

@alambike
Copy link
Author

Regarding this, we finally took another path, recently indicated in the GitHub documentation, by using a fallback workflow that fulfills the required status check, in case the real job has not been triggered, because the necessary conditions have not been met (no changes of code for example).

@piotrekkr
Copy link

piotrekkr commented Sep 1, 2022

@LouisBrunner

I wrote to GitHub Support and after few weeks I got answer and some help with this. In general they know that Checks API is not working properly with GitHub Action because GitHub Actions were not designed for this, GitHub apps are for this:

Thank you for reaching out to GitHub Support! We apologize for the delayed response here.

At this time, it is heavily discouraged to create or update checks during runtime of a GitHub Actions workflow run.

The available Checks API were designed specifically for use with GitHub Apps, so generating or updating checks at runtime via Actions can lead to unexpected behavior with how status results are reported to PRs and commits.

Our engineering team is aware of the end-user desired functionality that comes with using the Checks API with Actions, and are looking into implementing features that are designed for these functionalities specifically.

Please let me know if you have any additional questions, or if there is anything further we can assist with. Thanks!

Then I explained my exact problem to them:

We created this check because GitHub was behaving (in our opinion) incorrectly, with how he was treating required checks that were skipped.

We have three required checks, A, B and C. Checks A, B and C are running on every pull_request event. Checks B and C are taking long time and should not be run when there is no PR review with approval. To not run them on every new push to PR we added special PR label run-all-checks:

  • When PR is approved this label is added to PR
  • When this label is missing on PR only check A is running, B and C are skipped
  • Adding this label to PR or pushing new commit when this label exist will trigger all checks

The problem

When there is no label, check A is successful and checks B and C are skipped, GitHub threats those skipped required checks B and C as successful and developer can merge PR

Our workaround

We created this custom check D that was triggered just after checks A, B, and C were done (workflow_run event). It was required and was checking all those other required checks statuses to see if all of them were successful. If some were missing or not successful then it failed and blocked PR merge.

Questions

  • Can we somehow make skipped checks to be truly required?

And as a workaround they suggested:

  • create another job in workflows (that is also required check) alongside other required checks
  • it should always run
  • it should check statuses of other required jobs from same workflow and fail if some of them are not successful

But this last job would fail a lot and this would make a lot of workflows as failed in GitHub Actions page. And this generated problems for devs:

Thank you explaining this. I tried your suggested approach previously but it was also causing few issues for us:

  • It was generating a lot of failed workflows that were not really failed
  • Developers needed to actually check if workflow failed because of some change in their PR or if it was because it was skipped

So as a workaround for this they suggested to use continue-on-error: true on job level. This works quite well and I ended up with doing something like this:

  check-required-tests:
    name: "All required checks must succeed"
    needs:
      - skip-tests
      - build-push-ci-image
      - unit-tests
      - integration-tests
      - functional-tests-no-db
      - functional-tests-with-db
      - functional-tests-salesforce
    continue-on-error: true
    if: always()
    runs-on: ubuntu-22.04
    steps:
      - name: Fail with message if required checks failed
        if: |
          contains(needs.*.result, 'failure')
          || contains(needs.*.result, 'cancelled')
          || contains(needs.*.result, 'skipped')
        uses: actions/github-script@v3
        with:
          script: |
            core.setFailed('Not all required checks were successful!');

@alambike My problem was slightly different as you can see above but I had same problem as you with other workflow that was running only if one subdirectory was modified. I also used same solution you did and it worked fine.

@LouisBrunner LouisBrunner pinned this issue Sep 1, 2022
@LouisBrunner
Copy link
Owner

Look like this can be addressed through other ways, nice!

Cyberboss added a commit to tgstation/tgstation-server that referenced this issue Feb 3, 2024
Create the check using our own token
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants