-
-
Notifications
You must be signed in to change notification settings - Fork 128
Do less cding around in the third-party workflow
#637
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| - name: Run litestar tests | ||
| run: uv run --with=../typing-extensions-latest -- python -m pytest tests/unit/test_typing.py tests/unit/test_dto | ||
| - name: Run litestar tests with typing_extensions main branch | ||
| working-directory: litestar | ||
| run: | | ||
| # litestar's python-requires means uv won't let us add typing-extensions-latest | ||
| # as a requirement unless we do this | ||
| sed -i 's/^requires-python = ">=3.8/requires-python = ">=3.9/' pyproject.toml | ||
| uv add --editable ../typing-extensions-latest | ||
| uv sync | ||
| printf "\n\nINSTALLED DEPENDENCIES ARE:\n\n" | ||
| uv pip list | ||
| printf "\n\n" | ||
| uv run python -m pytest tests/unit/test_typing.py tests/unit/test_dto |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the old invocation here meant that the typing_extensions main branch would be installed into the ephemeral environment created by uv run --with, but the project environment (which the ephemeral environment would delegate to) would have a typing_extensions installation from PyPI installed into it.
Everything was probably working okay here? But it seems fragile, and it's hard to verify that we were actually using the main branch of typing_extensions during the pytest run. The new setup is more complicated, obviously, but it's much easier to verify that we are actually using the main branch of typing_extensions during the pytest run
| uv pip list | ||
| printf "\n\n" | ||
| uv run pytest tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we know uv run won't install a different version of typing-extensions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The uv add command a few lines above modifies the pyproject.toml file to specify that the project depends on an editable install of our checkout of the typing_extensions main branch. So if uv did an implicit sync as part of the uv run command (which you're right, it often does), all it would do as part of that sync is check that the editable install of typing_extensions in the virtual environment points to the directory specified in the pyproject.toml file
The heavy use of
cdin this workflow makes it kinda hard to figure out which directory you're in at any one time. This PR tries to reduce the number of places we switch the working directory; to me it makes the workflow easier to reason about and understand