Conversation
|
Or we can just add those exceptions in the |
|
I would say the |
| name: check test file name convention | ||
| entry: python tools/check-test-file-name.py | ||
| language: python | ||
| files: '(tests/.*\.py)$' |
There was a problem hiding this comment.
Did I understand correctly that it will flag any file that does not end with _test?
Sometimes we may have some code in a module that isn't a test but contains common code for setting up tests? And I think these are not always in the conftest.py?
That's probably the case for the mcstas example thing. Is it always possible to move to a conftest file, or are there cases where we cannot?
Also, do we ever have non-python files in the test/ folder, which could end in something else? Like some data files for testing?
There was a problem hiding this comment.
That's probably the case for the mcstas example thing. Is it always possible to move to a conftest file, or are there cases where we cannot?
I guess we can always use the conftest file ...
And then we can accept all non-python files under the tests folder.
There was a problem hiding this comment.
That's probably the case for the mcstas example thing. Is it always possible to move to a conftest file, or are there cases where we cannot?
This file is imported by only one other file. So as long as it stays that way, we can move the contents to that importing file.
Also, do we ever have non-python files in the test/ folder, which could end in something else? Like some data files for testing?
The regex will only look for .py files. So anything else won't be checked and therefore will be accepted.
Did I understand correctly that it will flag any file that does not end with _test?
Sometimes we may have some code in a module that isn't a test but contains common code for setting up tests? And I think these are not always in the conftest.py?
Correct. I think in principle, we can always move common code into fixtures. But that does not always make sense.
Looking through our ess* packages, I found one case in loki: common.py. This defines a single function to create a workflow which could be provided as a fixture. We do that in other packages.
And esslivedata has a number of modules in its tests. But they seem to define fixtures. So they could be moved into a conftest.py file. But that file would be fairly large.
Currently, this hook fails with
I think the first one needs to be moved or inlined into the file that imports it. But is that too much of a limitation?