-
Notifications
You must be signed in to change notification settings - Fork 5
Enable pytest-asyncio tests in CI #92
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
Put everything in an `async def test_cog_s3` function, with assertions checking different metadata attributes.
| - name: uv sync | ||
| working-directory: python | ||
| run: uv sync --no-install-package geoindex-rs | ||
|
|
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.
it looks like the line above this is incorrect
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.
You mean uv sync --no-install-package geoindex-rs should be just uv sync?
Edit: Just removed the entire uv sync ... line at commit 638d602, and it seems to run fine.
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.
no, it needs to be uv sync --no-install-package async-tiff. And then the lines after that still need the --no-project.
You see this line: https://github.com/developmentseed/async-tiff/actions/runs/14786441598/job/41515604193#step:7:19
When you run uv sync it builds the rust code in release mode as part of the current project. For fastest CI we want to build and test in debug mode, not release mode. That means we want to tell uv not to automatically build async-tiff, because we'll manually build it after with maturin develop
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.
Ok, I think you're right. Checking the timings:
- Commit 12b230f, job https://github.com/developmentseed/async-tiff/actions/runs/14783290369/job/41506715402:
uv sync= 2m0suv run maturin develop= 46s- Total: 2m46s
- Commit 638d602, job https://github.com/developmentseed/async-tiff/actions/runs/14786316208/job/41515269407:
uv run maturin develop= 2m54s
So let's do uv sync --no-install-package async-tiff && uv run --no-project maturin develop.
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.
Final timings at commit aa21368, job https://github.com/developmentseed/async-tiff/actions/runs/14825301405/job/41617636468:
uv sync --no-install-package async-tiff= 3suv run --no-project maturin develop= 46s- Total: 49s 🚀
|
|
||
| - name: maturin venv Build | ||
| working-directory: python | ||
| run: uv run --no-project maturin develop |
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 --no-project was here because I thought whenever we ran uv sync or uv run, uv would automatically build the current project, which would compile the rust code and take a while.
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.
Hmm, I though if the working directory is set to python/ already, then it should only compile what's inside the python/ folder? Don't we need to compile the rust code anyway before compiling the pyo3 code? A little confused here.
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.
It's not the cargo project that's the issue. It's the uv project. We don't want uv automatically building the rust parts of async-tiff in release mode.
Bumps [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) from 3 to 6. - [Release notes](https://github.com/astral-sh/setup-uv/releases) - [Commits](astral-sh/setup-uv@v3...v6)
Bumps [pytest-asyncio](https://github.com/pytest-dev/pytest-asyncio) from 0.24.0 to 0.26.0. - [Release notes](https://github.com/pytest-dev/pytest-asyncio/releases) - [Commits](pytest-dev/pytest-asyncio@v0.24.0...v0.26.0) Also set asyncio_default_fixture_loop_scope="function" following pytest-dev/pytest-asyncio#924 (comment)
This reverts commit 638d602.
Partially undo bfb3a43
| uses: astral-sh/setup-uv@v3 | ||
| uses: astral-sh/setup-uv@v6 | ||
| with: | ||
| enable-cache: true |
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 assume enable-cache is true by default in the latest setup-uv?
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.
Yes, default was changed to enable-cache: true since setup-uv v5, see https://github.com/astral-sh/setup-uv/releases/tag/v5.0.0
kylebarron
left a comment
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.
Thanks!
What I am changing
How I did it
asyncio_mode="auto"in the pytest config following https://pytest-asyncio.readthedocs.io/en/latest/reference/configuration.html#asyncio-modeuv run pytestline in.github/workflows/test-python.ymlpython/tests/test_cog.pyinto a properasync def test_cog_s3function.How you can test it
cd python/ && uv run pytest --verboselocally, or check the CI logs.Related Issues