Skip to content
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

ci: use tox in ci #2031

Open
wants to merge 39 commits into
base: master
Choose a base branch
from
Open

ci: use tox in ci #2031

wants to merge 39 commits into from

Conversation

moe-ad
Copy link
Contributor

@moe-ad moe-ad commented Jan 22, 2025

Related to #296

@moe-ad
Copy link
Contributor Author

moe-ad commented Jan 22, 2025

Moving to "Ready for review" so I can observe workflow runs.

@moe-ad moe-ad marked this pull request as ready for review January 22, 2025 13:44
@moe-ad moe-ad changed the title feat: use tox in ci ci: use tox in ci Jan 22, 2025
@moe-ad moe-ad force-pushed the feat/use-tox-in-ci branch from 452c771 to 2d7b82b Compare January 22, 2025 14:02
Copy link

codecov bot commented Jan 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.52%. Comparing base (678564f) to head (78a278b).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2031      +/-   ##
==========================================
- Coverage   88.55%   86.52%   -2.04%     
==========================================
  Files          90       90              
  Lines       10225    10225              
==========================================
- Hits         9055     8847     -208     
- Misses       1170     1378     +208     

@moe-ad moe-ad requested a review from a team as a code owner January 24, 2025 11:13
@moe-ad moe-ad force-pushed the feat/use-tox-in-ci branch 2 times, most recently from 5f45198 to f766ee6 Compare January 24, 2025 12:44
@moe-ad moe-ad force-pushed the feat/use-tox-in-ci branch from f766ee6 to 6c010c6 Compare January 24, 2025 12:56
@moe-ad moe-ad force-pushed the feat/use-tox-in-ci branch 6 times, most recently from a2f6f53 to 99072f7 Compare January 27, 2025 09:06
@moe-ad moe-ad force-pushed the feat/use-tox-in-ci branch 4 times, most recently from d3ac598 to 1e5c237 Compare January 30, 2025 11:22
@moe-ad moe-ad force-pushed the feat/use-tox-in-ci branch from 1e5c237 to 0ea6e4a Compare January 30, 2025 11:32
@moe-ad
Copy link
Contributor Author

moe-ad commented Jan 30, 2025

Tests run fine now but overall takes twice as long as current CI workflow.
Maybe we can test tox-uv out here @jorgepiloto? I can see much of the delay is due to the time it takes tox to spin up a new environment for each test target.

But at least we are now benefitting from the fact that each testing environment is clearly isolated, and random PyDPF test failures should occur sparingly in this case.

@jorgepiloto
Copy link
Member

This is great news, @moe-ad. Let's give it a try to ' tox-uv' plugin, in that case. Just to make sure the bottle neck is now Tox installing all the necessary dependencies for every environment.

@moe-ad
Copy link
Contributor Author

moe-ad commented Jan 31, 2025

pytest-xdist seems problematic in retro versions. May have to remove it for those.
More than 15 mins being saved by using uv from my observation, but still not low enough.
One other thing to try is activating caching.
Tests need to run in 30 mins or less for this to be a solid replacement for current CI workflow.

@moe-ad moe-ad force-pushed the feat/use-tox-in-ci branch from a7c783f to 78a278b Compare January 31, 2025 16:04
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.

2 participants