Fastq screen

[JPejovicApis]

PR checklist

Closes #3156
Closes #3157

• 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 module conventions in the contribution docs
• Remove all TODO statements.
• Emit the versions.yml file.
• Follow the naming conventions.
• Follow the parameters requirements.
• Follow the input/output options guidelines.
• Add a resource label
• Use BioConda and BioContainers if possible to fulfil software requirements.
• Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
  • PROFILE=docker pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware
  • PROFILE=singularity pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware
  • PROFILE=conda pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware

[SPPearce]

This is looking really close, if the suggested change fixes the error.

[SPPearce]

Looking close! Just the conda check is complaining about a md5sum being different, can you swap that to a contains text?

[SPPearce]

I'd like to get another review for this before merging, as it is a very tricky tool to get working.
Well done to all those involved.

[SPPearce]

I've ported the tests over to nf-test.
I haven't manage to sort out the syntax to check the fastqscreen/fastqscreen output channel though, that is eluding me. It has three files:

test_fq_screen/test_1_screen.txt
test_fq_screen/test_1_screen.png
test_fq_screen/test_1_screen.html

Of which the .txt and .png files are stable, the .html isn't. But I can't manage to get it to snapshot properly without some kind of error.

[adamrtalbot]

For the variable *.txt, *.png and *.html files, capture them as separate outputs then it will be easier to separate them in the process.

e.g. in the output block:

tuple val(meta), path("*.txt"), emit: txt
tuple val(meta), path("*.png"), emit: png
tuple val(meta), path("*.html"), emit: html

in the nf-test:

assert snapshot(process.out.txt, process.out.png).match()

[SPPearce]

Grrr, the other two files have consistent md5sums on conda and singularity, but they are different to docker...

[adamrtalbot]

Grrr, the other two files have consistent md5sums on conda and singularity, but they are different to docker...

That's very unusual, I would expect Singularity and Docker to be the same.

Might have to accept whatever testing is possible here (process success and file exists), plus any easy file contents checking that makes sense.

[SPPearce]

Grrr, the other two files have consistent md5sums on conda and singularity, but they are different to docker...

That's very unusual, I would expect Singularity and Docker to be the same.

Might have to accept whatever testing is possible here (process success and file exists), plus any easy file contents checking that makes sense.

Yes, I just swapped to file existance.

[adamrtalbot]

It all looks good, but there's something up with the indentation and perhaps a few styling choices, but the code itself is fine.

[adamrtalbot]

The indentation seems a bit weird here?

[SPPearce]

To be honest, I don't know how we are supposed to ident things like this

[adamrtalbot]

Definitely something funky with the indentation.