Skip to content

Implement Node.js caching, and fix Python caching, in the GitHub Actions workflows#20952

Open
timvandermeij wants to merge 2 commits intomozilla:masterfrom
timvandermeij:github-actions-caching
Open

Implement Node.js caching, and fix Python caching, in the GitHub Actions workflows#20952
timvandermeij wants to merge 2 commits intomozilla:masterfrom
timvandermeij:github-actions-caching

Conversation

@timvandermeij
Copy link
Contributor

The commit messages contain more details about the individual changes.

The `setup-node` action contains built-in support for caching [1], so
this commit makes sure we use it for all Node.js-based workflows to
reduce workflow execution time.

Note that, contrary what one might expect [2], the `node_modules`
directory is deliberately not cached because it can conflict with
differing Node.js versions and because it's not useful in combination
with `npm ci` usage which wipes the `node_modules` folder
unconditionally. Therefore, the action instead caches the global `npm`
cache directory instead which does not suffer from these problems and
still provides a speed-up at installation time.

[1] https://github.com/actions/setup-node?tab=readme-ov-file#caching-global-packages-data
[2] actions/setup-node#416
[3] actions/cache#67
For the Python-based workflows we were already using `pip` caching [1],
but sadly this isn't fully functional at the moment because the caching
functionality uses `requirements.txt` to determine when to create or
invalidate the cache. However, we have two different `pip` install
commands but only a `requirements.txt` for one of them (the Fluent
linter), which means that the other job (the font tests) will not
populate the cache with its dependencies.

This can be seen by opening any font tests or Fluent linting build and
noticing that they report the exact same cache key even though their
dependencies are different. In the installation step the dependencies
are reported as "Downloading [package].whl" instead of the expected
"Using cached [package].whl".

This commit fixes the issue by explicitly defining a `requirements.txt`
file for both jobs and pointing the caching functionality to the
specific file paths to make sure that unique caches with the correct
package data are used. While we're here we also align the syntax and
step titles in the files for consistency.

[1] https://github.com/actions/setup-python?tab=readme-ov-file#caching-packages-dependencies
@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.57%. Comparing base (56fe5fb) to head (8d4151c).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #20952      +/-   ##
==========================================
- Coverage   62.58%   62.57%   -0.01%     
==========================================
  Files         174      174              
  Lines      121947   121951       +4     
==========================================
- Hits        76318    76311       -7     
- Misses      45629    45640      +11     
Flag Coverage Δ
fonttest 7.66% <ø> (?)
unittestcli 62.54% <ø> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@calixteman calixteman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you.

Copy link

@themavik themavik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding cache: npm on setup-node and wiring pip cache-dependency-path to the renamed requirements files should cut cold CI time. nit: font_tests_requirements.txt floats fonttools on 4.* — if font tests ever break on a minor bump, consider pinning tighter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants