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.

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

Change profile configuration for CI tests #374

Merged
merged 9 commits into from
Aug 19, 2024
Merged

Change profile configuration for CI tests #374

merged 9 commits into from
Aug 19, 2024

Conversation

atrigila
Copy link
Contributor

@atrigila atrigila commented Aug 14, 2024 •
edited
Loading

The CI tests are currently executed with the profile configuration -profile test,illumina,docker, which differs from the typical -profile test,docker that most nf-core pipelines use. If the users run the pipeline with -profile test,docker it would lead to the protocol custom to be used, instead of illumina. This inconsistency could lead to confusion, as the standard user environment does not accurately reflect the same results as the CI test environment.

This PR addresses #371.
Closes #375
It improves README documentation regarding how to use the protocol profiles.
It includes the illumina protocol profiles directly in the test profiles so that users can run them out-of-the-box as they do with any other nf-core pipeline:

nextflow run nf-core/smrnaseq -profile test,docker --outdir <results>

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/smrnaseq branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nf-test test main.nf.test -profile test,docker).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

@atrigila atrigila self-assigned this Aug 14, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 14, 2024 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit ba48e81

+| ✅ 213 tests passed       |+
#| ❔   1 tests were ignored |#
!| ❗   4 tests had warnings |!

❗ Test warnings:

  • pipeline_todos - TODO string in main.nf: Optionally add in-text citation tools to this list.
  • pipeline_todos - TODO string in main.nf: Optionally add bibliographic entries to this list.
  • pipeline_todos - TODO string in main.nf: Only uncomment below if logic in toolCitationText/toolBibliographyText has been filled!
  • schema_description - No description provided in schema for parameter: protocol

❔ Tests ignored:

  • nextflow_config - Config default ignored: params.fastp_known_mirna_adapters

✅ Tests passed:

Run details

  • nf-core/tools version 2.14.1
  • Run at 2024-08-16 20:38:19

@atrigila atrigila changed the title Fix test profiles with docker Change profile configuration for CI tests Aug 14, 2024
@atrigila atrigila marked this pull request as draft August 14, 2024 16:02
@atrigila
Copy link
Contributor Author

I am not sure why tests are not passing since I reverted changes to the initial configuration.

@atrigila
Copy link
Contributor Author

@grst would you mind retrying this job again from your side? when I do, I still get the mirbase issue. thank you!

@grst
Copy link
Member

grst commented Aug 15, 2024

now it fails for me as well :(
Which is strange, because I can still download it manually.

Maybe a workround could be to wget it in the CI yaml and then specify the local path in nextflow?

@atrigila
Copy link
Contributor Author

I solved the mirBase issue by changing the CI configuration so that when a test fails, it won't stop all other tests. This means that if one of the tests cannot get the mirBase, it is possible to re-run that test without affecting the others :)

@atrigila atrigila marked this pull request as ready for review August 16, 2024 21:35
@apeltzer apeltzer merged commit f69721e into nf-core:dev Aug 19, 2024
10 checks passed
@atrigila atrigila mentioned this pull request Aug 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apeltzer apeltzer apeltzer approved these changes

@nschcolnicov nschcolnicov Awaiting requested review from nschcolnicov

Assignees

@atrigila atrigila

Labels
ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@atrigila @grst @apeltzer