-
Notifications
You must be signed in to change notification settings - Fork 192
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
Pipeline template: Fix the faulty Download test / GitHub action #3389
Pipeline template: Fix the faulty Download test / GitHub action #3389
Conversation
e03dbc0
to
5aee4b1
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files☔ View full report in Codecov by Sentry. |
5aee4b1
to
c67027d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Also tested with the nf-core/testpipeline https://github.com/nf-core/testpipeline/actions/runs/12744809455/job/35517501873?pr=101
Great! I think, it was by now already manually incorporated to the development version of the Taxprofiler pipeline, so that also suggests it does now work.
Did you by chance also test the conditional part? Because I have never created a pipeline with a cherry-picked template, and there is this |
Exactly, it won't contain anything that is between |
c67027d
to
51720db
Compare
51720db
to
737b515
Compare
In #3351, I refactored the nf-core pipelines download test to use
$GITHUB_OUTPUT
instead of the environment for improved security.However, splitting the test into two independent jobs introduced the possibility of them running on different runners, which resulted in the second job lacking the correct setup. I have now resolved this issue by ensuring that all necessary setup setups are retained within the main job. Only the potentially vulnerable step remains isolated from the main job, but also does not require any setup.
The new version of the CI pipeline was successfully tested in my Test pipeline repository. Any further independent tests are highly appreciated.
PR checklist
CHANGELOG.md
is updateddocs
is updated