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.

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

Add lint test to check existence of test profile #2478

Merged
merged 6 commits into from Oct 18, 2023

Conversation

fa2k
Copy link
Contributor

@fa2k fa2k commented Oct 17, 2023

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
Copy link
Contributor Author

fa2k commented Oct 17, 2023

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
Copy link

codecov bot commented Oct 17, 2023

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
Copy link
Contributor

mashehu commented Oct 17, 2023

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.

nf_core/lint/nextflow_config.py Outdated Show resolved Hide resolved
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
@fa2k fa2k merged commit 66dbf0b into nf-core:dev Oct 18, 2023
17 of 18 checks passed
@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.

None yet

2 participants