Skip to content

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

Merged
abravalheri merged 8 commits intopypa:mainfrom
abravalheri:remove-need-for-tmp-src-fixture
Jan 7, 2022
Merged

[CI] Remove the need for the tmp_src fixture#2968
abravalheri merged 8 commits intopypa:mainfrom
abravalheri:remove-need-for-tmp-src-fixture

Conversation

@abravalheri
Copy link
Copy Markdown
Contributor

@abravalheri abravalheri commented Dec 25, 2021

This change supersedes #2922.

Summary of changes

As discussed in #2922, the tmp_src fixture copies the .git repository, which is error prone and may increase testing time.

I noticed that this fixture was used only in test_virtualenv and test_distutils_adoption, mainly to populate a virtual environment with the version of setuptools that is currently under test.

This PR proposes building sdist/wheel artifacts from the source tree and using them to populate these virtualenvs, instead of pip install <setuptools directory> or python setup.py install/develop. It also uses a lockfile approach discussed in the pytest-xdist homepage to make these artifacts session-scoped fixtures that will run only once and be available to all tests (that can use them in parallel for installation).

As a nice side-effect, I also took the opportunity to:

  • Replace the problematic pytest_virtualenv plugin with jaraco.envs as the later was already being used in other tests (this unification of libraries allowed sharing fixtures).
  • Improve isolation for some tests that where inadvertently using the project root as CWD for building dummy distribution objects.

This change might introduce some merge conflicts with other PRs that I have open (e.g. #2863), but I can quickly fix them (the conflicts are mostly due to lines being added/removed to the same region of setup.cfg for the changes in test dependencies).

Closes #2921.

Pull Request Checklist

@abravalheri abravalheri force-pushed the remove-need-for-tmp-src-fixture branch from 379f32c to f36a128 Compare December 25, 2021 12:08
@abravalheri abravalheri changed the title [Tests] Remove the need for the tmp_src fixture [CI] Remove the need for the tmp_src fixture Dec 25, 2021
@abravalheri abravalheri force-pushed the remove-need-for-tmp-src-fixture branch from f36a128 to 56751c9 Compare December 25, 2021 12:41
@abravalheri abravalheri marked this pull request as ready for review December 25, 2021 13:23
They should be build once for each session and be able to be re-used in
parallel (assuming read-only) for all tests.

This is useful when dealing with virtual environments
… and change it to install the pre-build setuptools wheel (fixture)
instead of installing from the source tree
Instead of re-building/installing setuptools from the source tree
every time, the tests now rely on the venv, wheel and sdist fixtures
(the venv fixture is populated from sdist/wheel).

Moreover migrate `test_virtualenv` to use `jaraco.envs`
(so it uses the same libraries ad `test_distutils_adoption`).
Now that all tests use `jaraco.envs`, there is no need to depend on
`pytest_virtualenv`.
Some tests are running the build process using setuptools own directory
as cwd. This impacts the build process and also leave behind artifacts
produced during tests (like .egg-info folders)
It seems that the release action in the CI was running even when the
cygwin tests did not pass... It is likely we want it to pass first.
@abravalheri abravalheri force-pushed the remove-need-for-tmp-src-fixture branch from 56751c9 to 9c7e59c Compare January 6, 2022 22:52
@abravalheri abravalheri merged commit ab9ca3f into pypa:main Jan 7, 2022
@abravalheri abravalheri deleted the remove-need-for-tmp-src-fixture branch January 7, 2022 00:10

@pytest.fixture(scope="session")
def setuptools_wheel(tmp_path_factory, request):
with contexts.session_locked_tmp_dir(tmp_path_factory, "wheel_build") as tmp:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this operation has the desired effect. Since it's a session fixture, I'd expect it to only be run once per session, even if tests are run in parallel, so there shouldn't be any race conditions on generating the artifact even if multiple tests solicit it. Maybe I gave a bad hint when I was pondering the need for a lockfile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] tmp_src fixture seems to be copying everything including .git

2 participants