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

.circleci/config.yml

@@ -379,6 +379,14 @@ jobs:
           root: .
           paths:
             - changed-files
+      - run:
+          name: Save changed files as artifact
+          command: |
+            mkdir -p changed-files
+            cp changed-files/changed-files.txt changed-files/
+      - store_artifacts:
+          path: changed-files
+          destination: changed-files
 
   validate-locales-only:
     executor: node-browsers-small

.circleci/scripts/test-run-e2e-timeout-minutes.ts

@@ -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);
 
     // 20 minutes, plus 3 minutes for every changed file, up to a maximum of 30 minutes
     timeout = Math.min(20 + changedOrNewTests.length * 3, 30);

.github/workflows/main.yml

@@ -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``.

.github/workflows/utils/utils.ts

@@ -0,0 +1,73 @@
+import { execSync } from 'child_process';
+import fs from 'fs';
+
+const OWNER = 'MetaMask';
+const REPOSITORY = 'metamask-extension';
+
+/**
+ * Downloads an artifact from CircleCI.
+ * @param branch - The branch name.
+ * @param headCommitHash - The commit hash of the branch.
+ * @param artifactName - The name of the artifact to download.
+ * @param outputFilePath - The path to save the downloaded artifact.
+ * @param jobName - The name of the job that produced the artifact.
+ */
+export function downloadCircleCiArtifact(branch: string, headCommitHash: string, artifactName: string, outputFilePath: string, jobName: string): void {
+    // Get the pipeline ID for the current branch
+    const pipelineId = execSync(
+        `curl --silent "https://circleci.com/api/v2/project/gh/${OWNER}/${REPOSITORY}/pipeline?branch=${branch}" | jq --arg head_commit_hash "${headCommitHash}" -r '.items | map(select(.vcs.revision == $head_commit_hash)) | first | .id'`
+    ).toString().trim();
+
+    // Get the workflow ID for the pipeline
+    const workflowId = execSync(
+        `curl --silent "https://circleci.com/api/v2/pipeline/${pipelineId}/workflow" | jq -r '.items[0].id'`
+    ).toString().trim();
+
+    // Get the job details for the specific job that produces artifacts
+    const jobDetails = execSync(
+        `curl --silent "https://circleci.com/api/v2/workflow/${workflowId}/job" | jq --arg job_name "${jobName}" -r '.items[] | select(.name == $job_name)'`
+    ).toString();
+
+    const jobId = JSON.parse(jobDetails).id;
+
+    // Get the artifact URL
+    const artifactList = execSync(
+        `curl --silent "https://circleci.com/api/v2/project/gh/${OWNER}/${REPOSITORY}/${jobId}/artifacts"`
+    ).toString();
+    const artifact = JSON.parse(artifactList).items.find((item: any) => item.path.includes(artifactName));
+
+    if (!artifact) {
+        throw new Error(`Artifact ${artifactName} not found`);
+    }
+
+    const artifactUrl = artifact.url;
+
+    // Download the artifact
+    execSync(`curl --silent --location "${artifactUrl}" --output "${outputFilePath}"`);
+
+    if (!fs.existsSync(outputFilePath)) {
+        throw new Error(`Failed to download artifact to ${outputFilePath}`);
+    }
+}
+
+/**
+ * Reads the content of an artifact file.
+ * @param filePath - The path to the downloaded artifact.
+ * @returns The content of the artifact file.
+ */
+export function readArtifact(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));
+}

.github/workflows/utils/validate-e2e-page-object-usage.ts

@@ -0,0 +1,88 @@
+import fs from 'fs';
+import { execSync } from 'child_process';
+import { filterE2eChangedFiles } from '../../../test/e2e/changedFilesUtil';
+import { downloadCircleCiArtifact, readArtifact, sleep } from './utils';
+
+async function verifyE2ePageObjectsUsage() {
+    let e2eFiles: string[];
+
+    if (process.env.GITHUB_ACTIONS) {
+        // Running in Github Actions
+        const branch = process.env.GITHUB_REF_NAME || '';
+        const headCommitHash = process.env.GITHUB_SHA || '';
+        const artifactName = 'changed-files.txt';
+        const artifactPath = 'changed-files';
+        const jobName = 'get-changed-files-with-git-diff'; // Specify the job name
+
+        let attempts = 0;
+        const maxAttempts = 3;
+        let changedFilesContent = '';
+
+        while (attempts < maxAttempts) {
+            try {
+                console.log(`Downloading artifact: Attempt ${attempts + 1}/${maxAttempts}`);
+                const outputDir = `${artifactPath}/changed-files.txt`;
+                downloadCircleCiArtifact(branch, headCommitHash, artifactName, outputDir, jobName); // Pass the job name
+
+                changedFilesContent = readArtifact(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 60 seconds... (${attempts}/${maxAttempts})`);
+                await sleep(60000); // Wait for 60 seconds before retrying
+            }
+        }
+
+        if (!changedFilesContent) {
+            console.error('No artifacts found for changed files. Exiting with failure.');
+            process.exit(1);
+        }
+
+        // Use the filterE2eChangedFiles function to filter E2E files
+        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...');
+        const diffOutput = execSync('git diff --name-only 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) {
+        const content = fs.readFileSync(file, 'utf8');
+        // Check for the presence of page object imports
+        const usesPageObjectModel = content.includes("from './page-objects/") ||
+                                    content.includes("import") && content.includes("from '../../page-objects/");
+
+        if (!usesPageObjectModel) {
+            console.error(`\x1b[31mFailure: You need to use Page Object Model in ${file}\x1b[0m`);
+            process.exit(1);
+        }
+    }
+
+    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);
+});

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

@@ -0,0 +1,19 @@
+name: Validate E2E Page Object usage on modified files
+
+on:
+  pull_request:
+    types: [opened, synchronize]
+
+jobs:
+  run-e2e-validation:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Setup environment
+        uses: metamask/github-tools/.github/actions/setup-environment@main
+
+      - name: Run E2E Page Object Validation
+        run: |
+          yarn validate-e2e-page-object-usage

package.json

@@ -124,7 +124,8 @@
     "attributions:check": "./development/attributions-check.sh",
     "attributions:generate": "./development/generate-attributions.sh",
     "ci-rerun-from-failed": "tsx .circleci/scripts/rerun-ci-workflow-from-failed.ts",
-    "git-diff-default-branch": "tsx .circleci/scripts/git-diff-default-branch.ts"
+    "git-diff-default-branch": "tsx .circleci/scripts/git-diff-default-branch.ts",
+    "validate-e2e-page-object-usage": "tsx .github/workflows/utils/validate-e2e-page-object-usage.ts"
   },
   "resolutions": {
     "chokidar": "^3.6.0",

test/e2e/changedFilesUtil.js

@@ -11,7 +11,7 @@ const CHANGED_FILES_PATH = path.join(
 /**
  * Reads the list of changed files from the git diff file.
  *
- * @returns {<string[]>} An array of changed file paths.
+ * @returns {string[]} An array of changed file paths.
  */
 function readChangedFiles() {
   try {
@@ -29,10 +29,10 @@ function readChangedFiles() {
 /**
  * Filters the list of changed files to include only E2E test files within the 'test/e2e/' directory.
  *
- * @returns {<string[]>} An array of filtered E2E test file paths.
+ * @param {string[]} changedFiles - An array of changed file paths to filter.
+ * @returns {string[]} An array of filtered E2E test file paths.
  */
-function filterE2eChangedFiles() {
-  const changedFiles = readChangedFiles();
+function filterE2eChangedFiles(changedFiles) {
   const e2eChangedFiles = changedFiles
     .filter(
       (file) =>

test/e2e/run-all.js

@@ -6,7 +6,10 @@ const { hideBin } = require('yargs/helpers');
 const { runInShell } = require('../../development/lib/run-command');
 const { exitWithError } = require('../../development/lib/exit-with-error');
 const { loadBuildTypesConfig } = require('../../development/lib/build-type');
-const { filterE2eChangedFiles } = require('./changedFilesUtil');
+const {
+  filterE2eChangedFiles,
+  readChangedFiles,
+} = require('./changedFilesUtil');
 
 // These tests should only be run on Flask for now.
 const FLASK_ONLY_TESTS = [];
@@ -64,7 +67,8 @@ function applyQualityGate(fullTestList, changedOrNewTests) {
 
 // For running E2Es in parallel in CI
 function runningOnCircleCI(testPaths) {
-  const changedOrNewTests = filterE2eChangedFiles();
+  const changedFiles = readChangedFiles();
+  const changedOrNewTests = filterE2eChangedFiles(changedFiles);
   console.log('Changed or new test list:', changedOrNewTests);
 
   const fullTestList = applyQualityGate(

test/e2e/tests/account/add-account.spec.ts

@@ -16,7 +16,7 @@ import HomePage from '../../page-objects/pages/home/homepage';
 import LoginPage from '../../page-objects/pages/login-page';
 import ResetPasswordPage from '../../page-objects/pages/reset-password-page';
 
-describe('Add account', function () {
+describe('Add account TEST', function () {
   it('should not affect public address when using secret recovery phrase to recover account with non-zero balance', async function () {
     await withFixtures(
       {

test/e2e/tests/transaction/send-eth.spec.js

@@ -13,7 +13,7 @@ const {
 const FixtureBuilder = require('../../fixture-builder');
 
 describe('Send ETH', function () {
-  describe('from inside MetaMask', function () {
+  describe('from inside MetaMask TEST', function () {
     it('finds the transaction in the transactions list using default gas', async function () {
       await withFixtures(
         {