CI: add pip cache to tests workflow to reduce runtime#920
Conversation
There was a problem hiding this comment.
ACK
I tested this on my fork. I used the upstream dev branch as a baseline and pr-920 branch with the changes, then ran the jobs 10 times on each to average out the numbers.
The setup looks right to me.
cache: "pip" is the built-in option on actions/setup-python@v5 and the cache key it generates already includes the Python version, so 3.10 and 3.12 get separate caches and don't collide.
cache-dependency-path lists both requirements.txt and tests/requirements.txt, so the key changes whenever either file is edited.
CI Performance Comparison - medians across 10 attempts per condition
Install dependencies step:
| Python | dev (no cache) | pr-920 cache hit | Change |
|---|---|---|---|
| 3.10 | 20.5 s | 13.0 s | -7.5 s (~37%) |
| 3.12 | 20.0 s | 17.0 s | -3.0 s (~15%) |
Total time for the whole job (not just the install step):
| Python | dev | pr-920 | Change |
|---|---|---|---|
| 3.10 | 73 s | 67 s | -6 s |
| 3.12 | 69 s | 68.5 s | -0.5 s |
Notes from the runs
- First run on the PR branch is a cache miss and pays a one-time cost to save the cache. After that, every attempt hits the cache until either the requirements file changes.
- The Set up Python step goes from 0 s on
devto about 2 to 2.5 s on thepr-920branch because it now has to download and extract the cache tarball. Although this lengthens the setup phase, it makes the installation step slightly faster. As a result, the overall job time for version 3.12 shows only a minimal change. - The steps that aren't cached (pytest, screenshot generation,
apt install libzbar0, andpip install .for the local package) still dominate the total runtime, so those bound the ceiling on what a pip cache can save.
|
Thanks for that data @PROWLERx15. I'm mostly a concept NACK. Doesn't hurt, but the improvement is so minimal that in its current form the PR incurs more mental load for review than it's worth. This CI workflow just isn't time critical and we're well under the free tier's limits of CI time. That being said, I only recently learned about GHCR images. Pre-baking ALL of the configuration / dependencies would cut the total execution time in about half. And our dependencies change infrequently enough that a base image will remain stable for quite a while. But even with that approach we still have the same problem: PR review cognitive load vs saving 30s on a CI workflow that isn't time-sensitive to begin with. But at least then the time/efficiency savings maybe push me to 51% yes, 49% why bother. Maybe more like 50.15% vs 49.85%. |
|
@muhahahmad68 I will likely be opening a PR to make some other minor changes to the CI jobs. I think it'll make more sense to include your changes (whether it's in this current form or maybe the bigger GHCR version) in that PR. When the time comes, if you open a PR with your changes into my PR branch, I can merge your changes into my PR. The final PR merge would then retain your contribution history. |
|
Thanks for the detailed review @PROWLERx15 and the feedback @kdmukai. Understood on the minimal gains, the data makes that clear. I'm happy to wait for your CI PR and contribute into that branch instead. Just let me know when it's open and I'll submit the changes there. Also interested in the GHCR approach if you want an extra hand on that. |
|
@muhahahmad68 I've opened my PR: #926 For now I recommend that you stick to just adding the pip cache since the GHCR approach would entail a new CI job (building the GHCR image) and then updating the existing job to use it. Your PR into my branch should target: https://github.com/kdmukai/seedsigner/tree/2026-04-22_CI_hardening |
|
kdmukai#12 has been merged into my PR #926. This PR can now be closed. |
Description
Problem or Issue being addressed
The CI tests workflow installs pip dependencies from scratch on every run,
even when
requirements.txtandtests/requirements.txthaven't changed.This increases runtime and runner cost unnecessarily.
Closes #917
Solution
Added pip caching to the
setup-pythonstep using the built-in cachesupport in
actions/setup-python@v5. The cache key is automaticallyderived from both
requirements.txtandtests/requirements.txt, soit invalidates and rebuilds whenever dependencies change.
Additional Information
The cache will miss on the first run after this change (expected behavior),
then hit on subsequent runs where requirements haven't changed. Both Python
versions (3.10 and 3.12) maintain separate caches via the matrix strategy.
Screenshots
N/A — no UI changes.
This pull request is categorized as a:
Checklist
I ran
pytestlocallyI included screenshots of any new or modified screens
Should be part of the PR description above.
I added or updated tests
Any new or altered functionality should be covered in a unit test. Any new or updated sequences require FlowTests.
I tested this PR hands-on on the following platform(s):
I have reviewed these notes:
Thank you! Please join our Devs' Telegram group to get more involved.