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 builds on forked pull requests #187

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

JojOatXGME
Copy link
Contributor

@JojOatXGME JojOatXGME commented May 4, 2024

Pull Request Details

Description

The action dorny/test-reporter@v1 requires write permissions, which are not available for pull requests from forked repositories.

This PR applies the same fix as in JetBrains/gradle-changelog-plugin#170. Due to dorny/test-reporter#343, this fix alone didn't work, so I also applied a workaround with commit b8ffd16.

Note that I also just created a pull request at dorny/test-reporter#436 which fixes the second issue according to my tests. Depending on whether and how fast the PR gets accepted, the workaround might become obsolete soon.

Related Issue

Motivation and Context

Fix the build of #179.

How Has This Been Tested

I created a pull request on my fork with a failing unit test.

JojOatXGME#1

This demonstrates that the test reports are still working. However, this test does not demonstrate that the bug is actually fixed, since my pull request is not coming from a different fork. However, it worked for the gradle-changelog-plugin.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

The action `dorny/test-reporter@v1` requires write permissions, which
are not available for pull requests from forked repositories.
@JojOatXGME
Copy link
Contributor Author

FYI, I would also suggest to set continue-on-error as I did at f4d17bd. This avoids redundant notifications due to the job failure. The job still creates a failed check-run for the pull request. The action also has an option fail-on-error: false, but it looks like the action convolutes the conclusion of the job with the conclusion of the report. If the option is used, it doesn't only prevent the job failure, but it also sets the state of the report to success.

https://github.com/dorny/test-reporter/blob/c40d89d5e987cd80f3a32b3c233556e22bdca958/src/main.ts#L184-L185

@JojOatXGME
Copy link
Contributor Author

I removed the workaround (b8ffd16) from this pull request since dorny/test-reporter#438 and dorny/test-reporter#444 are resolved.

PS: Due to these fixes, the reports should also start to work again for the gradle-changelog-plugin. Test Report 105 at https://github.com/JetBrains/gradle-changelog-plugin/actions/workflows/test-report.yml seems to be the last failed run, future runs should work again. You may also try to re-run one of them to test it.

@YannCebron YannCebron merged commit fdba8ca into JetBrains:master Jun 27, 2024
@YannCebron
Copy link
Member

Thanks a lot for this PR and the research, sorry for delay in handling.

@JojOatXGME JojOatXGME mentioned this pull request Jun 27, 2024
10 tasks
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.

None yet

2 participants