Modernize setuptools usage & exclude tests from wheel

[carlsmedstad]

Hey 👋

I maintain the Arch Linux package for zope.testrunner. This is a set of packaging changes that, in my view, are appropriate - the primary one being to stop including tests in the distributed wheel.

As the wheel's primary purpose is to be installed in a virtualenv or on a system, the tests for testrunner itself are unnecessary to include. Including them also makes distro-tooling such as Arch Linux's namcap produce noisy
warnings regarding missing dependencies.

Before these changes:

$ rm -rf build dist/ src/zope.testrunner.egg-info/; python -m build --wheel
...
Successfully built zope.testrunner-6.6.dev0-py3-none-any.whl
$ find build/lib/zope/testrunner/ -iname tests
build/lib/zope/testrunner/tests

After:

$ rm -rf build dist/ src/zope.testrunner.egg-info/; python -m build --wheel
...
Successfully built zope.testrunner-6.6.dev0-py3-none-any.whl
$ find build/lib/zope/testrunner/ -iname tests

The tests are still included in the sdist as per the MANIFEST.in file:

$ rm -rf build dist/ src/zope.testrunner.egg-info/; python -m build --sdist
...
Successfully built zope_testrunner-6.6.dev0.tar.gz
$ tar -xf dist/zope_testrunner-6.6.dev0.tar.gz
$ find zope_testrunner-6.6.dev0/ -iname tests
zope_testrunner-6.6.dev0/src/zope/testrunner/tests

[carlsmedstad]

One additional reason to exclude the tests from the wheel is that they contain invalid Python, and as such make the wheel fail installation without --no-compile:

$ rm -rf build dist/ src/zope.testrunner.egg-info/; python -m build --wheel
...
Successfully built zope.testrunner-6.6.dev0-py3-none-any.whl
$ python -m installer --destdir=tmp_install dist/*.whl
*** Error compiling 'tmp_install/usr/lib/python3.12/site-packages/zope/testrunner/tests/testrunner-ex/sample2/badsyntax.py'...
  File "/usr/lib/python3.12/site-packages/zope/testrunner/tests/testrunner-ex/sample2/badsyntax.py", line 16
    importx unittest  # noqa: E999
            ^^^^^^^^
SyntaxError: invalid syntax

*** Error compiling 'tmp_install/usr/lib/python3.12/site-packages/zope/testrunner/tests/testrunner-ex/sample2/badsyntax.py'...
  File "/usr/lib/python3.12/site-packages/zope/testrunner/tests/testrunner-ex/sample2/badsyntax.py", line 16
    importx unittest  # noqa: E999
            ^^^^^^^^
SyntaxError: invalid syntax

[icemac]

Thank you for your PR. I like the idea to get rid of these warnings during installation.

[d-maurer]

Carl Smedstad wrote at 2024-8-22 22:59 -0700:

... As the wheel's primary purpose is to be installed in a virtualenv or on a system, the tests for testrunner itself are unnecessary to include.

I like to have the tests around for `zopefoundation` packages: the tests often can be used as additional documentation (especially for situations sufficuently rare not to be covered by the "standard" documentation). In my view, including the tests in the distributions is only a marginal overhead. Of course, I am not familiar with distro-tools.

[carlsmedstad]

Carl Smedstad wrote at 2024-8-22 22:59 -0700:
... As the wheel's primary purpose is to be installed in a virtualenv or on a system, the tests for testrunner itself are unnecessary to include.
I like to have the tests around for zopefoundation packages: the tests often can be used as additional documentation (especially for situations sufficuently rare not to be covered by the "standard" documentation). In my view, including the tests in the distributions is only a marginal overhead. Of course, I am not familiar with distro-tools.

I would argue the opposite - that distributing the tests, specifically in the wheel, has marginal visibility/value in terms of documentation while being redundant/inconvenient for most consumers. Especially if they contain uncompilable Python files.

But whatever you align on, I can always patch them out in our package. It's just easier if the patch is upstreamed :)

[icemac]

I'd give it a go, to try out wheels without tests. @dataflake What do you think?

[dataflake]

I don't care either way. But there's a failing test that did not fail before.

[carlsmedstad]

I don't have the possibility of testing locally on Windows, and if I understand correctly the CI is not automatically triggered on push for me, so I'm afraid this makes it difficult for me to iterate on a Windows+PyPy test failure.

With that said, could this failure be timing-related? It only occurring on one platform suggests that. The sleep at src/zope/testrunner/tests/test_threadsupport.py#L121 looks like a candidate for the cause to me.

[mauritsvanrees]

I see no failing tests anymore. Maybe someone reran the failing test and it was solved.

In general in most packages, I very much want to keep the tests in both source distribution and binary wheel. This is because in Plone we run the tests of about 100 packages combined. We are not going to use editable installs for all those packages, but we install the actual distributions from PyPI (plus a few editable installs). And we use zope.testrunner for running the tests.
But in Plone we don't need to run the zope.testrunner tests, so from a Plone perspective I don't mind.

So I am -0 on this change: I prefer to keep the tests, but I don't mind much.

[carlsmedstad]

I see no failing tests anymore. Maybe someone reran the failing test and it was solved.

In general in most packages, I very much want to keep the tests in both source distribution and binary wheel. This is because in Plone we run the tests of about 100 packages combined. We are not going to use editable installs for all those packages, but we install the actual distributions from PyPI (plus a few editable installs). And we use zope.testrunner for running the tests. But in Plone we don't need to run the zope.testrunner tests, so from a Plone perspective I don't mind.

So I am -0 on this change: I prefer to keep the tests, but I don't mind much.

I appreciate your use-case. But, while distributing tests in wheels is (still) somewhat prevalent, in my experience it is definitely a minority of projects doing so, so I would avoid relying on it.

While slightly less convenient for your use-case, but still very viable, you could download and install the sdist from PyPI:

$ pip download --no-binary :all: zope.testrunner
Collecting zope.testrunner
  Downloading zope.testrunner-6.5.tar.gz (155 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Collecting setuptools (from zope.testrunner)
  Using cached setuptools-74.0.0.tar.gz (1.4 MB)
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Collecting zope.exceptions (from zope.testrunner)
  Downloading zope.exceptions-5.1.tar.gz (31 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Collecting zope.interface (from zope.testrunner)
  Downloading zope.interface-7.0.3.tar.gz (252 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Saved ./zope.testrunner-6.5.tar.gz
Saved ./setuptools-74.0.0.tar.gz
Saved ./zope.exceptions-5.1.tar.gz
Saved ./zope.interface-7.0.3.tar.gz
Successfully downloaded zope.testrunner setuptools zope.exceptions zope.interface
$ pip install *.tar.gz
Processing ./setuptools-74.0.0.tar.gz
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Processing ./zope.exceptions-5.1.tar.gz
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Processing ./zope.interface-7.0.3.tar.gz
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Processing ./zope.testrunner-6.5.tar.gz
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Building wheels for collected packages: setuptools, zope.exceptions, zope.interface, zope.testrunner
  Building wheel for setuptools (pyproject.toml) ... done
  Created wheel for setuptools: filename=setuptools-74.0.0-py3-none-any.whl size=1301729 sha256=dd7693ef28f3a714c724af409c1ddf80612caa7179840f896d462983a574b9c8
  Stored in directory: /home/carsme/.cache/pip/wheels/9c/5d/c7/01881acca67b97114f769d8a0d677744f8eb2aad907d1a208d
  Building wheel for zope.exceptions (pyproject.toml) ... done
  Created wheel for zope.exceptions: filename=zope.exceptions-5.1-py3-none-any.whl size=19884 sha256=794e3df5dba6bee9c4a20351ab1c21447b50f5c9efa13ff392fa47a7c625b473
  Stored in directory: /home/carsme/.cache/pip/wheels/8b/93/8d/a8a6204e91ea82ec83d411bee32f85f1b9bc9626b791467f2d
  Building wheel for zope.interface (pyproject.toml) ... done
  Created wheel for zope.interface: filename=zope.interface-7.0.3-cp312-cp312-linux_x86_64.whl size=259581 sha256=0ceca0d1d0a3cd58dc63b6ad4f6f0b92a0ae102ed72bba79aafd653db637b675
  Stored in directory: /home/carsme/.cache/pip/wheels/9f/3c/81/c8234495196a05e2de81195c6f541159e2e0967a910c2c76da
  Building wheel for zope.testrunner (pyproject.toml) ... done
  Created wheel for zope.testrunner: filename=zope.testrunner-6.5-py3-none-any.whl size=233733 sha256=c0e216b580dd6399754b706736498a453b232f6e62749db9d516f5dffe16368c
  Stored in directory: /home/carsme/.cache/pip/wheels/80/28/a7/a37919aa2a91c9f17eb3d1f95829ed316d94073702de10ee0e
Successfully built setuptools zope.exceptions zope.interface zope.testrunner
Installing collected packages: setuptools, zope.interface, zope.exceptions, zope.testrunner
Successfully installed setuptools-74.0.0 zope.exceptions-5.1 zope.interface-7.0.3 zope.testrunner-6.5

[dataflake]

So I am -0 on this change: I prefer to keep the tests, but I don't mind much.

I appreciate your use-case. But, while distributing tests in wheels is (still) somewhat prevalent, in my experience it is definitely a minority of projects doing so, so I would avoid relying on it.

This argument doesn't work here. What @mauritsvanrees means is testing projects that are fully under "our" control, the Plone and Zope folks. We're talking about several hundreds of projects that are set up similarly and almost all of them do ship the tests in the wheel. What you're proposing here is to create a one-off in this specific project that will behave differently. As one of the Zope release managers and a co-maintainer of the toolchain and the methodologies that are used across hundreds of repositories I am very sceptical of one-offs that behave differently from the rest.

[mauritsvanrees]

I am actually surprised that the source distribution contains different files than the wheel. I mean sure, there are different metadata files, and files may be in a different place, and a file like setup.py won't be in the wheel, but I would expect everything within the src directory to be contained in both. I am used to excluding files with MANIFEST.in, and that has the same effect on source dist and wheel. But apparently the way you do it within setup.py has effect on the wheel, but not on the source dist.

The good thing about this is that, as you point out, the tests are still available when you install the source dist, so that is available as fallback. The bad thing is that it matters whether you get an install of the source dist or of the wheel. If you run the tests of 10 packages combined, each with 10 tests, then it could be that on one machine you see 100 tests being run and on another 90. Good luck debugging that.

So it may be best if you can patch these files out on your end.

[dataflake]

After this discussion I am against merging this PR.

[d-maurer]

Jens Vagelpohl wrote at 2024-9-1 00:10 -0700:

After this discussion I am against merging this PR.

+1

[icemac]

I see consensus in rejecting this PR.
Thank you to everyone who joined the discussion and let us come to a conclusion.