create ASCAT nf tests and remove pytest #5314

mauro-saporita · 2024-03-20T11:02:09Z

PR checklist

nf-test not working - pytest runs as expected but the nf-test I've created returns the following error: Path value cannot be null

FriederikeHanssen · 2024-03-20T11:03:56Z

tests/modules/nf-core/ascat/main.nf

-// extended tests running with 1000 genomes data. Data is downloaded as follows:
-// wget http://ftp.1000genomes.ebi.ac.uk/vol1/ftp/phase1/data/HG00154/alignment/HG00154.mapped.ILLUMINA.bwa.GBR.low_coverage.20101123.bam
-// wget http://ftp.1000genomes.ebi.ac.uk/vol1/ftp/phase1/data/HG00154/alignment/HG00154.mapped.ILLUMINA.bwa.GBR.low_coverage.20101123.bam.bai
-// wget http://ftp.1000genomes.ebi.ac.uk/vol1/ftp/phase1/data/HG00155/alignment/HG00155.mapped.ILLUMINA.bwa.GBR.low_coverage.20101123.bam
-// wget http://ftp.1000genomes.ebi.ac.uk/vol1/ftp/phase1/data/HG00155/alignment/HG00155.mapped.ILLUMINA.bwa.GBR.low_coverage.20101123.bam.bai
-// wget https://www.dropbox.com/s/l3m0yvyca86lpwb/G1000_loci_hg19.zip
-// wget https://www.dropbox.com/s/3fzvir3uqe3073d/G1000_alleles_hg19.zip
-// wget https://www.dropbox.com/s/v0tgr1esyoh1krw/GC_G1000_hg19.zip
-// wget https://www.dropbox.com/s/50n7xb06x318tgl/RT_G1000_hg19.zip
-


It would be nice if we could keep this info somewhere for local testing on input file changes. maybe we can add a readme here

Good point! I'm going to save it in a readme; where do you suggest I should save it? In modules/nf-core/ascat/tests?

I think that is a good palce

FriederikeHanssen · 2024-03-20T11:04:30Z

modules/nf-core/ascat/tests/main.nf.test

+
+        then {
+            assertAll (
+                { assert process.success }


can you add a check for the version?

Added; but unfortunately the test doesn't work, I get the following error: Path value cannot be null

when you ru the module manually do you get a versions file?

I get the error when it runs { assert process.success } so it doesn't run the test at all - could be something wrong with the way I declare the inputs?

input[1] = [] input[2] = [] input[3] = [] input[4] = [] input[5] = [] input[6] = []

the pytest runs with the following:

ASCAT_SIMPLE ( input , [], [], [], [], [], [])

Path value cannot be null issue solved by @sateeshperi - but test still not working

modules/nf-core/ascat/tests/simple-nextflow.config

mauro-saporita · 2024-03-20T15:34:33Z

@maxulysse let's hold a bit on this PR; tests are not working yet 😣

modules/nf-core/ascat/tests/main.nf.test

famosab · 2024-03-21T10:46:06Z

modules/nf-core/ascat/tests/main.nf.test

+                    file(params.test_data['homo_sapiens']['illumina']['test2_paired_end_sorted_bam_bai'], checkIfExists: true)
+                ]
+                input[1] = file(params.test_data['homo_sapiens']['illumina']['test2_allele_specific_recal'], checkIfExists: true)
+                input[2] = []


Did you check if path(loci_files) is optional aswell? I know we discussed that on slack but maybe that is why the nf-test does not run?

It shouldn't be optional but for some reason pytest works without loci and allele files. I'm trying to figure out why.

I also would like to run nf-test using loci_files and allele_files but they are not in test_data

It shouldn't be optional but for some reason pytest works without loci and allele files. I'm trying to figure out why.

Got it now, pytest only executed stub test: nextflow run ./tests/modules/nf-core/ascat -entry test_ascat -c ./tests/config/nextflow.config -stub-run

Edit: I just say that you said there are none so we need to add them via PR.

(Ok then we'll need to search for alternatives for those files here here or if there are none add them.)

famosab · 2024-03-21T10:47:25Z

modules/nf-core/ascat/tests/main.nf.test

+                { assert snapshot(process.out.versions).match("versions") }
+            )
+        }
+    }


You will need to add the stub test here aswell. You can just copy the normal test and add stub to the name as well as a line saying options "-stub"

famosab · 2024-03-21T10:50:40Z

tests/config/pytest_modules.yml

@@ -43,9 +43,6 @@ artic/guppyplex:
 artic/minion:
  - modules/nf-core/artic/minion/**
  - tests/modules/nf-core/artic/minion/**
-ascat:


Does anyone know why the CI pytest is triggered despite removal from the config by the CI nf-test is not triggered?

Only pytest-changes is triggered (not sure if it should run or not); pytest doesn't run.

Yes my bad! But nf-test should be triggered but is not.

Could it be because snapshot is not available yet?

But then the linting should fail first and that is also not triggered. Can you run nf-test locally?
nf-core modules test ascat --profile docker?

But then the linting should fail first and that is also not triggered. Can you run nf-test locally? nf-core modules test ascat --profile docker?

I run the test locally but it fails because loci_files and allele_files are missing.

@famosab could you help me to identify the right loci_files and allele_files for testing?

Can we use the files used in Sarek?
https://github.com/nf-core/sarek/blob/6aeac929c924ba382baa42a0fe969b4e0e753ca9/conf/igenomes.config#L15

These should work when they work with sarek - I would go for the latest reference genome: https://github.com/nf-core/sarek/blob/6aeac929c924ba382baa42a0fe969b4e0e753ca9/conf/igenomes.config#L46
Somewhere in the sarek conf it also says that you can setup the reference for ascat yourself and there are some intermediate steps to take but I guess they already did those for igenomes. I think we just have to make sure that the test_data input is also hg38 if we use human data.

https://github.com/nf-core/test-datasets/blob/modules/docs/ADD_NEW_DATA.md

Co-authored-by: Famke Bäuerle <45968370+famosab@users.noreply.github.com>

famosab · 2024-03-21T12:09:53Z

modules/nf-core/ascat/tests/README.md

+include { UNZIP as UNZIP_RT }                from '../../../../modules/nf-core/unzip/main.nf'
+
+workflow test_ascat {
+    input = [


Can you also update the path as in #5281 ? :)

famosab · 2024-05-15T11:52:52Z

@mauro-saporita Do you still have time to work on the proposed changes?

mauro-saporita · 2024-05-16T14:21:09Z

@mauro-saporita Do you still have time to work on the proposed changes?

I'm sorry lately I've been off the radar; busy with some other projects. I can work on this issue in June unless someone else would like to take it in the next week Hackathon event.

create nf tests and remove pytest

80319fd

mauro-saporita requested a review from famosab March 20, 2024 11:02

mauro-saporita requested review from lassefolkersen, FriederikeHanssen, maxulysse, SusiJo and a team as code owners March 20, 2024 11:02

Merge branch 'master' into nf-test-ascat

2178c5b

FriederikeHanssen reviewed Mar 20, 2024

View reviewed changes

famosab reviewed Mar 20, 2024

View reviewed changes

modules/nf-core/ascat/tests/simple-nextflow.config Outdated Show resolved Hide resolved

mauro-saporita and others added 2 commits March 20, 2024 12:15

change config and add version test

13e2804

Merge branch 'master' into nf-test-ascat

45bff6a

lgrochowalski self-requested a review March 20, 2024 13:08

maxulysse approved these changes Mar 20, 2024

View reviewed changes

maxulysse added this pull request to the merge queue Mar 20, 2024

mauro-saporita removed this pull request from the merge queue due to a manual request Mar 20, 2024

Merge branch 'master' into nf-test-ascat

006f3fb

mauro-saporita and others added 4 commits March 20, 2024 18:19

add readme with old pytest code

ef2f25d

Merge branch 'master' into nf-test-ascat

acbfc79

fix readme with prettier

4eb8ad9

Merge branch 'master' into nf-test-ascat

11d707c

mauro-saporita added draft Do not merge yet labels Mar 21, 2024

famosab reviewed Mar 21, 2024

View reviewed changes

mauro-saporita and others added 3 commits March 21, 2024 11:59

change test name

71c1791

Co-authored-by: Famke Bäuerle <45968370+famosab@users.noreply.github.com>

Merge branch 'master' into nf-test-ascat

4f5dfb8

add stub test

85983fb

mauro-saporita added the nf-test label Mar 21, 2024

famosab reviewed Mar 21, 2024

View reviewed changes

mauro-saporita and others added 3 commits March 22, 2024 10:20

change test data path

0a0b365

Merge branch 'master' into nf-test-ascat

c90e04a

Merge branch 'master' into nf-test-ascat

444ae42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

create ASCAT nf tests and remove pytest #5314

create ASCAT nf tests and remove pytest #5314

mauro-saporita commented Mar 20, 2024

FriederikeHanssen Mar 20, 2024

mauro-saporita Mar 20, 2024

FriederikeHanssen Mar 20, 2024

FriederikeHanssen Mar 20, 2024

mauro-saporita Mar 20, 2024

FriederikeHanssen Mar 20, 2024

mauro-saporita Mar 20, 2024

mauro-saporita Mar 20, 2024

mauro-saporita Mar 21, 2024

mauro-saporita commented Mar 20, 2024

famosab Mar 21, 2024

mauro-saporita Mar 21, 2024

mauro-saporita Mar 21, 2024

mauro-saporita Mar 21, 2024

famosab Mar 21, 2024 •

edited

famosab Mar 21, 2024

famosab Mar 21, 2024

mauro-saporita Mar 21, 2024

famosab Mar 21, 2024

mauro-saporita Mar 21, 2024

famosab Mar 21, 2024

mauro-saporita Mar 21, 2024

mauro-saporita Mar 21, 2024

mauro-saporita Mar 22, 2024

famosab Mar 22, 2024

famosab Mar 22, 2024

famosab Mar 21, 2024

famosab commented May 15, 2024

mauro-saporita commented May 16, 2024

create ASCAT nf tests and remove pytest #5314

Are you sure you want to change the base?

create ASCAT nf tests and remove pytest #5314

Conversation

mauro-saporita commented Mar 20, 2024

PR checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mauro-saporita commented Mar 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

famosab Mar 21, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

famosab commented May 15, 2024

mauro-saporita commented May 16, 2024

famosab Mar 21, 2024 •

edited