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

Fix workflows for PRs coming from forked repositories #154

Merged
merged 2 commits into from
Jan 28, 2022

Conversation

remcowesterhoud
Copy link
Contributor

@remcowesterhoud remcowesterhoud commented Jan 27, 2022

Description

PRs from forked repositories would always fail because the workflow is unable to publish the unit test results. This is because the GITHUB_TOKEN in these workflows have read-only right.
By putting the publishing of these results in a separate workflow we can work around these access rights. This workflow gets triggered upon completion of the first workflow and downloads the artifacts uploaded in the first workflow. This allows it to publish them without problem.

See: https://github.com/EnricoMi/publish-unit-test-result-action#support-fork-repositories-and-dependabot-branches

Related issues

closes #148

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

  • The changes are backwards compatibility with previous versions
  • If it fixes a bug then PRs are created to backport the fix

Testing:

  • There are unit/integration tests that verify all acceptance criterias of the issue
  • New tests are written to ensure backwards compatibility with further versions
  • The behavior is tested manually

Documentation:

  • Javadoc has been written
  • The documentation is updated

The step was called "... on Failure". Since this step has been changed in the past to always run, regardless of failure it makes sense to remove the "on Failure" part.
Publishing unit test results the way we did does not work when we get a PR from a forked repo (eg. dependabot). This is because forked repositories run with a read-only GITHUB_TOKEN.
We can circumvent this problem by publishing the test results in a separate workflow. This workflow gets triggered upon completion of the workflow that is triggered by the forked PR. The second workflow will have the right to write, thus is able to publish the test results. The first workflow has to upload its event file as an artifact for the second workflow to be able to publish the results.

For more info see: https://github.com/EnricoMi/publish-unit-test-result-action#support-fork-repositories-and-dependabot-branches
@remcowesterhoud
Copy link
Contributor Author

remcowesterhoud commented Jan 27, 2022

@korthout I'd love a review from you here. I wanted to test this works but I don't think there is a way to do this without this being merged first (if you know one please enlighten me 🧠).

I am confident it will work though, since this solution is copied straight from the docs 😄

Copy link
Member

@korthout korthout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally, this would be resolved by changing the context in which the workflow is run, i.e. changing pull_request into pull_request_target, but the docs explicitly mention that this is dangerous for this action (referring to this article).

In any case, the recommended solution (although a bit more verbose), looks like it would resolve the problem. So LGTM 🦕

I'm not sure about testing it. Seems this workflow is run on any pull request event. Perhaps give the PR a label and see if that runs the workfow(s).

@remcowesterhoud remcowesterhoud merged commit f858976 into main Jan 28, 2022
@remcowesterhoud remcowesterhoud deleted the 148-forked-pr-workflow branch January 28, 2022 07:37
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

Successfully merging this pull request may close these issues.

Fix workflows for forked pull requests (dependatabot pull requests)
2 participants