Separate CI tests #350
Merged
Separate CI tests #350
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
sofstam
reviewed
Aug 9, 2023
format the if condition
jfy133
reviewed
Aug 10, 2023
.github/workflows/ci.yml
Outdated
Comment on lines
39
to
42
| - "--preprocessing_qc_tool=falco" | ||
| - "--shortread_qc_tool=fastp" | ||
| - "--shortread_qc_tool=adapterremoval" | ||
| - "--shortread_complexityfilter_tool=bbduk" |
There was a problem hiding this comment.
I guess these still need to be replaced with profiles?
There was a problem hiding this comment.
Do you mean changed it into:
- "docker,--preprocessing_qc_tool=falco --shortread_qc_tool=fastp --shortread_qc_tool=adapterremoval --shortread_complexityfilter_tool=bbduk --shortread_complexityfilter_tool=prinseqplusplus"
There was a problem hiding this comment.
No, if i have a spare 30 minutes today I'll push changes what I mean
There was a problem hiding this comment.
What I mean is: -profile test_falco for example, then -profile test_bbduk
There was a problem hiding this comment.
Thanks! I will prepare the rest of test configs.
.github/workflows/ci.yml
Outdated
Comment on lines
68
to
71
| wget https://raw.githubusercontent.com/motu-tool/mOTUs/master/motus/downloadDB.py | ||
| python downloadDB.py > download_db_log.txt | ||
| echo 'tool,db_name,db_params,db_path' > 'database_motus.csv' | ||
| echo 'motus,db_mOTU,,db_mOTU' >> 'database_motus.csv' |
There was a problem hiding this comment.
And these lines need to be replace with a bash if else statement (if matrix.tags), and merged with the main tests, and the github actions YAML if: statement needs to be removed -> allowing to have a single test, entry in the YAML and the conditionals within the command: block (I hope that makes sense!)
There was a problem hiding this comment.
I am not sure about this error.
There was a problem hiding this comment.
You'll need to check the bash, something is stopping the full evaluation before the final fi. You could try copy pasting into shellcheck and maybe that will give you more info
There was a problem hiding this comment.
I agree with you. I though it could be caused by indentation. But the command lines work in shell if I just replace the 2 space (default in yml ) into tab (default in shell). No idea how to solve it.
There was a problem hiding this comment.
Thanks to @samuell the issue was solved. It was not allowed to use multiline code within a bash block while using Wandalen/[email protected]
|
@jfy133 All CI tests passed except test_motus which seems caused by the path of database_motus.csv. Have tried to change it into ${{github.workspace}}/database_motus.csv and it didn't help. |
|
Edit: maybe try |
|
The error in GHa to me looks like the if/else still isn't being parsed properly? No wait Indeed I think the bash isn't workinng:
|
|
@jfy133 you are right that it seems like I can not use if condition under action according to Wandalen/wretry.action#16 |
jfy133
reviewed
Aug 16, 2023
|
Oops - @LilyAnderssonLee all test profiles except test_nothing and test_full!! The latter is a special case that will only run on AWS! |
jfy133
reviewed
Aug 16, 2023
jfy133
reviewed
Aug 16, 2023
jfy133
approved these changes
Aug 17, 2023
There was a problem hiding this comment.
A few minor tweaks:
- For test configs that use tools that 'modify' the FASTQ files (e.g. complexity filtering) I think it is safer to actually run all downstream steps to make sure for whatever reason they don't 'corrupt' the FASTQ file for the downstream steps for whatever reason (however with a smaller set of profilers to reduce our computational footprint)
|
Good point! I agree with changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR checklist
Fix the issue #314
nf-core lint).nextflow run . -profile test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).