Pytest Style: test_imports.py #1473
Conversation
| if self.is_excluded(fpath): | ||
| continue | ||
| yield self._run_test_relative_import, fpath | ||
| # yield self._run_test_relative_import, fpath |
| return paths | ||
|
|
||
|
|
||
| @pytest.mark.parametrize('testing_module', get_file_paths()) |
There was a problem hiding this comment.
looks like a sensible and pretty clean way to turn the yield-based test into a parametrized one, good!
There was a problem hiding this comment.
It is rather neat indeed!
|
|
||
| path_to_testing_modules = MDAnalysisTests.__path__[0] | ||
| # Exclusion path relative to MDAnalysisTests | ||
| exclusions = ['/plugins'] |
There was a problem hiding this comment.
Also add '/data' to exclusions – will save a tiny bit in the os.walk and makes clear that nothing in the data directory should be looked at. (There are no .py files in data so nothing changes overall anyway.)
|
@kain88-de @richardjgowers Review please. |
|
|
||
| path_to_testing_modules = MDAnalysisTests.__path__[0] | ||
| # Exclusion path relative to MDAnalysisTests | ||
| exclusions = ['/plugins', '/data'] |
There was a problem hiding this comment.
Constants in the global scope should be in capital. PATH_TO_TESTING_MODULES and EXCLUSIONS. I'd like to see this changed but it is not blocking.
There was a problem hiding this comment.
Yeah I'd have preferred this too, it makes it obvious where things have come from
| return paths | ||
|
|
||
|
|
||
| @pytest.mark.parametrize('testing_module', get_file_paths()) |
There was a problem hiding this comment.
It is rather neat indeed!
Fixes #
Changes made in this Pull Request:
PR Checklist