UCL-ARC / python-tooling

Python package template for new research software projects
http://github-pages.arc.ucl.ac.uk/python-tooling/
MIT License
43 stars 2 forks source link

Refactor tests #405

Closed dstansby closed 5 months ago

dstansby commented 5 months ago

This makes the tests a bit easier to extend by:

samcunliffe commented 5 months ago

Ah. The conflicts are from me in

(Soz)

dstansby commented 5 months ago

Thanks for comments! I won't have time to fix this in the next couple of weeks, so someone else feel free to take over if you're keen to get this in quicker!

samcunliffe commented 5 months ago

At the moment the approach of making the tests directory a package by adding an __init__.py (presumably so we can use a relative import for helpers module) seems to break the ability to run tests directly by running pytest or pytest -s from the root of the repository (with the latter being what we currently recommend in the developer notes in the README), though tests do still run fine using tox. Specifically I get the following errors on test collection

_______________________________________ ERROR collecting tests/test_git_init.py ________________________________________
ImportError while importing test module '.../python-tooling/tests/test_git_init.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
tests/test_git_init.py:8: in <module>
    from .helpers import gen_package
E   ModuleNotFoundError: No module named 'tests'
______________________________________ ERROR collecting tests/test_package_gen.py ______________________________________
ImportError while importing test module '.../python-tooling/tests/test_package_gen.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
tests/test_package_gen.py:6: in <module>
    from .helpers import gen_package
E   ModuleNotFoundError: No module named 'tests'

I guess this is because the tests package is not being added to the Python path by pytest and so the relative import doesn't work?

I think we can avoid having tests be a package by either wrapping the gen_package function into a fixture in conftest.py

import pytest

def _gen_package(
    path: pathlib.Path, project_config: dict[str, str]
) -> subprocess.CompletedProcess[str]:
    ...

@pytest.fixture
def gen_package():
    return _gen_package

and having the tests which use it accept a gen_package argument [...]

I went for your first suggestion. Adding fixtures as defaults to functions returned by fixtures ended up a bit messy. This way is explicit-er and only repeats one line in the two tests... so not too terrible?