modules icon indicating copy to clipboard operation
modules copied to clipboard

Add nf-test to unzip module

Open jennylsmith opened this issue 1 year ago • 4 comments

PR checklist

Adds nf-tests to unzip module.

  • [x] This comment contains a description of changes (with reason).
  • [x] 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
  • [ ] If necessary, include test data in your PR.
  • [x] Remove all TODO statements.
  • [x] Emit the versions.yml file.
  • [x] Follow the naming conventions.
  • [x] Follow the parameters requirements.
  • [x] 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:
    • For modules:
      • [x] nf-core modules test <MODULE> --profile docker
      • [x] nf-core modules test <MODULE> --profile singularity
      • [x] nf-core modules test <MODULE> --profile conda
    • For subworkflows:
      • [ ] nf-core subworkflows test <SUBWORKFLOW> --profile docker
      • [ ] nf-core subworkflows test <SUBWORKFLOW> --profile singularity
      • [ ] nf-core subworkflows test <SUBWORKFLOW> --profile conda

jennylsmith avatar Mar 20 '24 21:03 jennylsmith

any reason why you chose a different input to the old pytests versions for this module?

and because they are very similar, do you mind to also convert the unzipfiles module in this PR?

@mashehu My only reasoning to change the input was that the process itself is generic, so I thought it might make more sense to use a generic filetype input. I also thought it might be a smaller and so quicker to run, but I admit I didn't compare file sizes directly. Is it preferred to change it back to the original input dataset from pytest?

Happy to convert the unzipfiles nf-tests to this PR as well.

jennylsmith avatar Mar 20 '24 22:03 jennylsmith

@mashehu My only reasoning to change the input was that the process itself is generic, so I thought it might make more sense to use a generic filetype input. I also thought it might be a smaller and so quicker to run, but I admit I didn't compare file sizes directly. Is it preferred to change it back to the original input dataset from pytest?

Okay, yes, makes total sense.

mashehu avatar Mar 21 '24 07:03 mashehu

@jennylsmith LGTM! I think you should be good to merge it!

CarsonJM avatar Apr 03 '24 15:04 CarsonJM

@jennylsmith , can you update the branch with the changes from the master and see if the tests pass? I don't have access to your branch to do that.

SPPearce avatar May 08 '24 13:05 SPPearce

@jennylsmith, are you able to update your branch with the most recent changes please.

SPPearce avatar May 22 '24 16:05 SPPearce

Incorpated changes to #5897

SPPearce avatar Jul 04 '24 09:07 SPPearce