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: Migrate metamaskbot PR comment #29373

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Dec 20, 2024

Description

Migrate the metamaskbot PR comment from CircleCI to GitHub Actions. CircleCI is still used for artifact storage for linked artifacts.

Open in GitHub Codespaces

Related issues

Relates to #28572

These changes were extracted from #29256

Manual testing steps

Screenshots/Recordings

N/A

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.

@Gudahtt Gudahtt force-pushed the migrate-metamaskbot-comment branch 4 times, most recently from 051cd52 to 3cbc139 Compare December 20, 2024 05:40
@metamaskbot
Copy link
Collaborator

Builds ready [6a4182e]
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [dc9fd81]
Page Load Metrics (1422 ± 42 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1327161014318340
domContentLoaded1292157514027838
load1301161014228742
domInteractive206232147
backgroundConnect76122178
firstReactRender1596372813
getState491202512
initialActions01000
loadScripts919120410397737
setupStore69013209
uiStartup146522171713222107
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -113.98 KiB (-2.03%)
  • ui: 197 Bytes (0.00%)
  • common: -272.89 KiB (-3.27%)

@metamaskbot
Copy link
Collaborator

Builds ready [fb737b2]
Page Load Metrics (1766 ± 68 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint24620551605464223
domContentLoaded14631975173314469
load15162040176614268
domInteractive236637126
backgroundConnect879342210
firstReactRender1686362311
getState67021209
initialActions01000
loadScripts10971482130110551
setupStore75214105
uiStartup16912301200816579
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -113.98 KiB (-2.03%)
  • ui: 1.18 KiB (0.01%)
  • common: -273.38 KiB (-3.28%)

@metamaskbot
Copy link
Collaborator

Builds ready [44f95d4]
Page Load Metrics (1610 ± 85 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint13902238161517584
domContentLoaded13852207158817282
load13902241161017785
domInteractive23127372512
backgroundConnect86222168
firstReactRender1696342612
getState46119199
initialActions01000
loadScripts10091687119414268
setupStore65414168
uiStartup16172476188820398
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -114.89 KiB (-2.04%)
  • ui: -9.25 KiB (-0.12%)
  • common: -317.6 KiB (-3.79%)

@Gudahtt Gudahtt force-pushed the migrate-metamaskbot-comment branch from 54e05e1 to 0bce5e0 Compare December 20, 2024 20:16
@metamaskbot
Copy link
Collaborator

Builds ready [0bce5e0]
Page Load Metrics (1771 ± 75 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint41320671702335161
domContentLoaded15212038174716479
load15692046177115675
domInteractive268843178
backgroundConnect96725189
firstReactRender1677442411
getState55515168
initialActions01000
loadScripts10911477128712058
setupStore66115168
uiStartup17942317201716177
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@Gudahtt Gudahtt force-pushed the migrate-metamaskbot-comment branch from 0bce5e0 to e10477e Compare December 20, 2024 20:47
@@ -9,7 +9,7 @@ async function getHighlights({ artifactBase }) {
// here we assume the PR base branch ("target") is `main` in lieu of doing
// a query against the github api which requires an access token
// see https://discuss.circleci.com/t/how-to-retrieve-a-pull-requests-base-branch-name-github/36911
const changedFiles = await getChangedFiles({ target: 'main' });
const changedFiles = await getChangedFiles({ target: 'origin/main' });
Copy link
Member Author

Choose a reason for hiding this comment

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

This is now running on GitHub Actions rather than CircleCI, which doesn't checkout the main branch locally. origin/main works across both CI systems.

BRANCH: ${{ github.head_ref }}
HEAD_COMMIT_HASH: ${{ github.event.pull_request.head.sha }}
run: |
pipeline_id=$(curl --silent "https://circleci.com/api/v2/project/gh/$OWNER/$REPOSITORY/pipeline?branch=$BRANCH" | jq -r ".items | map(select(.vcs.revision == \"${HEAD_COMMIT_HASH}\" )) | first | .id")
Copy link
Member Author

Choose a reason for hiding this comment

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

Same strategy as used elsewhere for getting the CircleCI workflow corresponding to the current branch+commit:

pipeline_id=$(curl --silent "https://circleci.com/api/v2/project/gh/$OWNER/$REPOSITORY/pipeline?branch=$BRANCH" | jq -r ".items | map(select(.vcs.revision == \"${HEAD_COMMIT_HASH}\" )) | first | .id")


echo "Getting artifacts from pipeline '${pipeline_id}', workflow '${workflow_id}', build number '${build_num}', job ID '${job_id}'"

- name: Get CircleCI job artifacts
Copy link
Member Author

Choose a reason for hiding this comment

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

Most of the artifacts referenced in the metamaskbot comment are referenced by CircleCI artifact link. We didn't need to download those.

These three are pulled from the filesystem by the script though, so we needed to download these.

PR_COMMENT_TOKEN: ${{ secrets.PR_COMMENT_TOKEN }}
PR_NUMBER: ${{ github.event.pull_request.number }}
HEAD_COMMIT_HASH: ${{ github.event.pull_request.head.sha }}
MERGE_BASE_COMMIT_HASH: ${{ steps.get-merge-base.outputs.MERGE_BASE }}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is equivalent to the PARENT_COMMIT used previously, they both use git merge-base to find the base commit between the current branch and the target branch.

const sourceMapRoot = '/build-artifacts/source-map-explorer/';
const bundleFiles = await glob(`.${sourceMapRoot}*${fileType}`);
Copy link
Member Author

@Gudahtt Gudahtt Dec 20, 2024

Choose a reason for hiding this comment

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

The strategy used to collect these links was changed. Previously we used glob to look for these files on the filesystem. It would have been tedious to migrate each file one-by-one between CI systems, so instead we're hard-coding the 5 bundle types and testing each bundle index until we get a 404.

This ensures the links are constructed correctly even as the number of bundles fluctuates (which is common), and it doesn't require transferring the files between CI systems.

@metamaskbot
Copy link
Collaborator

Builds ready [2727f61]
Page Load Metrics (1783 ± 125 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint155028221789262126
domContentLoaded152228111754265127
load155028221783260125
domInteractive20128412512
backgroundConnect880282311
firstReactRender1581322311
getState458222210
initialActions01000
loadScripts111522931327243117
setupStore771182010
uiStartup175030912012273131
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@Gudahtt Gudahtt marked this pull request as ready for review December 20, 2024 21:29
@Gudahtt Gudahtt requested a review from a team as a code owner December 20, 2024 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants