Skip to content

Add lint test to check existence of test profile#2478

Merged
fa2k merged 6 commits into
nf-core:devfrom
fa2k:add-test-for-test-profile
Oct 18, 2023
Merged

Add lint test to check existence of test profile#2478
fa2k merged 6 commits into
nf-core:devfrom
fa2k:add-test-for-test-profile

Conversation

@fa2k

@fa2k fa2k commented Oct 17, 2023

Copy link
Copy Markdown
Contributor

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

This is the first part of #1837.

See below comment: The following is no longer true (no way to "strike through" in github?):
It is not an ideal implementation, because it has to call nextflow, and only gets indirect indication of the failure due to the exit code. If there's a better way to reliably parse the config with python, I will use that instead- I hope someone can offer guidance on that if possible.

This is a draft PR, not ready for merge.

@fa2k fa2k force-pushed the add-test-for-test-profile branch from 2efda30 to 0cf3fbd Compare October 17, 2023 13:15
@fa2k fa2k force-pushed the add-test-for-test-profile branch from 0cf3fbd to 6c0325f Compare October 17, 2023 13:22
@fa2k

fa2k commented Oct 17, 2023

Copy link
Copy Markdown
Contributor Author

The test is now parsing nextflow.config to verify the existence of the test profile. It's failing one CI test and I don't know why. I'm going to change it to not be a draft.

@fa2k fa2k marked this pull request as ready for review October 17, 2023 17:48
@codecov

codecov Bot commented Oct 17, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2478 (c57cb8c) into dev (9171fa0) will decrease coverage by 0.08%.
Report is 7 commits behind head on dev.
The diff coverage is 95.23%.

❗ Current head c57cb8c differs from pull request most recent head 9457021. Consider uploading reports for the commit 9457021 to get more accurate results

@@            Coverage Diff             @@
##              dev    #2478      +/-   ##
==========================================
- Coverage   70.71%   70.63%   -0.08%     
==========================================
  Files          87       87              
  Lines        9424     9461      +37     
==========================================
+ Hits         6664     6683      +19     
- Misses       2760     2778      +18     
Files Coverage Δ
nf_core/lint/actions_ci.py 82.85% <ø> (ø)
nf_core/lint/nextflow_config.py 80.48% <95.23%> (+3.03%) ⬆️

... and 7 files with indirect coverage changes

@mashehu

mashehu commented Oct 17, 2023

Copy link
Copy Markdown
Contributor

we added a new type checker for python code, which unfortunately is still a bit too aggressive. you can ignore that specific test for now.

Comment thread nf_core/lint/nextflow_config.py Outdated
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
@fa2k fa2k merged commit 66dbf0b into nf-core:dev Oct 18, 2023
@fa2k fa2k mentioned this pull request Oct 18, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants