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.

Sign up for GitHub

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: Fix metamaskbot comment test build links #29403

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+41 −54
Open

ci: Fix metamaskbot comment test build links #29403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3d2466a
ci: Fix metamaskbot comment build links
Gudahtt Dec 20, 2024
b1d9ed9
Skip labels with no builds
Gudahtt Dec 20, 2024
016f315
Fix comment build link formatting
Gudahtt Dec 20, 2024
dad8928
Remove validation
Gudahtt Dec 20, 2024
d679d4c
Fix webpack and test-flask-mv2 builds, remove test-mmi
Gudahtt Dec 20, 2024
f694c71
Remove webpack build link (no zips)
Gudahtt Dec 20, 2024
de0f2dd
Remove beta link
Gudahtt Dec 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ workflows:
requires:
- prep-deps
- prep-build-test-flask-mv2:
<<: *main_master_rc_only
Copy link
Member Author

Choose a reason for hiding this comment

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

We need to build it on all PRs in order to include the build link

requires:
- prep-deps
- prep-build-test-mmi:
Expand Down Expand Up @@ -301,10 +300,14 @@ workflows:
- prep-deps
- prep-build
- prep-build-mv2
- trigger-beta-build
- prep-build-mmi
- prep-build-flask
- prep-build-flask-mv2
- prep-build-test
Copy link
Member Author

Choose a reason for hiding this comment

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

These are all required by the prepublish job because it uses the builds persisted to the workspace by these jobs.

Some of these were indirectly guaranteed to be present in the workspace due to them being required by some other listed dependency here. But best not to rely on that.

- prep-build-test-mv2
- prep-build-test-flask
- prep-build-test-flask-mv2
- trigger-beta-build
- prep-build-storybook
- prep-build-ts-migration-dashboard
- benchmark
Expand Down Expand Up @@ -765,10 +768,10 @@ jobs:
name: Build extension for testing
command: yarn build:test:flask:mv2
- run:
name: Move test build to 'dist-test-flask' to avoid conflict with production build
name: Move test build to 'dist-test-flask-mv2' to avoid conflict with production build
Copy link
Member Author

Choose a reason for hiding this comment

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

These two changes were copy+paste mistakes in the step description

command: mv ./dist ./dist-test-flask-mv2
- run:
name: Move test zips to 'builds-test-flask' to avoid conflict with production build
name: Move test zips to 'builds-test-flask-mv2' to avoid conflict with production build
command: mv ./builds ./builds-test-flask-mv2
- persist_to_workspace:
root: .
Expand Down Expand Up @@ -1408,8 +1411,12 @@ jobs:
destination: builds-mv2
- store_artifacts:
path: builds-test
- store_artifacts:
path: builds-test-mv2
- store_artifacts:
path: builds-test-flask
- store_artifacts:
path: builds-test-flask-mv2
Copy link
Member Author

@Gudahtt Gudahtt Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This was the main issue with the broken test MV2 build links: we forgot toe store_artifacts step.

- store_artifacts:
path: test-artifacts
destination: test-artifacts
Expand Down
80 changes: 30 additions & 50 deletions development/metamaskbot-build-announce.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,50 +65,34 @@ async function start() {
// build the github comment content

// links to extension builds
const platforms = ['chrome', 'firefox'];
const buildLinks = platforms
.map((platform) => {
const url =
platform === 'firefox'
? `${BUILD_LINK_BASE}/builds-mv2/metamask-${platform}-${VERSION}.zip`
: `${BUILD_LINK_BASE}/builds/metamask-${platform}-${VERSION}.zip`;
return `<a href="${url}">${platform}</a>`;
})
.join(', ');
const betaBuildLinks = `<a href="${BUILD_LINK_BASE}/builds-beta/metamask-beta-chrome-${VERSION}.zip">chrome</a>`;
const flaskBuildLinks = platforms
.map((platform) => {
const url =
platform === 'firefox'
? `${BUILD_LINK_BASE}/builds-flask-mv2/metamask-flask-${platform}-${VERSION}-flask.0.zip`
: `${BUILD_LINK_BASE}/builds-flask/metamask-flask-${platform}-${VERSION}-flask.0.zip`;
return `<a href="${url}">${platform}</a>`;
})
.join(', ');
const mmiBuildLinks = platforms
.map((platform) => {
const url = `${BUILD_LINK_BASE}/builds-mmi/metamask-mmi-${platform}-${VERSION}-mmi.0.zip`;
return `<a href="${url}">${platform}</a>`;
})
.join(', ');
const testBuildLinks = platforms
.map((platform) => {
const url =
platform === 'firefox'
? `${BUILD_LINK_BASE}/builds-test-mv2/metamask-${platform}-${VERSION}.zip`
: `${BUILD_LINK_BASE}/builds-test/metamask-${platform}-${VERSION}.zip`;
return `<a href="${url}">${platform}</a>`;
})
.join(', ');
const testFlaskBuildLinks = platforms
.map((platform) => {
const url =
platform === 'firefox'
? `${BUILD_LINK_BASE}/builds-test-flask-mv2/metamask-flask-${platform}-${VERSION}-flask.0.zip`
: `${BUILD_LINK_BASE}/builds-test-flask/metamask-flask-${platform}-${VERSION}-flask.0.zip`;
const buildMap = {
Copy link
Member Author

Choose a reason for hiding this comment

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

Specifying the labels and builds declaratively made it easier to apply the same validation and formatting to all of them.

builds: {
chrome: `${BUILD_LINK_BASE}/builds/metamask-chrome-${VERSION}.zip`,
firefox: `${BUILD_LINK_BASE}/builds-mv2/metamask-firefox-${VERSION}.zip`,
},
'builds (flask)': {
chrome: `${BUILD_LINK_BASE}/builds-flask/metamask-flask-chrome-${VERSION}-flask.0.zip`,
firefox: `${BUILD_LINK_BASE}/builds-flask-mv2/metamask-flask-firefox-${VERSION}-flask.0.zip`,
},
'builds (MMI)': {
chrome: `${BUILD_LINK_BASE}/builds-mmi/metamask-mmi-chrome-${VERSION}-mmi.0.zip`,
},
'builds (test)': {
chrome: `${BUILD_LINK_BASE}/builds-test/metamask-chrome-${VERSION}.zip`,
firefox: `${BUILD_LINK_BASE}/builds-test-mv2/metamask-firefox-${VERSION}.zip`,
},
'builds (test-flask)': {
chrome: `${BUILD_LINK_BASE}/builds-test-flask/metamask-flask-chrome-${VERSION}-flask.0.zip`,
firefox: `${BUILD_LINK_BASE}/builds-test-flask-mv2/metamask-flask-firefox-${VERSION}-flask.0.zip`,
},
};

const buildContentRows = Object.entries(buildMap).map(([label, builds]) => {
const buildLinks = Object.entries(builds).map(([platform, url]) => {
return `<a href="${url}">${platform}</a>`;
})
.join(', ');
});
return `${label}: ${buildLinks.join(', ')}`;
});

// links to bundle browser builds
const bundles = {};
Expand Down Expand Up @@ -171,12 +155,7 @@ async function start() {
const allArtifactsUrl = `https://circleci.com/gh/MetaMask/metamask-extension/${CIRCLE_BUILD_NUM}#artifacts/containers/0`;

const contentRows = [
`builds: ${buildLinks}`,
`builds (beta): ${betaBuildLinks}`,
`builds (flask): ${flaskBuildLinks}`,
`builds (MMI): ${mmiBuildLinks}`,
`builds (test): ${testBuildLinks}`,
`builds (test-flask): ${testFlaskBuildLinks}`,
...buildContentRows,
`build viz: ${depVizLink}`,
`mv3: ${moduleInitStatsBackgroundLink}`,
`mv3: ${moduleInitStatsUILink}`,
Expand All @@ -198,8 +177,9 @@ async function start() {
const exposedContent = `Builds ready [${SHORT_SHA1}]`;
const artifactsBody = `<details><summary>${exposedContent}</summary>${hiddenContent}</details>\n\n`;

const benchmarkPlatforms = ['chrome'];
const benchmarkResults = {};
for (const platform of platforms) {
for (const platform of benchmarkPlatforms) {
const benchmarkPath = path.resolve(
__dirname,
'..',
Expand Down
Loading