-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Changes from all commits
8778a7d
0bd48c6
118eafb
0bbc3c5
b535197
ba153d5
1374f33
4836725
1906d2f
acf625a
df4b0c5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,72 @@ | ||||
name: Publish prerelease | ||||
|
||||
on: | ||||
workflow_call: | ||||
secrets: | ||||
PR_COMMENT_TOKEN: | ||||
required: true | ||||
|
||||
jobs: | ||||
publish-prerelease: | ||||
name: Publish prerelease | ||||
runs-on: ubuntu-latest | ||||
steps: | ||||
- name: Checkout repository | ||||
uses: actions/checkout@v4 | ||||
with: | ||||
fetch-depth: 0 # This is needed to get merge base to calculate bundle size diff | ||||
|
||||
- name: Setup environment | ||||
uses: metamask/github-tools/.github/actions/setup-environment@main | ||||
|
||||
- name: Get merge base commit hash | ||||
id: get-merge-base | ||||
env: | ||||
BASE_REF: ${{ github.event.pull_request.base.ref }} | ||||
run: | | ||||
merge_base="$(git merge-base "origin/${BASE_REF}" HEAD)" | ||||
echo "Merge base is '${merge_base}'" | ||||
echo "MERGE_BASE=${merge_base}" >> "$GITHUB_OUTPUT" | ||||
|
||||
- name: Get CircleCI job details | ||||
id: get-circleci-job-details | ||||
env: | ||||
REPOSITORY: ${{ github.repository }} | ||||
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") | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could probably refactor this later into something reusable, to avoid this duplication. But it is a temporary workaround anyway - since we intend to migrate away from CircleCI. |
||||
workflow_id=$(curl --silent "https://circleci.com/api/v2/pipeline/$pipeline_id/workflow" | jq -r ".items[0].id") | ||||
job_details=$(curl --silent "https://circleci.com/api/v2/workflow/$workflow_id/job" | jq -r '.items[] | select(.name == "job-publish-prerelease")') | ||||
build_num=$(echo "$job_details" | jq -r '.job_number') | ||||
|
||||
echo 'CIRCLE_BUILD_NUM='"$build_num" >> "$GITHUB_OUTPUT" | ||||
job_id=$(echo "$job_details" | jq -r '.id') | ||||
echo 'CIRCLE_WORKFLOW_JOB_ID='"$job_id" >> "$GITHUB_OUTPUT" | ||||
|
||||
echo "Getting artifacts from pipeline '${pipeline_id}', workflow '${workflow_id}', build number '${build_num}', job ID '${job_id}'" | ||||
|
||||
- name: Get CircleCI job artifacts | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most of the artifacts referenced in the These three are pulled from the filesystem by the script though, so we needed to download these. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically we could also fetch it in the .js script and keep it in memory instead of having it on the filesystem, but in my original implementation I did not want to make any "breaking changes" to the script. |
||||
env: | ||||
CIRCLE_WORKFLOW_JOB_ID: ${{ steps.get-circleci-job-details.outputs.CIRCLE_WORKFLOW_JOB_ID }} | ||||
run: | | ||||
mkdir -p "test-artifacts/chrome/benchmark" | ||||
curl --silent --location "https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/0/test-artifacts/chrome/benchmark/pageload.json" > "test-artifacts/chrome/benchmark/pageload.json" | ||||
|
||||
bundle_size=$(curl --silent --location "https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/0/test-artifacts/chrome/bundle_size.json") | ||||
mkdir -p "test-artifacts/chrome" | ||||
echo "${bundle_size}" > "test-artifacts/chrome/bundle_size.json" | ||||
|
||||
stories=$(curl --silent --location "https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/0/storybook/stories.json") | ||||
mkdir "storybook-build" | ||||
echo "${stories}" > "storybook-build/stories.json" | ||||
|
||||
- name: Publish prerelease | ||||
env: | ||||
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 }} | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is equivalent to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a much better name, |
||||
CIRCLE_BUILD_NUM: ${{ steps.get-circleci-job-details.outputs.CIRCLE_BUILD_NUM }} | ||||
CIRCLE_WORKFLOW_JOB_ID: ${{ steps.get-circleci-job-details.outputs.CIRCLE_WORKFLOW_JOB_ID }} | ||||
run: ./development/metamaskbot-build-announce.js |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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' }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
console.log(`detected changed files vs main:`); | ||
for (const filename of changedFiles) { | ||
console.log(` ${filename}`); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,6 @@ const path = require('path'); | |
// Fetch is part of node js in future versions, thus triggering no-shadow | ||
// eslint-disable-next-line no-shadow | ||
const fetch = require('node-fetch'); | ||
const glob = require('fast-glob'); | ||
const VERSION = require('../package.json').version; | ||
const { getHighlights } = require('./highlights'); | ||
|
||
|
@@ -38,29 +37,41 @@ function getPercentageChange(from, to) { | |
return parseFloat(((to - from) / Math.abs(from)) * 100).toFixed(2); | ||
} | ||
|
||
/** | ||
* Check whether an artifact exists, | ||
* | ||
* @param {string} url - The URL of the artifact to check. | ||
* @returns True if the artifact exists, false if it doesn't | ||
*/ | ||
async function artifactExists(url) { | ||
// Using a regular GET request here rather than HEAD because for some reason CircleCI always | ||
// returns 404 for HEAD requests. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can feel the frustration you must have felt when you discovered this fact :DDD |
||
const response = await fetch(url); | ||
return response.ok; | ||
} | ||
|
||
async function start() { | ||
const { | ||
GITHUB_COMMENT_TOKEN, | ||
CIRCLE_PULL_REQUEST, | ||
CIRCLE_SHA1, | ||
PR_COMMENT_TOKEN, | ||
PR_NUMBER, | ||
HEAD_COMMIT_HASH, | ||
MERGE_BASE_COMMIT_HASH, | ||
CIRCLE_BUILD_NUM, | ||
CIRCLE_WORKFLOW_JOB_ID, | ||
PARENT_COMMIT, | ||
} = process.env; | ||
|
||
console.log('CIRCLE_PULL_REQUEST', CIRCLE_PULL_REQUEST); | ||
console.log('CIRCLE_SHA1', CIRCLE_SHA1); | ||
console.log('PR_NUMBER', PR_NUMBER); | ||
console.log('HEAD_COMMIT_HASH', HEAD_COMMIT_HASH); | ||
console.log('MERGE_BASE_COMMIT_HASH', MERGE_BASE_COMMIT_HASH); | ||
console.log('CIRCLE_BUILD_NUM', CIRCLE_BUILD_NUM); | ||
console.log('CIRCLE_WORKFLOW_JOB_ID', CIRCLE_WORKFLOW_JOB_ID); | ||
console.log('PARENT_COMMIT', PARENT_COMMIT); | ||
|
||
if (!CIRCLE_PULL_REQUEST) { | ||
console.warn(`No pull request detected for commit "${CIRCLE_SHA1}"`); | ||
if (!PR_NUMBER) { | ||
console.warn(`No pull request detected for commit "${HEAD_COMMIT_HASH}"`); | ||
return; | ||
} | ||
|
||
const CIRCLE_PR_NUMBER = CIRCLE_PULL_REQUEST.split('/').pop(); | ||
const SHORT_SHA1 = CIRCLE_SHA1.slice(0, 7); | ||
const SHORT_SHA1 = HEAD_COMMIT_HASH.slice(0, 7); | ||
const BUILD_LINK_BASE = `https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/0`; | ||
// build the github comment content | ||
|
||
|
@@ -96,30 +107,30 @@ async function start() { | |
|
||
// links to bundle browser builds | ||
const bundles = {}; | ||
const fileType = '.html'; | ||
const sourceMapRoot = '/build-artifacts/source-map-explorer/'; | ||
const bundleFiles = await glob(`.${sourceMapRoot}*${fileType}`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The strategy used to collect these links was changed. Previously we used 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how many requests are we making here? It seems like a lot There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah it's a lot. A bit regrettable. But at least we can replace this soon, and in the overall comment building this is very fast in comparison to the other steps. |
||
|
||
bundleFiles.forEach((bundleFile) => { | ||
const fileName = bundleFile.split(sourceMapRoot)[1]; | ||
const bundleName = fileName.split(fileType)[0]; | ||
const url = `${BUILD_LINK_BASE}${sourceMapRoot}${fileName}`; | ||
let fileRoot = bundleName; | ||
let fileIndex = bundleName.match(/-[0-9]{1,}$/u)?.index; | ||
|
||
if (fileIndex) { | ||
fileRoot = bundleName.slice(0, fileIndex); | ||
fileIndex = bundleName.slice(fileIndex + 1, bundleName.length); | ||
} | ||
|
||
const link = `<a href="${url}">${fileIndex || fileRoot}</a>`; | ||
const fileRoots = [ | ||
'background', | ||
'common', | ||
'ui', | ||
'content-script', | ||
'offscreen', | ||
]; | ||
|
||
if (fileRoot in bundles) { | ||
for (const fileRoot of fileRoots) { | ||
bundles[fileRoot] = []; | ||
let fileIndex = 0; | ||
let url = `${BUILD_LINK_BASE}${sourceMapRoot}${fileRoot}-${fileIndex}.html`; | ||
console.log(`Verifying ${url}`); | ||
while (await artifactExists(url)) { | ||
const link = `<a href="${url}">${fileIndex}</a>`; | ||
bundles[fileRoot].push(link); | ||
} else { | ||
bundles[fileRoot] = [link]; | ||
|
||
fileIndex += 1; | ||
url = `${BUILD_LINK_BASE}${sourceMapRoot}${fileRoot}-${fileIndex}.html`; | ||
console.log(`Verifying ${url}`); | ||
} | ||
}); | ||
console.log(`Not found: ${url}`); | ||
} | ||
|
||
const bundleMarkup = `<ul>${Object.keys(bundles) | ||
.map((key) => `<li>${key}: ${bundles[key].join(', ')}</li>`) | ||
|
@@ -295,7 +306,7 @@ async function start() { | |
}; | ||
|
||
const devSizes = Object.keys(prSizes).reduce((sizes, part) => { | ||
sizes[part] = devBundleSizeStats[PARENT_COMMIT][part] || 0; | ||
sizes[part] = devBundleSizeStats[MERGE_BASE_COMMIT_HASH][part] || 0; | ||
return sizes; | ||
}, {}); | ||
|
||
|
@@ -346,7 +357,7 @@ async function start() { | |
} | ||
|
||
const JSON_PAYLOAD = JSON.stringify({ body: commentBody }); | ||
const POST_COMMENT_URI = `https://api.github.com/repos/metamask/metamask-extension/issues/${CIRCLE_PR_NUMBER}/comments`; | ||
const POST_COMMENT_URI = `https://api.github.com/repos/metamask/metamask-extension/issues/${PR_NUMBER}/comments`; | ||
console.log(`Announcement:\n${commentBody}`); | ||
console.log(`Posting to: ${POST_COMMENT_URI}`); | ||
|
||
|
@@ -355,7 +366,7 @@ async function start() { | |
body: JSON_PAYLOAD, | ||
headers: { | ||
'User-Agent': 'metamaskbot', | ||
Authorization: `token ${GITHUB_COMMENT_TOKEN}`, | ||
Authorization: `token ${PR_COMMENT_TOKEN}`, | ||
}, | ||
}); | ||
if (!response.ok) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentionally not required for
all-jobs-completed
because this is informational only, not a quality gate