[CI] Remove the need for the tmp_src fixture
#2968
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
abravalheri
force-pushed
the
remove-need-for-tmp-src-fixture
branch
from
379f32c
to
f36a128
Compare
abravalheri
changed the title
tmp_src fixturetmp_src fixture
abravalheri
force-pushed
the
remove-need-for-tmp-src-fixture
branch
from
f36a128
to
56751c9
Compare
2 tasks
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
force-pushed
the
remove-need-for-tmp-src-fixture
branch
from
56751c9
to
9c7e59c
Compare
jaraco
reviewed
Jan 29, 2022
|
|
||
| @pytest.fixture(scope="session") | ||
| def setuptools_wheel(tmp_path_factory, request): | ||
| with contexts.session_locked_tmp_dir(tmp_path_factory, "wheel_build") as tmp: |
There was a problem hiding this comment.
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change supersedes #2922.
Summary of changes
As discussed in #2922, the
tmp_srcfixture copies the.gitrepository, which is error prone and may increase testing time.I noticed that this fixture was used only in
test_virtualenvandtest_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>orpython setup.py install/develop. It also uses a lockfile approach discussed in thepytest-xdisthomepage 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:
pytest_virtualenvplugin withjaraco.envsas the later was already being used in other tests (this unification of libraries allowed sharing fixtures).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.cfgfor the changes in test dependencies).Closes #2921.
Pull Request Checklist
changelog.d/.(See documentation for details)