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

chore: validate page object usage in new spec files on every PR #29915

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
5ea6f0f
base work for pom validation
seaona Jan 27, 2025
d11f3c4
yarn version
seaona Jan 27, 2025
0383d86
changed files as artifacts
seaona Jan 27, 2025
7c39992
wait for circle ci
seaona Jan 27, 2025
c209015
some debugging
seaona Jan 27, 2025
451fd5d
envars
seaona Jan 27, 2025
b450b65
fetch calls
seaona Jan 27, 2025
04c7dd4
rmv wait
seaona Jan 27, 2025
8a9f7c1
fix script
seaona Jan 27, 2025
ccabc5e
run
seaona Jan 27, 2025
62f8058
add env
seaona Jan 27, 2025
575b28d
error handling and wait time
seaona Jan 27, 2025
c8d27ab
fix url with envar
seaona Jan 28, 2025
8910bc8
wait
seaona Jan 28, 2025
725fab7
download buffer
seaona Jan 28, 2025
de4975b
directory checks
seaona Jan 28, 2025
da7acf5
remove tests and clean up
seaona Jan 28, 2025
9376bf7
Merge branch 'main' into page-object-enforce
seaona Jan 28, 2025
0b58c0c
on workflow call
seaona Jan 28, 2025
c501046
lint
seaona Jan 28, 2025
2adb681
reset envars again
seaona Jan 28, 2025
a65e2eb
env
seaona Jan 28, 2025
ba02294
smaller sleep
seaona Jan 28, 2025
28a43d2
review comment: update name
seaona Jan 28, 2025
d96ac91
review: address argument name and filter includes
seaona Jan 28, 2025
4702302
changedFilesPaths
seaona Jan 28, 2025
684366c
address more review comments
seaona Jan 29, 2025
3f58395
fix
seaona Jan 29, 2025
41427bd
back to js
seaona Jan 29, 2025
d49bd8b
Howard's fix .github/scripts/shared/circle-artifacts.ts
seaona Jan 30, 2025
9e15a13
Howard's suggestion getbranch
seaona Jan 30, 2025
0792a3b
add file status
seaona Feb 4, 2025
3bd0661
change workflow call for trigger
seaona Feb 4, 2025
0642693
test
seaona Feb 4, 2025
fdea90e
re-add tests as check has been successful
seaona Feb 4, 2025
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
3 changes: 3 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,9 @@ jobs:
root: .
paths:
- changed-files
- store_artifacts:
path: changed-files
destination: changed-files
Copy link
Contributor Author

Choose a reason for hiding this comment

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

before, we were just reading from the files, as the jobs that needed this were all in circle ci.
Now we need to store the results as an artifact, because we need to access the result from github actions too.


validate-locales-only:
executor: node-browsers-small
Expand Down
5 changes: 3 additions & 2 deletions .circleci/scripts/test-run-e2e-timeout-minutes.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import { fetchManifestFlagsFromPRAndGit } from '../../development/lib/get-manifest-flag';
import { filterE2eChangedFiles } from '../../test/e2e/changedFilesUtil';
import { filterE2eChangedFiles, readChangedFiles } from '../../test/e2e/changedFilesUtil';

fetchManifestFlagsFromPRAndGit().then((manifestFlags) => {
let timeout;

if (manifestFlags.circleci?.timeoutMinutes) {
timeout = manifestFlags.circleci?.timeoutMinutes;
} else {
const changedOrNewTests = filterE2eChangedFiles();
const changedFiles = readChangedFiles();
const changedOrNewTests = filterE2eChangedFiles(changedFiles);
Copy link
Contributor Author

@seaona seaona Jan 28, 2025

Choose a reason for hiding this comment

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

this is due to a change in the filterE2eChangedFiles as now it accepts the files in the func argument, so it can also be re-used in the github action job


// 20 minutes, plus 3 minutes for every changed file, up to a maximum of 30 minutes
timeout = Math.min(20 + changedOrNewTests.length * 3, 30);
Expand Down
49 changes: 13 additions & 36 deletions .devcontainer/download-builds.ts
Original file line number Diff line number Diff line change
@@ -1,43 +1,16 @@
import { execSync } from 'child_process';
import util from 'util';

import {
getJobsByWorkflowId,
getPipelineId,
getWorkflowId,
} from '../.github/scripts/shared/circle-artifacts';
const exec = util.promisify(require('node:child_process').exec);

function getGitBranch() {
const gitOutput = execSync('git status').toString();

const branchRegex = /On branch (?<branch>.*)\n/;
return gitOutput.match(branchRegex)?.groups?.branch || 'main';
}

async function getCircleJobs(branch: string) {
let response = await fetch(
`https://circleci.com/api/v2/project/gh/MetaMask/metamask-extension/pipeline?branch=${branch}`,
);

const pipelineId = (await response.json()).items[0].id;

console.log('pipelineId:', pipelineId);

response = await fetch(
`https://circleci.com/api/v2/pipeline/${pipelineId}/workflow`,
);

const workflowId = (await response.json()).items[0].id;

console.log('workflowId:', workflowId);

response = await fetch(
`https://circleci.com/api/v2/workflow/${workflowId}/job`,
);

const jobs = (await response.json()).items;

return jobs;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ℹ️ this logic is now in circle-artifacts file, as it's shared with the validate-page-object scriptl, as per @HowardBraham 's recommendation

async function getBuilds(branch: string, jobNames: string[]) {
const jobs = await getCircleJobs(branch);
const pipelineId = await getPipelineId(branch);
const workflowId = await getWorkflowId(pipelineId);
const jobs = await getJobsByWorkflowId(workflowId);
let builds = [] as any[];

for (const jobName of jobNames) {
Expand Down Expand Up @@ -137,7 +110,11 @@ function unzipBuilds(folder: 'builds' | 'builds-test', versionNumber: string) {
}

async function main(jobNames: string[]) {
const branch = getGitBranch();
const branch = process.env.CIRCLE_BRANCH;

if (!branch) {
throw new Error('CIRCLE_BRANCH environment variable is not set.');
}
HowardBraham marked this conversation as resolved.
Show resolved Hide resolved

const builds = await getBuilds(branch, jobNames);

Expand Down
181 changes: 181 additions & 0 deletions .github/scripts/shared/circle-artifacts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
import fs from 'fs';

// Set OWNER and REPOSITORY based on the environment
const OWNER = process.env.CIRCLE_PROJECT_USERNAME || process.env.OWNER;
const REPOSITORY = process.env.CIRCLE_PROJECT_REPONAME || process.env.REPOSITORY;
seaona marked this conversation as resolved.
Show resolved Hide resolved
const CIRCLE_BASE_URL = 'https://circleci.com/api/v2';

/**
* Retrieves the pipeline ID for a given branch and optional commit hash.
*
* @param {string} branch - The branch name to fetch the pipeline for.
* @param {string} [headCommitHash] - Optional commit hash to match a specific pipeline.
* @returns {Promise<string>} A promise that resolves to the pipeline ID.
* @throws Will throw an error if no pipeline is found or if the HTTP request fails.
*/
export async function getPipelineId(branch: string, headCommitHash?: string): Promise<string> {
const url = `${CIRCLE_BASE_URL}/project/gh/${OWNER}/${REPOSITORY}/pipeline?branch=${branch}`;
const response = await fetch(url);
if (!response.ok) {
throw new Error(`Failed to fetch pipeline data: ${response.statusText}`);
}

const pipelineData = await response.json();
const pipelineItem = headCommitHash
? pipelineData.items.find((item: any) => item.vcs.revision === headCommitHash)
: pipelineData.items[0];
if (!pipelineItem) {
throw new Error('Pipeline ID not found');
}
console.log('pipelineId:', pipelineItem.id);

return pipelineItem.id;
}

/**
* Retrieves the workflow ID for a given pipeline ID.
*
* @param {string} pipelineId - The ID of the pipeline to fetch the workflow for.
* @returns {Promise<string>} A promise that resolves to the workflow ID.
* @throws Will throw an error if no workflow is found or if the HTTP request fails.
*/
export async function getWorkflowId(pipelineId: string): Promise<string> {
const url = `${CIRCLE_BASE_URL}/pipeline/${pipelineId}/workflow`;
const response = await fetch(url);
if (!response.ok) {
throw new Error(`Failed to fetch workflow data: ${response.statusText}`);
}
const workflowData = await response.json();
const workflowId = workflowData.items[0]?.id;
if (!workflowId) {
throw new Error('Workflow ID not found');
}
console.log('workflowId:', workflowId);
return workflowId;
}

/**
* Retrieves a list of jobs for a given workflow ID.
*
* @param {string} workflowId - The ID of the workflow to fetch jobs for.
* @returns {Promise<any[]>} A promise that resolves to an array of jobs.
* @throws Will throw an error if no jobs are found or if the HTTP request fails.
*/
export async function getJobsByWorkflowId(workflowId: string): Promise<any[]> {
const url = `${CIRCLE_BASE_URL}/workflow/${workflowId}/job`;
const response = await fetch(url);
if (!response.ok) {
throw new Error(`Failed to fetch jobs: ${response.statusText}`);
}
const jobs = (await response.json()).items;
return jobs;
}

/**
* Retrieves job details for a given workflow ID and optional job name.
*
* @param {string} workflowId - The ID of the workflow to fetch job details for.
* @param {string} [jobName] - Optional job name to match a specific job.
* @returns {Promise<any>} A promise that resolves to the job details.
* @throws Will throw an error if no job details are found or if the HTTP request fails.
*/
export async function getJobDetails(workflowId: string, jobName?: string): Promise<any> {
const jobs = await getJobsByWorkflowId(workflowId);
const jobDetails = jobName
? jobs.find((item: any) => item.name === jobName)
: jobs[0];
if (!jobDetails) {
throw new Error('Job details not found');
}
return jobDetails;
}

/**
* Retrieves the artifact URL for a given branch, commit hash, job name, and artifact name.
* @param {string} branch - The branch name.
* @param {string} headCommitHash - The commit hash of the branch.
* @param {string} jobName - The name of the job that produced the artifact.
* @param {string} artifactName - The name of the artifact to retrieve.
* @returns {Promise<string>} A promise that resolves to the artifact URL.
* @throws Will throw an error if the artifact is not found or if any HTTP request fails.
*/
export async function getArtifactUrl(branch: string, headCommitHash: string, jobName: string, artifactName: string): Promise<string> {
const pipelineId = await getPipelineId(branch, headCommitHash);
const workflowId = await getWorkflowId(pipelineId);
const jobDetails = await getJobDetails(workflowId, jobName);

const jobNumber = jobDetails.job_number;
console.log('Job number', jobNumber);

const artifactResponse = await fetch(`${CIRCLE_BASE_URL}/project/gh/${OWNER}/${REPOSITORY}/${jobNumber}/artifacts`);
const artifactData = await artifactResponse.json();
const artifact = artifactData.items.find((item: any) => item.path.includes(artifactName));

if (!artifact) {
throw new Error(`Artifact ${artifactName} not found`);
}

const artifactUrl = artifact.url;
console.log('Artifact URL:', artifactUrl);;

return artifactUrl;
}

/**
* Downloads an artifact from a given URL and saves it to the specified output path.
* @param {string} artifactUrl - The URL of the artifact to download.
* @param {string} outputFilePath - The path where the artifact should be saved.
* @returns {Promise<void>} A promise that resolves when the download is complete.
* @throws Will throw an error if the download fails or if the file cannot be written.
*/
export async function downloadArtifact(artifactUrl: string, outputFilePath: string): Promise<void> {
try {
// Ensure the directory exists
const dir = require('path').dirname(outputFilePath);
if (!fs.existsSync(dir)) {
fs.mkdirSync(dir, { recursive: true });
}

console.log(`Downloading artifact from URL: ${artifactUrl} to ${outputFilePath}`);

// Download the artifact
const artifactDownloadResponse = await fetch(artifactUrl);
if (!artifactDownloadResponse.ok) {
throw new Error(`Failed to download artifact: ${artifactDownloadResponse.statusText}`);
}
const artifactArrayBuffer = await artifactDownloadResponse.arrayBuffer();
const artifactBuffer = Buffer.from(artifactArrayBuffer);
fs.writeFileSync(outputFilePath, artifactBuffer);

if (!fs.existsSync(outputFilePath)) {
throw new Error(`Failed to download artifact to ${outputFilePath}`);
}

console.log(`Artifact downloaded successfully to ${outputFilePath}`);
} catch (error) {
console.error(`Error during artifact download: ${error.message}`);
throw error;
}
}

/**
* Reads the content of a file.
* @param filePath - The path to the file.
* @returns The content of the file.
*/
export function readFileContent(filePath: string): string {
if (!fs.existsSync(filePath)) {
throw new Error(`File not found: ${filePath}`);
}

const content = fs.readFileSync(filePath, 'utf-8').trim();
return content;
}

/**
* Sleep function to pause execution for a specified number of seconds.
* @param seconds - The number of seconds to sleep.
*/
export function sleep(seconds: number) {
return new Promise(resolve => setTimeout(resolve, seconds * 1000));
}
107 changes: 107 additions & 0 deletions .github/scripts/validate-e2e-page-object-usage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import fs from 'fs';
import { execSync } from 'child_process';
import { filterE2eChangedFiles } from '../../test/e2e/changedFilesUtil';
import {
downloadArtifact,
getArtifactUrl,
readFileContent,
sleep,
} from './shared/circle-artifacts';

async function verifyE2ePageObjectsUsage() {
let e2eFiles: string[];

if (process.env.GITHUB_ACTIONS) {
// Running in Github Actions
const branch = process.env.BRANCH || '';
const headCommitHash = process.env.HEAD_COMMIT_HASH || '';
const artifactName = 'changed-files.txt';
const artifactPath = 'changed-files';
const jobName = 'get-changed-files-with-git-diff';

let attempts = 0;
const maxAttempts = 3;
let changedFilesContent = '';

// Small buffer to ensure the job id is accessible in circle ci
// once we have that job migrated into github actions, we can just add a dependency rule
await sleep(180);

while (attempts < maxAttempts) {
try {
console.log(`Attempt ${attempts + 1}/${maxAttempts}`);

const outputDir = `${artifactPath}/changed-files.txt`;
const changedFilesArtifactUrl = await getArtifactUrl(branch, headCommitHash, jobName, artifactName);

await downloadArtifact(changedFilesArtifactUrl, outputDir);

changedFilesContent = readFileContent(outputDir);

if (changedFilesContent) {
console.log('Artifact downloaded and read successfully.');
break;
}
} catch (error) {
console.error(`Error fetching artifact: ${error.message}`);
}

attempts++;
if (attempts < maxAttempts) {
console.log(`Retrying in 15 seconds... (${attempts}/${maxAttempts})`);
await sleep(15);
}
}

if (!changedFilesContent) {
console.error('No artifacts found for changed files. Exiting with failure.');
process.exit(1);
}

e2eFiles = filterE2eChangedFiles(changedFilesContent.split('\n').filter(file => file.trim() !== ''));
console.log('e2e changed files', e2eFiles);
} else {
// Running locally
console.log('Running locally, performing git diff against main branch...');
const diffOutput = execSync('git diff --name-only main...HEAD').toString().trim();
const changedFiles = diffOutput.split('\n').filter(file => file.trim() !== '');
console.log('Changed files:', changedFiles);

e2eFiles = filterE2eChangedFiles(changedFiles);
console.log('Filtered E2E files:', e2eFiles);
}

if (e2eFiles.length === 0) {
console.log('No E2E files to validate. Exiting successfully.');
process.exit(0);
}

// Check each E2E file for page object usage
for (const file of e2eFiles) {
try {
const content = fs.readFileSync(file, 'utf8');
// Check for the presence of page object imports
const usesPageObjectModel = content.includes('./page-objects/');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this has an underlying assumption that all specs that use POM will have at least 1 page-objects import, and that those who don't, won't have any.
This could be 'tricked' or not apply in some cases but it's hard to tell when would this happen

Copy link
Contributor

Choose a reason for hiding this comment

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

agree, we can keep it like this for now and adjust it in the future if any exceptions arise


if (!usesPageObjectModel) {
console.error(`\x1b[31mFailure: You need to use Page Object Model in ${file}\x1b[0m`);
process.exit(1);
}
} catch (error) {
if (error.code === 'ENOENT') {
console.warn(`File not found: ${file}`);
continue; // Skip this file because it was deleted, so no need to validate
} else {
throw error; // Re-throw if it's a different error
}
}
}

console.log("\x1b[32mSuccess: All the new or modified E2E files use the Page Object Model.\x1b[0m");
}

// Run the verification
verifyE2ePageObjectsUsage().catch((error) => {
console.error('Not all the modified e2e use the Page Object Model', error);
process.exit(1);
});
4 changes: 4 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ jobs:
name: Wait for CircleCI workflow status
uses: ./.github/workflows/wait-for-circleci-workflow-status.yml

validate-page-object-usage:
name: Verify E2E Page Object Usage on modified files
uses: ./.github/workflows/validate-page-object-usage.yml

runway:
name: Runway
# For a `pull_request` event, the branch is `github.head_ref``.
Expand Down
Loading
Loading