Revamp good practices

[flying-sheep]

The importlib import mode has no* drawbacks while the default mode prepend has a major footgun. That one has one popular workaround which in turn has further drawbacks.

Also one of the sections correctly says “subfolder”, but others misleadingly say “subpackage”. Should I get rid of that too?

*I consider the fact that you can’t import from test files a feature, as it cleans up the distinction between test files and python packages. It encourages to keep your test utils in one location and reduces dependencies between test files.

[flying-sheep]

No idea why there are test failures, but I’m going to assume you sort that out

[The-Compiler]

flying-sheep wants to merge 1 commit into pytest-dev:6.2.x from flying-sheep:patch-1

Note the PR should be against the main branch (and then will be backported to 7.1.x). The 6.2.x branch is long outdated.

It’s misleading to have them there, as the tests directory isn’t (and shouldn’t be a package)

I don't think I agree with that. See Good Integration Practices — pytest documentation - however, they are still not really needed for the example, so I'd be fine with removing them.

No idea why there are test failures, but I’m going to assume you sort that out

From a quick look, they indeed don't look like they are related to your changes. They will probably go away when targetting the correct branch.

[flying-sheep]

See Good Integration Practices — pytest documentation

I’m intimately familiar with that document:

If you need to have test modules with the same name, you might add __init__.py files to your tests folder and subfolders, changing them to packages:

(emphasis mine) and

Note

The --import-mode=importlib option (see Import modes) does not have any of the drawbacks above because sys.path is not changed when importing test modules, so users that run into this issue are strongly encouraged to try it.

[RonnyPfannschmidt]

Unless the default import mode of pytest gets pep420 aware these are absolutely needed

[flying-sheep]

@RonnyPfannschmidt please read the comment I made a few seconds before yours

[RonnyPfannschmidt]

The files missing is a footgun, the failure modes i keep observing when people expand testsuites are a pain, it's simply horrendous to debug for less experienced developers

So unless we fix the default import mode up im opposed to leaving them out

[flying-sheep]

Wait, so pytest doesn’t throw a simple clear error like

you appear to have multiple test files with the same name. Please add --import-mode=importlib to the addopts configuration or make your tests folder into a package by adding empty __init__.py files to each folder with files containing tests.

[RonnyPfannschmidt]

It can't know if it's a mistake or a fatal breakage in multiple ways

[RonnyPfannschmidt]

So neither making it a package nor impormode are a viable fix in multiple cases of import missmatch errors

[nicoddemus]

The files missing is a footgun, the failure modes i keep observing when people expand testsuites are a pain, it's simply horrendous to debug for less experienced developers

So perhaps we should update the docs to at least add a note to that end? Because if we advertise to users that having or not having them is fine depending on the case, we should not be reluctant to remove the __init__.py files from that section on the docs.

[flying-sheep]

My personal preferences would be

1. Ideally make --import-mode=importlib the default
2. If that’s not possible yet (what’s keeping us?) promote it more heavily in explanation/goodpractices.html. Basically start with “add it to addopts”, and then towards the end “if you can’t use it, make tests a package as a workaround”

[nicoddemus]

Unfortunately we found that importlib mode is not perfect either, and would break a ton of test suites if we ever changed the defaults. The main issue is that test modules cannot import from other modules in tests, which is a very common idiom.

I'm OK with changing the good practices docs to mention it as an alternative approach (listing the benefits and drawbacks of each).

[flying-sheep]

The main issue is that test modules cannot import from other modules in tests, which is a very common idiom.

Yeah, test suites relying on this misfeature* would break. If this was my personal project, I’d release a new major version with the change and tell people to do a) things this way and b) use --import-type=legacy for now to unbreak things. But I understand that your stability guarantees are stronger.

*misfeature because now your tests rely on modifying PYTHONPATH in a very specific way, which goes counter to one of the reasons why the src layout is a good idea: You don’t want to add the project dir to PYTHONPATH. Not being able to import from test files is cleaner as you don’t have dependencies between test files, and people will avoid the temptation to do it if it just doesn’t work.

|- src/mypackage/
|  |- __init__.py
|  |- testing.py  # public test utils to be used in packages depending on ours
|  '- _testing.py  # private test utils that somehow can’t be fixtures
'- tests/
   |- conftest.py  # private test fixtures used in most places
   |- test_stuff.py
   '- test_feature/
      |- conftest.py  # private test fixtures/overrides to test that feature
      '- test_stuff.py

Alternative layout

The testing global namespace package is a common way to distribure test utils.

Just define a subpackage with the same name as your main package.

|- src/
|  |- mypackage.py
|  '- testing/mypackage.py  # public test utils to be used in packages depending on ours
'- tests/
   |- conftest.py  # private test fixtures used in most places
   |- test_stuff.py
   '- test_feature/
      |- conftest.py  # private test fixtures/overrides to test that feature
      '- test_stuff.py

I'm OK with changing the good practices docs to mention it as an alternative approach (listing the benefits and drawbacks of each).

Great, I’ll see if I can come up with something :D

[flying-sheep]

OK, I took a shot at updating goodpractices.rst. Maybe someone has opinions.

• I think it can be further slimmed by moving the huge .. note:: block labelled test package name to what is now a footnote, and promote that footnote to its own page or header.

  Then goodpractices.rst will only contain good practices and pointers to the default.

• I also didn’t mention the testing.mypkg/mypkg.testing approach to “what if I want to import test utils”, that should probably also go in.

[nicoddemus]

Thanks @flying-sheep for the follow up.

Left a few comments, please take a look.

[nicoddemus]

I personally prefer that we use setuptools in our examples, because it is more traditional; using alternatives might give the impression that pytest recommends/only works with hatchling, or support requests on how to do the same with poetry, etc.

[flying-sheep]

hmm, maybe we can also use tabs like in the packaging tutorial?

I don’t recommend setuptools as it’s:

• slow
• messy (leaves a .egg-info folder lying around in your workspace, emits a bunch of unnecessary log messages during build)
• hard to configure (a bunch of legacy config files like MANIFEST.in, and misleadingly named config keys)
• no longer the first choice on the packaging tutorial

but of course I can leave it if you prefer. I’d still get rid of setup.cfg since setuptools these days supports PEP 621. Unfortunately configuring it with more than just basic metadata is labelled as “beta”, e.g. this:

[tool.setuptools.packages.find]
where = ["src"]  # ["."] by default

[nicoddemus]

no longer the first choice on the packaging tutorial

Ahh thanks for pointing that out, didn't know that has changed. If the packaging tutorial recommends hatchling as first choice, I think we should follow suit then.

[flying-sheep]

In case you would like some context, I followed that discussion, and the reasoning went something like that:

1. we don’t want to promote any standards following build backend above all others
2. so we use tabs
3. but since the first (auto-selected) tab is kind of an endorsement, we need something there that is good for the vast majority of people, i.e. ideal for pure python packages
4. because of the above mentioned drawbacks of setuptools, the first tab will be hatchling, because it provides a better experience for people creating a pure python package than setuptools, has a plugin to use setuptool-scm for versioning (hatch-vcs)

Hatchling doesn’t have a sufficient amount of plugins or example code yet to allow a highly custom build like numpy or numba or so, but for anything with purely static build configuration, it’s a very good choice.

[nicoddemus]

Thanks @flying-sheep!

[nicoddemus]

Anybody else wants to review this?

Otherwise I plan to merge this tomorrow.

[nicoddemus]

Thanks again @flying-sheep!