[prtester] improve prtester.py and prhtmlgenerator.yml for running in forks
This PR will improve prtester.py and prhtmlgenerator.yml for running in forks. It makes the hard-coded repo name and base url to the new rss-bridge-tests repo dynamic/configurable (by using the repo owners name and an optional ARTIFACTS_REPO variable). The Upload tests job will be skipped when the secret RSSTESTER_ACTION is missing (which will always be the case for forks, by default) instead of always failing. Additional small fixes to the Upload tests job: It failed when the prs directory did not exist yet and it failed when there was no change to the html files.
Thanks for all the help with this!
Because I've run into this before and maybe you know how to do it:
If you re-run the test manually within github, I've had the behavior that it cant identify the github event number (because the run wasnt caused by a commit but by a manual re-run request) and thus, the github event number defaults to "none".
I'm not sure if you can recreate that and if so, have a fallback to use something else but the github event number.
I could not recreate that behavior by re-running the pipeline or single jobs of the pipelines. github.event.number was always set, but to make sure I added a fallback to value none.
@Bockiii can this get merged?
I honestly dont understand whats happening in the code and which fail overs are in place.
python prtester.py --artifacts-base-url "https://${{ github.repository_owner }}.github.io/${{ vars.ARTIFACTS_REPO || 'rss-bridge-tests' }}/prs/${{ github.event.number || 'none' }}";
Not every github user has setup a github.io page.
repository: "${{ github.repository_owner }}/${{ vars.ARTIFACTS_REPO || 'rss-bridge-tests' }}"
Definitely no one has setup either this (undocumented) variable or has setup a repo named "rss-bridge-tests".
As an alternative, I would check if the workflow is run in the original repo or not and if not, dont do the upload to a repository. The github action will still have the artifact uploads, so you can just download the html files and check. You could also create an alternate comment in the PR that just says "go to the action and download the artifacts to check" or so.
I honestly dont understand whats happening in the code and which fail overs are in place.
python prtester.py --artifacts-base-url "https://${{ github.repository_owner }}.github.io/${{ vars.ARTIFACTS_REPO || 'rss-bridge-tests' }}/prs/${{ github.event.number || 'none' }}";
- Instead of the previous hard-coded url
https://rss-bridge.github.io/rss-bridge-tests/prs/{prid}inside the python script itself, the base url gets passed as parameter--artifacts-base-urlinto it. The base url gets build with dynamic variables insideprhtmlgenerator.yml. github.repository_ownerwill always return the owner of the repository which the PR targets. For PRs here in the original repo it will result inrss-bridgeagain. For PRs targeting e.g. my fork it would result toUser123698745.vars.ARTIFACTS_REPOis fully optional. It can be used to change the name of the repository, where the files will be uploaded to. If its not set, which will almost always be the case, it will fallback (see the||) torss-bridge-tests.github.event.numberwill always return the PR id, when the Job trigger ispull_request_targetorpull_request.prhtmlgenerator.ymldoes not use any otherontrigger, sogithub.event.numbershould always be set correctly. Because of your note about re-runs, I added a fallback tonone, but I was not able to reproduce that.github.event.numberwas always set correctly in my test re-runs.- So by default for this original repository it will result in to the exact same url
https://rss-bridge.github.io/rss-bridge-tests/prs/{prid}, but for forks it will result into a working url for them e.g for my forkhttps://User123698745.github.io/rss-bridge-tests/prs/{prid}.
Not every github user has setup a github.io page.
repository: "${{ github.repository_owner }}/${{ vars.ARTIFACTS_REPO || 'rss-bridge-tests' }}"Definitely no one has setup either this (undocumented) variable or has setup a repo named "rss-bridge-tests".
- Yes, that's why I added this condition
if: needs.checks.outputs.WITH_UPLOAD == 'true'. The upload only ever will run when the secret you introducedsecrets.RSSTESTER_ACTIONis set. By default forks will not have the secret, so by default there will be no upload, but you will still get the comment on all PRs including the helpful warning and error messages and the artifacts on the job run result. Most importantly the job will not fail by default like it currently does for all forks.
As an alternative, I would check if the workflow is run in the original repo or not and if not, don't do the upload to a repository. The github action will still have the artifact uploads, so you can just download the html files and check. You could also create an alternate comment in the PR that just says "go to the action and download the artifacts to check" or so.
- I am not a fan of limiting that feature to the original only. By default it will behave like you describe (except the alternate comment). Forks can optionally decide by them self if they want to enable uploads by creating their own
rss-bridge-testsrepository (or using any other name, but then and only then they also need to setvars.ARTIFACTS_REPOto that repository name), settingsecrets.RSSTESTER_ACTIONto a PAT with access to theirrss-bridge-testsrepository and enabling github pages on theirrss-bridge-testsrepository.
@Bockiii does that explain it?
it failed when there was no change to the html files
This now happened for PR https://github.com/RSS-Bridge/rss-bridge/pull/4324 (see the error nothing to commit, working tree clean in the pipeline output https://github.com/RSS-Bridge/rss-bridge/actions/runs/11992047625/job/33431439338?pr=4324), because code was moved, but the prtester.py result stayed the same.
@Bockiii or @dvikan would be nice if you could merge this to get that fixed
(or at least only that specific fix
git commit -m "$COMMIT_MESSAGE" || exit 0
instead
git commit -m "$COMMIT_MESSAGE"
)
i dont wanna mess with python and prtester sorry. @Bockiii
do you still want me to merge this?
do you still want me to merge this?
Yes, that would be nice. @Bockiii does not seem active since I created this PR (according to Bockiii's github contribution graph). I tested the change multiple times in my fork. It works as I expect