pypa / setuptools

Official project repository for the Setuptools build system
https://pypi.org/project/setuptools/
MIT License
2.34k stars 1.14k forks source link

Deduplicate testing dependencies by dropping `[testing-integration]` #4283

Closed Avasam closed 1 month ago

Avasam commented 2 months ago

Summary of changes

In https://github.com/pypa/setuptools/pull/4257#discussion_r1514754970, @abravalheri mentioned that:

[...] I think we just remove the testing-integration and use testing everywhere...

Although it might add a bit of overhead for the integration tests, it will simplify and streamline the setup... This way we reduce the divergence with skeleton. The additional overhead in the integration tests should not be too much, and the integration tests just run before the releases anyway.

This PR is doing just that.

Accepting this closes #4282 , which was my original idea.

Pull Request Checklist

[PR docs]: https://setuptools.pypa.io/en/latest/development/developer-guide.html#making-a-pull-request

abravalheri commented 1 month ago

@Avasam, when running the tests I was having an error[^1]:

setuptools/config/_validate_pyproject/formats.py:180: error: Cannot find implementation or library stub for module named "trove_classifiers"  [import-not-found]
setuptools/config/_validate_pyproject/formats.py:180: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 1 error in 1 file (checked 3 source files)

As a workaround, I added d98a9d92a. Does it make sense? Do you know why https://github.com/pypa/setuptools/blob/6ee23bf0579c52e1cbe7c97fc20fd085ff2a25c7/mypy.ini#L16 does not work?

[^1]: pipx run --python python3.11 tox -e integration

Avasam commented 1 month ago

@abravalheri I remember having trove_classifier be picked up as an import previously, but I don't remember if/how I fixed it. Or if I just fixed it locally by installing trove_classifier then the CI did something different that didn't need it explicitly installed.

Either way, the difference that I notice now is that integration tests will also run checkdocks, cov, mypy, ruff and perf whereas previously it didn't.

abravalheri commented 1 month ago

@Avasam 6c118beac827d233e0d5af76a1555092f631ce70 is a way of disabling those tests. Does it make sense for you, or should I revert that commit?

I believe that mypy, ruff, checkdocs already run on other tests so there is no need for repeating it... The cov plugin is more complicated. There is a discussion in https://github.com/pypa/setuptools/pull/3643, but the TL;DR of my problem is that pytest-cov is not reporting the real coverage of those tests, because we use nested subprocess (and therefore enabling the flag would bring a false sense of completeness). I haven't found a good way around of it.

Avasam commented 1 month ago

Hmm, in a way, I wonder if we're just moving the duplication from one file to another.

Duplicating the declaration of dependencies/tests not to be repeated, where before we were duplicating shared dependencies.

Whilst leaving the door open to forgetting to disable a test. Not that I see new tooling being added too often, so it'd probably be caught eventually.


Other than that, it does bring setup cfg closer to the skeleton and dedupes dependencies in that file.

abravalheri commented 1 month ago

Yeah, that is true... well, it is an optimisation thing, we can drop it at some point. I think that if the cov problem with integration tests is ever solved, it would be a nice moment to drop it.