Skip to content

e2e: Reduce CircleCI credit usage#22843

Merged
HowardBraham merged 2 commits intodevelopfrom
reduce-circleci-spend
Feb 9, 2024
Merged

e2e: Reduce CircleCI credit usage#22843
HowardBraham merged 2 commits intodevelopfrom
reduce-circleci-spend

Conversation

@HowardBraham
Copy link
Copy Markdown
Contributor

@HowardBraham HowardBraham commented Feb 7, 2024

Description

TL;DR: Reduces credit usage per successful workflow from ~12k to ~9k, see the screenshots and click the links to see for yourself.

Details

  • We were not using the resource class small before, but 29 of our jobs run just fine with 1 CPU and 2 GB RAM, most not even taking any extra time
  • I moved all resource class large back to medium+. This might cause Out of Memory errors to come back, but I've run this a few times and haven't seen any.
  • As @FrederikBolding suggested, we probably don't need to run the Desktop jobs every time. For now, I've cut it back to only running on the develop branch.
  • Reduced parallelism on a bunch of jobs, as we were seeing diminishing returns
  • Do not run the workflow at all on the branch l10n_crowdin_action

Related issues

Manual testing steps

Screenshots/Recordings

Before

~12k credit usage per workflow on develop, click to see Insights
image

After

~9k credit usage per workflow on this branch, click to see Insights
image

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

@HowardBraham HowardBraham self-assigned this Feb 7, 2024
@HowardBraham HowardBraham requested review from a team, brad-decker and kumavis as code owners February 7, 2024 08:18
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 7, 2024

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
Copy link
Copy Markdown
Collaborator

metamaskbot commented Feb 7, 2024

Builds ready [2743d76]
Page Load Metrics (1066 ± 60 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1022822025627
domContentLoaded9118382914
load8971401106612460
domInteractive9118382914
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 7, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c4c12ec) 68.47% compared to head (ffdc08e) 68.47%.
Report is 10 commits behind head on develop.

Additional details and impacted files 
@@           Coverage Diff            @@
##           develop   #22843   +/-   ##
========================================
  Coverage    68.47%   68.47%           
========================================
  Files         1088     1088           
  Lines        42902    42902           
  Branches     11424    11424           
========================================
  Hits         29377    29377           
  Misses       13525    13525

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

@HowardBraham HowardBraham force-pushed the reduce-circleci-spend branch from 2743d76 to b9cac82 Compare February 8, 2024 01:25
Gudahtt
Gudahtt reviewed Feb 8, 2024
View reviewed changes
Gudahtt
Gudahtt reviewed Feb 8, 2024
View reviewed changes
.circleci/config.yml
filters:
branches:
ignore: master
only: develop
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt Feb 8, 2024

Choose a reason for hiding this comment

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

cc @matthewwalsh0 , I know the desktop team had CI running in the desktop repo still. Does it rely on release branch CI runs here? I vaguely recall something about tests running for release branches but don't recall the details.

Restricting this to not run on PRs sounds like a great idea to me, but we might still have a use for this on release candidates.

Gudahtt
Gudahtt approved these changes Feb 8, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM, great work!

@Gudahtt
Copy link
Copy Markdown
Member

Gudahtt commented Feb 8, 2024

I'm surprised that any of our e2e test suites work on small instances! If we can keep these machine size reductions, this will make an enormous difference.

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Feb 8, 2024

Builds ready [ffdc08e]
Page Load Metrics (971 ± 66 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint872751884120
domContentLoaded898282512
load718123997113866
domInteractive898282512
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@HowardBraham HowardBraham added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Feb 8, 2024
@HowardBraham HowardBraham merged commit 480a671 into develop Feb 9, 2024
@HowardBraham HowardBraham deleted the reduce-circleci-spend branch February 9, 2024 20:20
@github-actions github-actions bot locked and limited conversation to collaborators Feb 9, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Feb 9, 2024
@metamaskbot metamaskbot added the release-11.12.0 Issue or pull request that will be included in release 11.12.0 label Feb 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Gudahtt Gudahtt Gudahtt approved these changes

@FrederikBolding FrederikBolding Awaiting requested review from FrederikBolding

@bowensanders bowensanders Awaiting requested review from bowensanders

@kumavis kumavis Awaiting requested review from kumavis kumavis is a code owner

@davidmurdoch davidmurdoch Awaiting requested review from davidmurdoch

@hjetpoluru hjetpoluru Awaiting requested review from hjetpoluru

+1 more reviewer

@brad-decker brad-decker brad-decker approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@HowardBraham HowardBraham

Labels

area-testSuite release-11.12.0 Issue or pull request that will be included in release 11.12.0 team-extension-platform Extension Platform team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@HowardBraham @metamaskbot @Gudahtt @brad-decker