Separate testing utils from tests

[flying-sheep]

Pytest supports two test layouts, in-package and outside-of-package. I prefer the outside-of-package mode outlined here: https://docs.pytest.org/en/6.2.x/goodpractices.html

Scanpy currently mixes test utils with tests, but pytest’s test files (test_*.py and conftest.py) aren’t Python modules one is supposed to import from.

To clean things up, we can refactor scanpy to a in-package structure:

• pyproject.toml: add addopts = ['--import-mode=importlib'] to [tool.pytest.ini_options]
• scanpy/tests/__init__.py during implementation, make it throw an error on import so we can make sure nobody imports things from there, then delete
• scanpy/tests/**/__init__.py delete
• scanpy/test_utils/ or scanpy/testing/
  • __init__.py: leave empty for now, later add public, documented test utils
  • _private.py add private test utils that can be imported in our tests, such as the @needs_somepackage decorators

Later we can decide if we want to keep the in-package layout or switch to the outside-of-package layout

[ivirshup]

@flying-sheep, do you know of a large package (ideally in our dependencies) which uses the directory structure you're advocating for? I'd ideally like to have another repo to look at/ crib from for test organization strategies.

[flying-sheep]

Most things?

• pandas has https://github.com/pandas-dev/pandas/tree/main/pandas/_testing

• numpy has https://github.com/numpy/numpy/tree/main/numpy/testing

  It also literally has _private.py which is awesome since I came up with that on the fly!

They both have __init__.py files in their tests directory for which I can forgive them since they probably didn’t know about --import-mode=importlib (or it didn’t exist) when they created their test suites. They probably ran into some problem about test files having identical names and hacked their way around it. But we can do better since we know better: --import-mode=importlib just fixes problems like that, no caveats.

[ivirshup]

Oh, I specifically meant tests not {package}/tests

Though looking through the pandas test it does look like there are fewer internal imports than I recall.

[flying-sheep]

As said:

Later we can decide if we want to keep the in-package layout or switch to the outside-of-package layout

[ivirshup]

Could you point some projects who's testing layout you'd like to emulate? I'd ideally like to have something to look at for reference. I would also like something I could try out, since I recall "acceptable test discovery arguments" can be a bit fiddly with pytest.

For modules of test utils, I think I'd go scanpy.testing and scanpy.testing._marks/ scanpy.testing._pytest or something like that.

[flying-sheep]

I literally never had problems with test discovery, so idk what to look for.

As said: Numpy and pandas have separated their testing utils from their tests. For the time being I want just that, no change to where the tests are. Would you accept a PR that simply moves the test utils into private submodules of scanpy.testing and switches the import mode to (future default, drawback-less) importlib?

Any change to the test layout can come later or never. I’d like to follow pytest’s recommendation (/src/scanpy/ and /tests/) but this issue is orthogonal to that.

[ivirshup]

Can you point to a package whose test organization you would like our tests to emulate?

I find pytests docs rather hard to navigate and would really prefer to see an example of what you're advocating for. From your description above I had thought you didn't want to emulate pandas use of conftest.

---

Would you accept a PR that simply moves the test utils into private submodules of scanpy.testing

I'd lean towards it, but I fully expect issues like #685 to come up. This is why I'd like to see a working example of what you want to work towards.

---

switches the import mode to (future default, drawback-less) importlib?

Is it definitely the future default? It looks like they are walking that back. Current versions of pytest docs say:

We intend to make importlib the default in future releases, depending on feedback.

Where it previously said:

We intend to make importlib the default in future releases.

[flying-sheep]

Can you point to a package whose test organization you would like our tests to emulate?

• pytest: https://github.com/pytest-dev/pytest/tree/main/testing
• loompy: https://github.com/linnarsson-lab/loompy/tree/master/tests

The others have their tests in the package, and just have that useless __init__.py in the tests directory because of either cargo culting it or becaue they know that makes setuptools’ discover_packages or so find it.

From your description above I had thought you didn't want to emulate pandas use of conftest.

What do you mean specifically?

Pandas are defining special pytest functions/variables there and fixtures, which is what it’s for. I’d probably judge that we don’t need all those fixtures for our complete test suite and move some of them to a smaller scope (e.g. tests/io/conftest.py or so)

I'd lean towards it, but I fully expect issues like #685 to come up. This is why I'd like to see a working example of what you want to work towards.

Actually I think we can fix that: the docs for pytest_addoption say it has to be defined at the tests root directory which can be configured using the rootpath config option.

Is it definitely the future default? It looks like they are walking that back.

The question is if they remove the others or not, I think: pytest-dev/pytest#7245

---

My intention here is to make clear which code lives under which laws. Pytest world is very different from Python module world. The presence of __init__.py fools people into thinking that we’re dealing with python packages/modules here, but that’s not true. The way pytest works is pretty simple:

1. it collects all test modules (test_*.py files, no directories) and determines which conftest.py files, plugins, … apply to which test module
2. it collects all tests in those modules and checks which fixtures they need
3. it sets up and tears down fixtures according to the needs of each test and executes the tests

accepting that makes it easier to reason about how our test suite works.

[ivirshup]

I'm not sure that pytest issue convinced me importlib is a good thing... A few of the recent pytest developer comments that caught my eye were:

FWIW I'm convinced at this point that we should not change the import-mode to importlib anytime soon, some things just get harder to setup for out-source testing setups.

We're interested in making our tests future-proof

One way to do that is to add addopts = --importmode=prepend to your pytest.ini file. We don't intend to remove the prepend mode in the future at all.

FTR, IMO we probably should not change the default to importlib anytime soon (or ever)...

I would be up for a PR that only moved things outside of the test module. Things that would probably slow down or prevent merging would include:

• Changing the import mode
• Changing organization of tests
• Changing calling conventions for pytest

All of these things seem like they can be done in other PRs easily after test utilities are moved. Right?

[flying-sheep]

I’m pretty happy with #2235 for several reasons. Let’s call!