TEST_DIR location hardcoded to installation dir

[drew-parsons]

System

• Custodian version: 2024.1.9
• Python version: 3.11
• OS version: Linux (debian unstable)

Summary

• custodian hard codes the location of test files in __init__.py, setting TEST_DIR and TEST_FILES from the installation dir defined by __file__.
• this interferes with testing for linux distribution packages, and with CI testing of installed packages whether the test files are not located in /usr/lib/python3/dist-packages/custodian/../tests

Example code

cd tests
pytest-3

Error message

ERROR: test (tests.cp2k.test_handlers.HandlerTests.test)
Ensure modder works
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/projects/custodian/tests/cp2k/test_handlers.py", line 43, in setUp
    shutil.copy(f"{TEST_FILES_DIR}/cp2k.inp.orig", f"{TEST_FILES_DIR}/cp2k.inp")
  File "/usr/lib/python3.11/shutil.py", line 419, in copy
    copyfile(src, dst, follow_symlinks=follow_symlinks)
  File "/usr/lib/python3.11/shutil.py", line 256, in copyfile
    with open(src, 'rb') as fsrc:
         ^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/usr/lib/python3/dist-packages/tests/files/cp2k/cp2k.inp.orig'

Suggested solution

• It would be better to not have TEST_DIR handled by custodian/__init__.py in the first place. Tests and library code should be separate. Test can be run from the actual test dir, in which the location is known (e.g. os.curdir). The pattern used for pymatgen could work, defining conftest.py
• If __init__.py needs to keep a definition of TEST_DIR, then it should be configurable with an environment variable, similar to TEST_FILES_DIR used for pymatgen

See also pymatgen PR#3227

It would be sufficient to replace the current

custodian/custodian/__init__.py

Line 20 in 27dfee3

TEST_DIR = f"{ROOT}/tests"

with

TEST_DIR = os.getenv('CUSTODIAN_TEST_DIR', default=f"{ROOT}/tests")

[janosh]

Thanks for the thorough bug report and suggested solutions! Very helpful.

I moved TEST_DIR to conftest.py in #311. Do you think we still should make it overridable via env var? Can't see any use cases myself.

[drew-parsons]

I agree, in the context of controlling it in conftest.py from test dir, the env var is redundant, so the patch should work fine. It would be too odd to separate the test files from the test dir.