+
Skip to content

Add CI tests to test running a pipeline when it's created from a template #2334

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 28 commits into from
Jun 26, 2023
Merged

Add CI tests to test running a pipeline when it's created from a template #2334

merged 28 commits into from
Jun 26, 2023

Conversation

mirpedrol
Copy link
Member

This is a test to see if it's viable to add CI tests for all possible templates:

  • unbranded pipelines
  • skip ci + github_badges + igenomes + nf_core_configs (github can't be skipped as it's run in GHA)
  • skip ci
  • skip github_badges
  • skip igenomes
  • skip nf_core_configs

➕ We could also add the same for linting.
👁️ This will make sure that bugs as the ones found recently (#2332 and #2333) are not repeated.
⚠️ It will take considerably more time and Github runners, so needs to be discussed before merging.

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

mashehu
mashehu reviewed Jun 21, 2023
View reviewed changes
Copy link
Contributor

@mashehu mashehu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

What speaks against adding this test to create-test-wf.yml instead of having it in a separate file?

@mirpedrol
Copy link
Member Author

What speaks against adding this test to create-test-wf.yml instead of having it in a separate file?

Can do both, I just found it more organized in a different file

@codecov
Copy link

codecov bot commented Jun 21, 2023 •
edited
Loading

Codecov Report

Merging #2334 (18a76e8) into dev (4f32c07) will increase coverage by 0.00%.
The diff coverage is 83.33%.

@@           Coverage Diff           @@
##              dev    #2334   +/-   ##
=======================================
  Coverage   73.08%   73.08%           
=======================================
  Files          78       78           
  Lines        8779     8784    +5     
=======================================
+ Hits         6416     6420    +4     
- Misses       2363     2364    +1
Impacted Files Coverage Δ
nf_core/create.py 83.27% <83.33%> (-0.07%) ⬇️

@mirpedrol
Copy link
Member Author

Actually, because these pipelines created from a template are unbranded, the pipeline prefix is different, so I think it's easier to have two separate files

@mirpedrol mirpedrol changed the title Add CI tests to test running a pipeline whe it's created from a templ… Add CI tests to test running a pipeline when it's created from a template Jun 22, 2023
mashehu
mashehu reviewed Jun 22, 2023
View reviewed changes
Copy link
Contributor

@mashehu mashehu left a comment

Choose a reason for hiding this comment

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

should also only run if the pipeline template was actually changed:

on:
  push:
    paths:
      - nf_core/pipeline-template/**

@mirpedrol
Copy link
Member Author

Everything working now! 🥳

@mirpedrol mirpedrol merged commit b5eb200 into nf-core:dev Jun 26, 2023
@mirpedrol mirpedrol deleted the different-template-tests branch June 26, 2023 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mashehu mashehu mashehu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mirpedrol @mashehu @drpatelh
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载