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: a new prep-deps workflow with caching (#29979) #30863

Draft
wants to merge 1 commit into
base: Version-v12.13.1
Choose a base branch
from

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Mar 7, 2025

  • prep-deps

  • There's a new workflow that parallels the CircleCI workflow. prep-deps runs first and caches the result, then prep-build-test-webpack makes an artifact, then the benchmarks run.

  • Several of the existing workflows were changed to now depend on prep-deps

  • Many of the file changes are just using the new checkout-and-setup@caching and deleting the independent "Checkout repository" step

  • vars.USE_CACHING No longer using this

  • This allows us to toggle the caching feature on and off in a centralized way. I left instructions as a comment in main.yml for how to toggle it. We can discuss implementing this in a different way, but passing this variable through different workflows is actually kind of a pain (that's the first way I implemented it). Several of the jobs are 30-60 seconds faster when this is turned on.

  • test-short-suite repository-health-checks

  • I think this could be slightly controversial, but I think it's for the better.

  • I noticed that there were 7 individual really short tests, that each ran on their own VM. Each VM instance took about 1m30s to execute. I combined these together into a single VM that runs all the short tests in sequence, and takes about 1m20s combined. The if: always() statements make all independent tests run, even if one fails.

  • The 7 jobs were: test-lint-shellcheck, test-lint-changelog, test-lint-lockfile, test-deps-audit, test-yarn-dedupe, test-deps-depcheck, validate-lavamoat-allow-scripts

  • If this were running on VMs that cost money, this would be a no-brainer. As is on GitHub runners, it doesn't really cost us any time or money. All we can really say is 7 separate jobs burn fossil fuels for a slight bit of convenience in terms of seeing individual test failures in the GitHub PR page.

  • Depends on feat: GitHub-hosted runners for benchmarks #29955 MERGED

  • Merge this caching branch of github-tools: ci: new checkout-and-setup to support prep-deps and caching github-tools#42 MERGED

  • Change checkout-and-setup@caching and checkout-and-setup-secure@caching to new @hash

Open in GitHub Codespaces

Split off from: #29955


Description

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

- **prep-deps**
- There's a new workflow that parallels the CircleCI workflow.
`prep-deps` runs first and caches the result, then
`prep-build-test-webpack` makes an artifact, then the benchmarks run.
- Several of the existing workflows were changed to now depend on
`prep-deps`
- Many of the file changes are just using the new
`checkout-and-setup@caching` and deleting the independent "Checkout
repository" step
- ~~**vars.USE_CACHING**~~ No longer using this
- ~~This allows us to toggle the caching feature on and off in a
centralized way. I left instructions as a comment in `main.yml` for how
to toggle it. We can discuss implementing this in a different way, but
passing this variable through different workflows is actually kind of a
pain (that's the first way I implemented it). Several of the jobs are
30-60 seconds faster when this is turned on.~~
- **~~test-short-suite~~ repository-health-checks**
- I think this could be slightly controversial, but I think it's for the
better.
- I noticed that there were 7 individual really short tests, that each
ran on their own VM. Each VM instance took about 1m30s to execute. I
combined these together into a single VM that runs all the short tests
in sequence, and takes about 1m20s combined. The `if: always()`
statements make all independent tests run, even if one fails.
- The 7 jobs were: `test-lint-shellcheck, test-lint-changelog,
test-lint-lockfile, test-deps-audit, test-yarn-dedupe,
test-deps-depcheck, validate-lavamoat-allow-scripts`
- If this were running on VMs that cost money, this would be a
no-brainer. As is on GitHub runners, it doesn't really cost us any time
or money. All we can really say is 7 separate jobs burn fossil fuels for
a slight bit of convenience in terms of seeing individual test failures
in the GitHub PR page.

- ~~Depends on #29955~~ **MERGED**
- ~~Merge this `caching` branch of `github-tools`:
MetaMask/github-tools#42~~ **MERGED**
- ~~Change `checkout-and-setup@caching` and
`checkout-and-setup-secure@caching` to new `@hash`~~

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29979?quickstart=1)

Split off from: #29955

<!--
-->

---------

Co-authored-by: Hassan Malik <[email protected]>
Copy link
Contributor

github-actions bot commented Mar 7, 2025

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the team-extension-platform Extension Platform team label Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-extension-platform Extension Platform team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants