reuse-tool icon indicating copy to clipboard operation
reuse-tool copied to clipboard

Use importlib mode for pytest

Open khardix opened this issue 1 year ago • 2 comments

Attempt at implementing #993.

  • Treat tests/ as namespace and relative-import conftest.
  • Switch the mode in pyproject.toml.
  • [x] Added a change log entry in changelog.d/<directory>/.
  • [x] My changes do not contradict the current specification.
  • [x] I agree to license my contribution under the licenses indicated in the changed files.

khardix avatar Jul 08 '24 13:07 khardix

Thank you very much for your contribution! I like that this would bring us closer to best practice in regards to pytest. Would you mind fixing the failing tests so I can do a proper review?

floriansnow avatar Jun 22 '25 10:06 floriansnow

Rebased on the current main. I was unable to convince mypy to follow the now-relative .conftest import without turning tests/ into a proper python package, so there is a new empty __init__.py file in there. I hope this is not an issue. :crossed_fingers:

khardix avatar Jun 23 '25 09:06 khardix

@floriansnow Ping, this should be ready to review.

khardix avatar Jul 04 '25 13:07 khardix

Hi @khardix, thanks for the PR.

I will close this because it doesn't really seem like an improvement over the status quo, and the relevant pytest web page now says:

Initially we intended to make importlib the default in future releases, however it is clear now that it has its own set of drawbacks so the default will remain prepend for the foreseeable future.

And importantly, under 'disadvantages', it lists this:

Testing utility modules in the tests directories (for example a tests.helpers module containing test-related functions/classes) are not importable. The recommendation in this case is to place testing utility modules together with the application/library code, for example app.testing.helpers.

Because we do have 'test utility [functions]' in conftest.py, we need to be able to import them. Moving this stuff to the module being tested seems a little silly to me.

Adding __init__.py to the tests/ directory remains disrecommended by pytest, so that's sadly a non-starter.

carmenbianca avatar Sep 05 '25 13:09 carmenbianca

Acknowledged. I was proposing this change because the prepend method was failing when building the Fedora rpm (for some reason), and with the importlib method being the future™ at that point, it seemed to be the best solution moving forwards. With the rescinding of that proposal from the pytest side, I'll go back to debugging why it fails for me.

khardix avatar Sep 09 '25 14:09 khardix