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

Fix tags entries and rename pipeline level tests #1324

Merged
merged 14 commits into from
Jun 20, 2024
Merged

Fix tags entries and rename pipeline level tests #1324

merged 14 commits into from
Jun 20, 2024

Conversation

drpatelh
Copy link
Member

  • Remove and edit tag entries in tests to make them consistent
  • Rename pipeline level tests and fix descriptions
  • Add missing PRs and params updates to CHANGELOG

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 19, 2024 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit aae6587

+| ✅ 173 tests passed       |+
#| ❔   9 tests were ignored |#
!| ❗   7 tests had warnings |!

❗ Test warnings:

  • files_exist - File not found: assets/multiqc_config.yml
  • files_exist - File not found: .github/workflows/awstest.yml
  • files_exist - File not found: .github/workflows/awsfulltest.yml
  • 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!
  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your preferred methods description, e.g. add publication citation for this pipeline

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 2.14.1
  • Run at 2024-06-20 13:25:37

adamrtalbot
adamrtalbot requested changes Jun 20, 2024
View reviewed changes
Copy link
Contributor

@adamrtalbot adamrtalbot left a comment

Choose a reason for hiding this comment

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

  • You don't need any tags anymore, so they can be removed
  • Put all the test names back. Test names should say what they are testing, not what parameters you are changing. I don't give a flying fig that --parameterA false, I care that the test is checking that resultA is not produced when parameterA is disabled. nf-core has a terrible habit of naming things what they are and not what they do which we need to get rid of.

modules/nf-core/summarizedexperiment/summarizedexperiment/tests/main.nf.test
Comment on lines +6 to +8
tag "UNTAR"
tag "CUSTOM_TX2GENE"
tag "TXIMETA_TXIMPORT"
Copy link
Contributor

Choose a reason for hiding this comment

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

What are these tags doing here?

Copy link
Member Author

Choose a reason for hiding this comment

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

These modules are used in setup sections for this test. If any of those modules change upstream do we not want to run this test too? Or is there some other magic that nf-test uses to track this behind the scenes that I am not aware of?

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct, tags have not been used at all for a few months now.

subworkflows/nf-core/bedgraph_bedclip_bedgraphtobigwig/tests/main.nf.test Outdated Show resolved Hide resolved
modules/nf-core/salmon/quant/tests/main.nf.test
tag "salmon/quant"
tag "salmon/index"

tag "SALMON_INDEX"
Copy link
Contributor

Choose a reason for hiding this comment

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

These tags aren't required, but they don't cause any harm here.

Copy link
Member Author

Choose a reason for hiding this comment

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

See #1324 (comment)

tests/featurecounts_group_type.nf.test Outdated Show resolved Hide resolved
tests/featurecounts_group_type.nf.test Outdated Show resolved Hide resolved
tests/featurecounts_group_type.nf.test Outdated Show resolved Hide resolved
tests/kallisto.nf.test Outdated Show resolved Hide resolved
tests/kallisto.nf.test Outdated Show resolved Hide resolved
@drpatelh drpatelh merged commit 579684a into nf-core:dev Jun 20, 2024
65 checks passed
This was referenced Jun 20, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamrtalbot adamrtalbot adamrtalbot approved these changes

@pinin4fjords pinin4fjords pinin4fjords 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
@drpatelh @pinin4fjords @adamrtalbot