Skip to content

Speed up test data caching in CI #566

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

Merged
merged 2 commits into from
May 1, 2025
Merged

Conversation

niksirbi
Copy link
Member

@niksirbi niksirbi commented Apr 30, 2025

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?
We've noticed for a long time that the Windows runner in CI takes significantly longer than other runners. I suspect it's related to the cross-OS cache not working as intended, likely due to ~ resolving to a different absolute path depending on OS, see https://github.com/actions/cache/blob/main/tips-and-workarounds.md#cross-os-cache.

Moreover, I noticed a significant slowdown in the build_sphinx_docs action run times, manifest since we started using videos in some of the examples. They get downloaded each time because we hadn't previously implemented caching for that action.

What does this PR do?

References

brainglobe/brainglobe-workflows#142

How has this PR been tested?

I manually ran CI several times in this PR, to observe run times, and it seems to work.
First Windows test runs were 23-32 minutes, and subsequent ones were 9-10 minutes (after windows-specific cache was populated). Similarly the build_sphinx_docs run times went from ~9 to ~2-3 minutes.

No jobs takes >10 minutes now!

Is this a breaking change?

No.

Does this PR require an update to the documentation?

No.

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

Copy link

codecov bot commented Apr 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (f2b539d) to head (f11fde7).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #566   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           28        28           
  Lines         1571      1571           
=========================================
  Hits          1571      1571           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@niksirbi niksirbi marked this pull request as ready for review April 30, 2025 15:34
@niksirbi niksirbi requested a review from adamltyson April 30, 2025 15:34
@adamltyson adamltyson added this pull request to the merge queue May 1, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 1, 2025
@adamltyson adamltyson added this pull request to the merge queue May 1, 2025
Merged via the queue into main with commit 9b38808 May 1, 2025
27 checks passed
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