Skip to content

Commit cafa47a

Browse files
chore: fix edge case for uploading git diff artifacts needed for POM github action and skip validation for RC (#30295)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Before, we skipped performing the `git-diff` operation in cases where the PR was not targetting the default branch or when there was the `skip-e2e-quality-gate` label. However, now we still need to perform the git-diff, as the artifacts are consumed not only by the e2e quality gate, but also by the Page Object Model validation github action, and it could be consumed by more jobs in the future. So, no matter if we want to skip the e2e quality gate, we still perform the git diff. This PR adds the base PR and the labels into the PR info file artifact, so we can then read it to check if we want to skip the e2e quality gate. And it performs the git diff (uploading the diff artifacts and PR info) always, to be accessible from other jobs. It also skips the POM validation on PRs which don't target `main`. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30295?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** - [check ci artifacts](https://circleci-tasks-prod.s3.us-east-1.amazonaws.com/storage/artifacts/fb281712-ea41-446f-b28f-c0bea18842cc/04b7f9bd-6353-43e1-b2a8-5104e5286774/0/changed-files/pr-body.txt?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Checksum-Mode=ENABLED&X-Amz-Credential=ASIAQVFQINEOMLWNSGNO%2F20250214%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20250214T115938Z&X-Amz-Expires=60&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEAQaCXVzLWVhc3QtMSJGMEQCIEmPY58AcRhJG3dQ3CcB6ERDkKb4h2gNAg7fBwZFpSOFAiB5N6qFA6r3IrEy71Ui2oPWLoXgmE4S34%2BjEQNEIHutFSqrAggtEAMaDDA0NTQ2NjgwNjU1NiIMUb0tACexmFZ3iwsaKogCr3iQRTTjeXNKC6ulWvBSVzpS9aN5TbYa9z6bU2H4Ys7iWZjGZI20MMMG1CiKu5J81j4m%2B5rKWEBsLRQZm1tXzntGjFHNLtBL2ZVlnNOw4m%2Bm2vVY1wy4a5r%2BSzFDYLnYgoBQsLxLH7a0A2bJbY4ju0f7L9lrA4M9nTPuNZhxPm5fznfaVnn%2FZbls9tDc%2F9EEyeuqyOwSUZM2uMSYMBkyCe3uy4A8k1aUB0HF7R6GDAb4v0vSpEZWCQSndPHMOwBTF9TrARAQX1gnm56B4Z5sznG3OjbrBHzWxhKcSxgmGB3dNPkiuZTmq7fZfr7cIGVBW1qUmSgVYI7WFZAe5P8jJUFtcbKQu3T8MLzfvL0GOp4B8CjFmdpcotJQy3iXkxyFvI1pYeXegpbcBReO2IiCuE01Y%2BorBw0%2BujIBkXWQUUN1JGJpetzgkJQEvILxa077MBz%2BVLJJz9krXTCDJlznFTs2cy8oY63FNi6Nd%2B%2Ban5LG6i3L4rzHRpNJMvqMZ92cusI56eKYg314ESdFIXtQh6e29fiUS9NMES48WljZsdETT%2BTw5YB%2FJiOzxFkZJGE%3D&X-Amz-SignedHeaders=host&x-id=GetObject&X-Amz-Signature=429e0beaffdf34cf408ac746ac6353f4aef3d5505fa6c888842c5ab7fa32135a) for PR info: now pr info has the PR labels as well as the PR base - Git diff is performed no matter if I have applied the `skip-e2e-quality-gate` label ## **Screenshots/Recordings** See PR label and base added in the top of PR info artifact ![Screenshot from 2025-02-14 11-48-19](https://github.com/user-attachments/assets/f55e1e41-ae6d-4b3a-b2d7-0cbec6b8e68f) ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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. --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
1 parent 346e3b8 commit cafa47a

File tree

4 files changed

+54
-18
lines changed

4 files changed

+54
-18
lines changed

.circleci/scripts/git-diff-default-branch.ts

+8-14
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,12 @@ async function gitDiff(): Promise<string> {
105105
return diffResult;
106106
}
107107

108-
function writePrBodyToFile(prBody: string) {
108+
function writePrBodyAndInfoToFile(prInfo: PRInfo) {
109109
const prBodyPath = path.resolve(CHANGED_FILES_DIR, 'pr-body.txt');
110-
fs.writeFileSync(prBodyPath, prBody.trim());
111-
console.log(`PR body saved to ${prBodyPath}`);
110+
const labels = prInfo.labels.map(label => label.name).join(', ');
111+
const updatedPrBody = `PR labels: {${labels}}\nPR base: {${prInfo.base.ref}}\n${prInfo.body.trim()}`;
112+
fs.writeFileSync(prBodyPath, updatedPrBody);
113+
console.log(`PR body and info saved to ${prBodyPath}`);
112114
}
113115

114116
/**
@@ -135,17 +137,9 @@ async function storeGitDiffOutputAndPrBody() {
135137
if (!baseRef) {
136138
console.log('Not a PR, skipping git diff');
137139
return;
138-
} else if (baseRef !== GITHUB_DEFAULT_BRANCH) {
139-
console.log(`This is for a PR targeting '${baseRef}', skipping git diff`);
140-
writePrBodyToFile(prInfo.body);
141-
return;
142-
} else if (
143-
prInfo.labels.some((label) => label.name === 'skip-e2e-quality-gate')
144-
) {
145-
console.log('PR has the skip-e2e-quality-gate label, skipping git diff');
146-
return;
147140
}
148-
141+
// We perform git diff even if the PR base is not main or skip-e2e-quality-gate label is applied
142+
// because we rely on the git diff results for other jobs
149143
console.log('Attempting to get git diff...');
150144
const diffOutput = await gitDiff();
151145
console.log(diffOutput);
@@ -155,7 +149,7 @@ async function storeGitDiffOutputAndPrBody() {
155149
fs.writeFileSync(outputPath, diffOutput.trim());
156150
console.log(`Git diff results saved to ${outputPath}`);
157151

158-
writePrBodyToFile(prInfo.body);
152+
writePrBodyAndInfoToFile(prInfo);
159153

160154
process.exit(0);
161155
} catch (error: any) {

.github/workflows/validate-page-object-usage.yml

+2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ name: Validate E2E Page Object usage on modified files
22

33
on:
44
pull_request:
5+
branches:
6+
- main
57
types:
68
- opened
79
- reopened

test/e2e/changedFilesUtil.js

+40
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const CHANGED_FILES_PATH = path.join(
77
'changed-files',
88
'changed-files.txt',
99
);
10+
const PR_INFO_PATH = path.join(BASE_PATH, 'changed-files', 'pr-body.txt');
1011

1112
/**
1213
* Reads the list of changed files from the git diff file with status (A, M, D).
@@ -80,10 +81,49 @@ function getChangedAndNewFiles(changedFiles) {
8081
return changedFiles.map((file) => file.filePath);
8182
}
8283

84+
/**
85+
* Checks if the E2E quality gate should be skipped based on the PR info.
86+
*
87+
* @returns {boolean} True if the quality gate should be skipped, otherwise false.
88+
*/
89+
function shouldE2eQualityGateBeSkipped() {
90+
try {
91+
const data = fs.readFileSync(PR_INFO_PATH, 'utf8');
92+
const lines = data.split('\n');
93+
const labelsLine = lines.find((line) => line.startsWith('PR labels:'));
94+
const baseLine = lines.find((line) => line.startsWith('PR base:'));
95+
96+
const labels = labelsLine
97+
? labelsLine
98+
.replace(/PR labels: \{/gu, '')
99+
.replace(/\}/gu, '')
100+
.split(',')
101+
.map((label) => label.trim())
102+
: [];
103+
const base = baseLine
104+
? baseLine
105+
.replace(/PR base: \{/gu, '')
106+
.replace(/\}/gu, '')
107+
.trim()
108+
: '';
109+
console.log('PR labels', labels);
110+
console.log('PR base', base);
111+
112+
const skipGate =
113+
labels.includes('skip-e2e-quality-gate') || base !== 'main';
114+
console.log('Should we skip the e2e quality gate:', skipGate);
115+
return skipGate;
116+
} catch (error) {
117+
console.error('Error reading PR body file:', error);
118+
return false;
119+
}
120+
}
121+
83122
module.exports = {
84123
filterE2eChangedFiles,
85124
getChangedAndNewFiles,
86125
getChangedFilesOnly,
87126
getNewFilesOnly,
88127
readChangedAndNewFilesWithStatus,
128+
shouldE2eQualityGateBeSkipped,
89129
};

test/e2e/run-all.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const {
1010
filterE2eChangedFiles,
1111
getChangedAndNewFiles,
1212
readChangedAndNewFilesWithStatus,
13+
shouldE2eQualityGateBeSkipped,
1314
} = require('./changedFilesUtil');
1415

1516
// These tests should only be run on Flask for now.
@@ -75,10 +76,9 @@ function runningOnCircleCI(testPaths) {
7576
const changedOrNewTests = filterE2eChangedFiles(changedandNewFilesPaths);
7677
console.log('Changed or new test list:', changedOrNewTests);
7778

78-
const fullTestList = applyQualityGate(
79-
testPaths.join('\n'),
80-
changedOrNewTests,
81-
);
79+
const fullTestList = shouldE2eQualityGateBeSkipped()
80+
? testPaths.join('\n')
81+
: applyQualityGate(testPaths.join('\n'), changedOrNewTests);
8282

8383
console.log('Full test list:', fullTestList);
8484
fs.writeFileSync('test/test-results/fullTestList.txt', fullTestList);

0 commit comments

Comments
 (0)