-
Notifications
You must be signed in to change notification settings - Fork 6
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
Check and expand Ci tests #93
Conversation
…peptide lengths automatically to test this functionality
|
@nf-core-bot fix linting |
.github/workflows/ci.yml
Outdated
@@ -71,7 +80,7 @@ jobs: | |||
strategy: | |||
fail-fast: false | |||
matrix: | |||
tests: ["test", "test_mouse", "test_tiny"] # add further test profiles here, will be run in parallel (but only with one nextflow version) | |||
tests: ["test", "test_mouse", "test_tiny", "test_taxa_only"] # add further test profiles here, will be run in parallel (but only with one nextflow version) |
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.
Could you change the order here (it was changed in some refactoring in the past), such that the profile test
is used in the test above and the others here as additional? would seem more intuitive, or is there still a reason why it is like it is?
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.
doesn't work due to credentials ...
Co-authored-by: Sabrina Krakau <[email protected]>
Co-authored-by: Sabrina Krakau <[email protected]>
I added the versioning directory on test-datasets, so all tests will fail until this is merged. |
@nf-core-bot fix linting |
@skrakau linting is still failing due to template update, but it still expects test_bins! I will change it when this PR is merged or would you first change the expected tests for branch protection before merging? |
@skrakau Maybe different question: Which tests should I make required? All of the new/old ones? :) |
would there be a reason for any of the tests to not make them required? Otherwise I would make them required |
it's fine to do it afterwards, if you can? Maybe would be easier before |
No not really, okay I will make them required! They should all pass if one makes a PR! :) |
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.
LGTM!
I added some new tests with already existing data and changed some input sample sheets to have the following test:
without NCBI credentials:
with NCBI credentials:
I also opened a PR on test-datasets (nf-core/test-datasets#982) to add the new sample sheets.
PR checklist
nf-core lint
).nextflow run . -profile test,docker --outdir <OUTDIR>
).docs/usage.md
is updated.docs/output.md
is updated.CHANGELOG.md
is updated.README.md
is updated (including new tool citations and authors/contributors).