Skip to content

ci: try using tox-uv#738

Merged
gaborbernat merged 5 commits intopypa:mainfrom
henryiii:henryiii/ci/ub
Mar 25, 2025
Merged

ci: try using tox-uv#738
gaborbernat merged 5 commits intopypa:mainfrom
henryiii:henryiii/ci/ub

Conversation

@henryiii
Copy link
Contributor

@henryiii henryiii commented Feb 22, 2024 •
edited
Loading

Speeds up CI a little bit. Closes #858.

@gaborbernat gaborbernat marked this pull request as draft February 22, 2024 23:23
@henryiii henryiii force-pushed the henryiii/ci/ub branch 2 times, most recently from b03b9bd to e8bb33c Compare September 6, 2024 20:22
@FFY00
Copy link
Member

FFY00 commented Feb 4, 2025

Perhaps we could try using https://github.com/tox-dev/tox-gh?

@henryiii henryiii mentioned this pull request Feb 5, 2025
Merged
@henryiii
Copy link
Contributor Author

henryiii commented Feb 5, 2025

The hope here was to make the tests a bit faster; tox-gh might shorten the config, but wouldn't be faster (AFAIK). I don't understand why one test now uninstalls setuptools when it didn't before when running with tox-uv.

@henryiii henryiii marked this pull request as ready for review March 9, 2025 20:54
layday
layday reviewed Mar 9, 2025
View reviewed changes
.github/workflows/reusable-pytest.yml Outdated
uses: actions/setup-python@v5
with:
python-version: 3.9
python-version: 3.12
Copy link
Member

@layday layday Mar 9, 2025

Choose a reason for hiding this comment

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

Suggested change
python-version: 3.12
python-version: "3.12"

Copy link
Contributor Author

@henryiii henryiii Mar 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I think this isn't needed anymore, since I changed tox to be installed via uv tool. Let's check that first.

@henryiii
Copy link
Contributor Author

henryiii commented Mar 9, 2025 •
edited
Loading

Interesting, the three failures are: AssertionError: Expected 'get_paths' to be called once. Called 4 times. from test_can_get_venv_paths_with_posix_local_default_scheme[False].

@henryiii
tests: skip if venv is present
bbfb1cc 
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii henryiii requested a review from layday March 11, 2025 22:04
layday
layday reviewed Mar 13, 2025
View reviewed changes
tests/test_env.py


@pytest.mark.skipif('posix_local' not in SCHEME_NAMES, reason='workaround for Debian/Ubuntu Python')
@pytest.mark.skipif('venv' in SCHEME_NAMES, reason='different call if venv is in scheme names')
Copy link
Member

@layday layday Mar 13, 2025

Choose a reason for hiding this comment

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

Isn't venv always in SCHEME_NAMES, since 3.11?

Copy link
Contributor Author

@henryiii henryiii Mar 13, 2025

Choose a reason for hiding this comment

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

How does this test pass, then?

build/src/build/env.py

Lines 346 to 353 in e9adbf0

if 'venv' in scheme_names:
# Python distributors with custom default installation scheme can set a
# scheme that can't be used to expand the paths in a venv.
# This can happen if build itself is not installed in a venv.
# The distributors are encouraged to set a "venv" scheme to be used for this.
# See https://bugs.python.org/issue45413
# and https://github.com/pypa/virtualenv/issues/2208
paths = sysconfig.get_paths(scheme='venv', vars=config_vars)
seems to trigger a path that would fail this test.

Copy link
Contributor Author

@henryiii henryiii Mar 13, 2025

Choose a reason for hiding this comment

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

(get_paths.assert_called_once_with(scheme='posix_prefix', vars=mocker.ANY, specifically)

Copy link
Member

@layday layday Apr 22, 2025

Choose a reason for hiding this comment

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

Sorry, I forgot to reply here. This test isn't covered, so I assume it's always skipped.

@gaborbernat gaborbernat merged commit f88af00 into pypa:main Mar 25, 2025
64 checks passed
@henryiii henryiii deleted the henryiii/ci/ub branch March 25, 2025 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@layday layday layday approved these changes

@gaborbernat gaborbernat gaborbernat approved these changes

@FFY00 FFY00 Awaiting requested review from FFY00 FFY00 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@henryiii @FFY00 @gaborbernat @layday