Add nf-test review guidelines #2414
Conversation
✅ Deploy Preview for nf-core ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
|
||
| - Check that there are tests for all outputs, including optional ones. | ||
| - Check that the `meta.yml` file has correct documentation links and patterns of files. | ||
| - Check that the file has correct documentation links and patterns of files. | ||
| - Run the tool help and check that important input (usually optional) has not been missed. |
There was a problem hiding this comment.
that's quite an overkill imo... are people really doing this? I like more of an organic growth of inputs tbh
There was a problem hiding this comment.
That was already there 😁 do you want me to remove it?
| - [Different assertion types](https://nf-co.re/docs/contributing/tutorials/nf-test_assertions) should be used when a straightforward `process.out` snapshot is not feasible. | ||
| - Check that a stub test exists for the module. | ||
| - Check that tags for any dependent modules are specified to ensure changes to upstream modules will re-trigger tests for the current module. | ||
| - Check that the `assertAll()` function is used and that there is at minimum a success assertion and versions in the snapshot. |
There was a problem hiding this comment.
we should lint for this. could somebody open an issue please?
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
|
Thanks for the thorough review @mashehu |
|
Prettier is complaining about the sub-points in the checklist, is there a better way to do this? |
|
I would also be in favor of putting this on a new page. We could collect all checklists for example similar to the tutorials. |
I'm this close to rage quitting my TODO list and just do it. That said iirc I thought @robsyme was tasked this before I went on parental leave... I dunno if there was a PR for that? |
|
Oh I didn't know that section existed, my bad 😁 |
|
Ok, you know what, pause this PR. Imma gonna do the restructuring tomorrow once I arrive in the UK (if I don't find an existing PR). Once it's in maybe can re-do this PR into the section |
|
Okay, sorry for the confusion 😁 |
Not your fault, this has bothered me for almost a year now 😅 |
|
I agree it could use some restructuring :p |
|
I would suggest @nvnieuwk closes this makes a new PR actually 😬 |
|
Allright, PR on the way! |
@netlify /docs/contributing/modules