Refactor module linting code

[ewels]

In nf-core/tools we have two similar yet different commands:

• nf-core lint which runs tests on pipelines
• nf-core modules lint which runs tests on individual DSL2 modules

The latter was written by @KevinMenden and was largely based on my original pipeline lint code, but tailoring it to the specific needs of modules.

I think that there could be some advantages to refactoring the modules lint code. I refactored the pipeline linting heavily back in #805 and I think a lot of the same things could be done to the module linting.

• Moving tests into their own files + directories
• Splitting up python tests
• Move more things into classes / share more code
• Get nf-core lint to run nf-core modules lint tests

---

Moving tests into their own files + directories (python modules)

At the moment, all module tests are function defs in a single massive script. It's easier to work with them if they can be pulled in independently, as is now done for the pipeline linting (see pieline lint directory, imports and running setup). Basically the same as now with the named functions, but just a bit more separation.

Splitting up python tests

Similar to the above - can mimic the structure of what I started (and never finished) in the main pipeline lint tests:

tools/tests/test_lint.py

Lines 176 to 183 in 7721f37

	#######################
	# SPECIFIC LINT TESTS #
	#######################
	from lint.actions_awsfulltest import (
	test_actions_awsfulltest_warn,
	test_actions_awsfulltest_pass,
	test_actions_awsfulltest_fail,
	)

Importantly, I'd like to get rid of any pytests that check the number of results (unless they're meant to all be 0), as this breaks every time we add or remove a test / change anything:

tools/tests/test_modules.py

Lines 121 to 123 in 7721f37

	assert len(module_lint.passed) == 20
	assert len(module_lint.warned) == 0
	assert len(module_lint.failed) == 0

Move more things into classes / share more code

It is my suspicion that there is a fair amount of code between the two linting codebases that could be shared in a better way. However, from my quick read-over again maybe I'm wrong about this. But something to bear in mind.

Get nf-core lint to run nf-core modules lint tests

When we lint a pipeline, we also want to lint all of the modules that are installed in the pipeline. It would be good to get the pipeline linting to run the local / installed module lint tests. It's probably worth thinking about which ones are relevant - we probably don't need to run most modules lint tests on centralised modules as they should already be tested there. But we may want to run some tests on the local modules, and we definitely want to check that they are unmodified etc.

It may make sense to move some tests (eg. checking for modifications / new versions of a module) out of modules lint and into the pipeline linting. Then the remaining tests in nf-core modules lint would only be for running on the central nf-core modules repo.

Basically as with the other ones above I don't really know what the best approach is here, so needs some careful investigation and thought.

---

'cc @KevinMenden - what do you think?

[KevinMenden]

It could surely not hurt to have another go at refactoring the linting, especially when done together with nf-core lint to make the two work better together. For instance it would be nice to harmonize the output a bit such that both checks can be run together.

Maybe worth to note that I have already refactored the lint code to some extent, splitting up different tests into different files:
https://github.com/nf-core/tools/tree/dev/nf_core/modules/lint

Essentially what I've done is putting tests for specific files in the same python script (e.g. tests formain.nf, functions.nf, ...). Other scripts contain code about checking changes in modules, checking the versions or module TODOs (which is an example of code that is actually borrowed from the pipeline lint code).
One thing I had in mind here was that we could actually create Classes for every different file of an NfcoreModule object, which might make it nicer to lint. However it might also be better to just divide them into the smallest parts possible.

Dividing the tests into more more granular parts would of course make a lot of sense 👍
I did start removing those == statements and replacing them at least by > signs or similar to make them more robust - but that's still not ideal of course.

[ErikDanielsson]

Closing because of the merge of #1164 and #1166.