Add integration tests focusing on installing sdists via pip by abravalheri · Pull Request #2863 · pypa/setuptools

abravalheri · 2021-11-08T14:58:48Z

Motivation

As shown in #2849, sometimes it is tricky to identify when a change introduces backward compatibility problems, specially considering that a lot o packages still rely on distutils.

Integration tests able to identify when such rare errors scenarios happen are very useful.

Summary of changes

This PR introduces one specific type of integration tests: installing sdists using pip

Although very specific, this class of tests works very well for setuptools, since it exercises the API exposed by users that want to not only include binary extensions, but also customise their compilation process.

The configurations for pytest, tox and Github Actions were changed in such a way that integration tests will only run before a release (not on regular merge or push).

The selection of Python packages being used for the test was only kept small, to minimise the "cost" of the test (in terms of time and resources). I tried to choose between popular packages that would cover most of the parts of setuptools API, however there is always room for improvement, and the list can be changed to better suit the needs of the project/community.

Closes #2862

Evidence that the proposed changes work

Testing the tests is something always difficult. In order to provide evidence that the proposed test and tox/pytest/CI configuration changes work, I have created the following repository:

abravalheri/experiment-setuptools-integration-tests

This repository was created by taking the important parts from setuptools/this PR and organising them in a way that Github Actions would run as expected, as show in the CI logs. More information is available on the README.

Pull Request Checklist

Changes have tests
News fragment added in changelog.d/.
(See documentation for details)

abravalheri · 2021-11-08T15:17:45Z

conftest.py

+
+    if config.option.integration:
+        # Assume unit tests and flake already run
+        config.option.flake8 = False


Running flake again during integration would take more time.

Instead, you can pass -p no:flake8 to the pytest run.

Actually, rather than install the plugin and then disable it, you can instead provide separate sets of extras for the integration tests and just not include the unwanted plugins when doing integration tests.

Thanks @jaraco. I added a testing-integration to extras_require.

There is some small amount of duplication between testing-integration and testing but hopefully it would not add a lot of double bookkeeping.

abravalheri · 2021-11-08T15:18:45Z

pytest.ini

 	ignore:.* is an invalid version and will not be supported::pkg_resources
+
+	# This will result in an error in the integration tests if not ignored:
+	ignore:.*Coverage disabled via --no-cov switch!.*


pytest-cov emits a warning if it is disabled. That warning is converted to error by the strict pytest configuration.

Instead, you can pass -p no:cov to the pytest run.

abravalheri · 2021-11-08T15:20:32Z

tox.ini

+	{[testenv]passenv}
+	DOWNLOAD_PATH
+commands =
+	pytest --integration {posargs:-vv --durations=10 --no-cov setuptools/tests/integration}


By pointing to the specific setuptools/tests/integration directory by default we avoid time collecting/analysing the other tests (and also avoid displaying a lot of skips in the pytest output)

abravalheri · 2021-11-08T15:23:27Z

According to the CI logs in abravalheri/experiment-setuptools-integration-tests, the integration tests as implemented in this PR take approx 16min to run:

937.77s call     setuptools/tests/integration/test_pip_install_sdist.py::test_install_sdist[pandas-v.LATEST]
324.99s call     setuptools/tests/integration/test_pip_install_sdist.py::test_install_sdist[numpy-v.LATEST]
26.96s call     setuptools/tests/integration/test_pip_install_sdist.py::test_install_sdist[brotli-v.LATEST]
24.98s call     setuptools/tests/integration/test_pip_install_sdist.py::test_install_sdist[kiwisolver-1.3.2]
13.24s call     setuptools/tests/integration/test_pip_install_sdist.py::test_install_sdist[sphinx-v.LATEST]
8.49s call     setuptools/tests/integration/test_pip_install_sdist.py::test_install_sdist[botocore-v.LATEST]
6.80s call     setuptools/tests/integration/test_pip_install_sdist.py::test_install_sdist[pytest-v.LATEST]
5.51s call     setuptools/tests/integration/test_pip_install_sdist.py::test_install_sdist[mypy-v.LATEST]
4.22s call     setuptools/tests/integration/test_pip_install_sdist.py::test_install_sdist[pip-v.LATEST]

jaraco · 2021-11-12T20:01:48Z

setuptools/tests/integration/test_pip_install_sdist.py

+PIP = (sys.executable, "-m", "pip")
+SDIST_OPTIONS = (
+    "--ignore-installed",
+    "--no-build-isolation",


I'd prefer to use the build-isolation, as that's the most common scenario. Is there another option that can be passed that means "if requested, get setuptools from the current directory"?

I asked about this possibility in the pip discord, and based on the answers there, my conclusion is that if we want to force a specific version of setuptools we have to keep the --no-build-isolation flag.

To simulate build isolation we can create a separated virtualenv and install the build deps there.

An alternative suggestion was to try PIP_CONSTRAINTS.
To test this suggestion, I locally tried to:

Build a wheel of the development version of setuptools via python -m build --wheel
(PIP_CONSTRAINTS do not accept editable installs)

Create a constraints.txt file with:
setuptools @ file:///home/<...>/setuptools/dist/setuptools-58.5.3.post20211113-py3-none-any.whl

Run:

DISTUTILS_DEBUG=1 pip --verbose install -c constraints.txt dist/Brotli-1.0.9.zip

DISTUTILS_DEBUG=1 pip --verbose install -c constraints.txt --use-pep517 dist/Brotli-1.0.9.zip

DISTUTILS_DEBUG=1 PIP_CONSTRAINTS=./constraints.txt pip --verbose install --use-pep517 dist/Brotli-1.0.9.zip

For (1) pip does not seem to respect the isolation very much, also there is nothing in the stdout/stderr that could be used to assert during the tests that the correct version of setuptools is being used.

For both (2) and (3), pip ignores the constraint and re-downloads setuptools-58.5.3-py3-none-any.whl from PyPI.

The reason why I went with pip install -t instead of creating a virtual environment was purely to avoid the overhead and try to speed up the integration tests a little bit.

With that in mind, something I can do is to create a virtualenv for each test case and install the current version of setuptools under development in it. This way we can simulate pip's isolation, despite using the --no-build-isolation flag.

jaraco

This is looking quite good. Nice work. I made a few comments, none critical. Have a look, make any changes you'd like to make now, and we can get it merged and iterate.

abravalheri · 2021-11-13T17:40:45Z

Hi @jaraco, thank you very much for the review and suggestions.

This is a summary of the changes:

in the commits until 3c55e74, I just addressed your suggestions by adding a new set of extras for the integration tests, such that disabling pytest-cov and pytest-flake8 is no longer necessary -- associated CI run
in 738e1c8 I changed the tests to explicitly create a virtual environment for each package being compiled from the sdist. The idea here was to emulate isolation, when the --no-isolation was necessary to ensure the correct version of setuptools was being used -- associated CI run
The last commit is not exactly necessary since I just extracted some code that is not exactly related to installing things from PyPI -- associated CI run

When using virtualenvs I had to change a little bit the tests in abravalheri/experiment-setuptools-integration-tests, mostly:

- correct_setuptools = os.getenv("PROJECT_ROOT") or SETUPTOOLS_ROOT
+ correct_setuptools = (
+    "git+https://github.com/pypa/setuptools@main#egg=setuptools"
+ )
  run([*venv_pip, "install", "-Ie", correct_setuptools])
  run([*venv_pip, "install", *SDIST_OPTIONS, sdist])

Please feel free to revert to any of the preceding commits. I don't have a strong preference regarding using virtualenv vs pip install -t.

The selection of packages used in the integration test is arbitrary, and can be changed. The main criteria used was the time to build, and the number of "non-Python" dependencies. The only exception was numpy, due to its significance to the ecosystem.

Instead of disabling pytest plugins, simply don't install them

This is more reliable then simply globing (and will also work in other operating systems)

abravalheri · 2022-01-06T17:31:31Z

Let's try to see if this works:

@Mergifyio update

mergify · 2022-01-06T17:32:11Z

update

✅ Branch has been successfully updated

abravalheri commented Nov 8, 2021

View reviewed changes

abravalheri marked this pull request as ready for review November 8, 2021 15:35

jaraco reviewed Nov 12, 2021

View reviewed changes

jaraco approved these changes Nov 12, 2021

View reviewed changes

abravalheri mentioned this pull request Nov 13, 2021

BLD: in conda, pin setuptools to a known working version numpy/numpy#20299

Merged

abravalheri force-pushed the integration-tests branch from 4529c2c to 3c55e74 Compare November 13, 2021 12:01

abravalheri force-pushed the integration-tests branch 2 times, most recently from 1db94de to 79a251d Compare November 14, 2021 14:47

This was referenced Nov 16, 2021

Add default values for the packages and py_modules options #2888

Closed

Towards no-config packaging: Automatic discovery of packages, py_modules and name #2894

Merged

abravalheri added 11 commits December 21, 2021 02:22

Configure pytest to enable/disable integration tests

e645104

Add tox environment for integration tests

8e94277

Run integration tests before release on CI

266b905

Add news fragment about integration tests

159e9a1

Disable pytest plugins in integration tests via -p no:*

c42cbb1

Use separated "extras" for integration tests

698dd82

Instead of disabling pytest plugins, simply don't install them

Fix flake8 errors for conftest.py

60dcba4

Simulate pip's isolation using virtualenv

48361da

Use shutil to find executable

c83f83b

This is more reliable then simply globing (and will also work in other operating systems)

Separate some reusable integration helpers

01504a0

abravalheri force-pushed the integration-tests branch from 56747de to d246d8a Compare December 21, 2021 02:26

Use '=' as setup.cfg delimiter for extras option

6f48cb7

abravalheri force-pushed the integration-tests branch from d246d8a to 6f48cb7 Compare December 21, 2021 11:34

abravalheri mentioned this pull request Dec 25, 2021

[CI] Remove the need for the tmp_src fixture #2968

Merged

2 tasks

Merge branch 'main' into integration-tests

846a6c4

abravalheri added 2 commits January 6, 2022 18:55

Add SETUPTOOLS_USE_DISTUTILS configuration to integration tests

2c9205f

Allow integration tests to be manually triggered in GitHub Actions

818a6d9

abravalheri merged commit 3f1caa8 into pypa:main Jan 6, 2022

abravalheri deleted the integration-tests branch January 6, 2022 21:00

Uh oh!

Conversation

abravalheri commented Nov 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Summary of changes

Evidence that the proposed changes work

Pull Request Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abravalheri commented Nov 8, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abravalheri Nov 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abravalheri Nov 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaraco left a comment

Choose a reason for hiding this comment

Uh oh!

abravalheri commented Nov 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abravalheri commented Jan 6, 2022

Uh oh!

mergify bot commented Jan 6, 2022

✅ Branch has been successfully updated

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

abravalheri commented Nov 8, 2021 •

edited

Loading

abravalheri Nov 13, 2021 •

edited

Loading

abravalheri Nov 13, 2021 •

edited

Loading

abravalheri commented Nov 13, 2021 •

edited

Loading