[terra-functional-testing] Fixing Nexus screenshot bugs

packages/terra-functional-testing/CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+* Changed
+  * Updated upload and download logic in Nexus screenshots based on current locale, theme, browser, and formFactor
+  * Updated PR comment logic in Nexus screenshots.
+  * Updated Nexus mismatch warning to display testname and to be outputted via Logger
+
 ## 4.5.0 - (December 11, 2023)
 
 * Changed

packages/terra-functional-testing/src/commands/expect/toMatchReference.js

@@ -1,4 +1,10 @@
+const path = require('path');
+const fs = require('fs-extra');
+const { Logger } = require('@cerner/terra-cli');
+const getOutputDir = require('../../reporters/spec-reporter/get-output-dir');
 const { BUILD_BRANCH, BUILD_TYPE } = require('../../constants');
+
+const logger = new Logger({ prefix: '[terra-functional-testing:toMatchReference]' });
 /**
  * An assertion method to be paired with Visual Regression Service to assert each screenshot is within
  * the mismatch tolerance and are the same size.
@@ -11,7 +17,7 @@ const { BUILD_BRANCH, BUILD_TYPE } = require('../../constants');
  * @param {boolean} screenshot.screenshotWasUpdated - If the reference screenshot was updated with the latest captured screenshot.
  * @returns {Object} - An object that indicates if the assertion passed or failed with a message.
  */
-function toMatchReference(screenshot) {
+function toMatchReference(screenshot, testName) {
   const {
     isNewScreenshot,
     isSameDimensions,
@@ -41,11 +47,19 @@ function toMatchReference(screenshot) {
   if (global.Terra.serviceOptions.useRemoteReferenceScreenshots && !pass
     && (global.Terra.serviceOptions.buildBranch.match(BUILD_BRANCH.pullRequest) || global.Terra.serviceOptions.buildType === BUILD_TYPE.branchEventCause)) {
     pass = true;
-    process.env.SCREENSHOT_MISMATCH_CHECK = true;
-    if (message.length > 0) {
-      message = message.concat('\n');
+    const outputDir = getOutputDir();
+    const fileName = path.join(outputDir, 'ignored-mismatch.json');
+    // Create the output directory if it does not already exist.
+    if (!fs.existsSync(outputDir)) {
+      fs.mkdirSync(outputDir, { recursive: true });
+      // Since  output directory didn't exist file couldnt so just go ahead and make the file
+      fs.writeFileSync(fileName, JSON.stringify({ screenshotMismatched: true }, null, 2));
+    } else if (!fs.existsSync(fileName)) {
+      // If output directory exists but mismatch file has not been created create one
+      fs.writeFileSync(fileName, JSON.stringify({ screenshotMismatched: true }, null, 2));
     }
-    message = message.concat('Screenshot has changed and needs to be reviewed.');
+
+    logger.info(`Test: '${testName}' has a mismatch difference of ${misMatchPercentage}% and needs to be reviewed.`);
   }
 
   return {

packages/terra-functional-testing/src/commands/utils/ScreenshotRequestor.js

@@ -55,10 +55,11 @@ class ScreenshotRequestor {
 
   /**
    * Deletes the existing screenshots
+   * @param {string} referenceName - the name of the reference screenshots file to download
    */
-  async deleteExistingScreenshots() {
+  async deleteExistingScreenshots(referenceName) {
     const response = await fetch(
-      this.serviceUrl,
+      `${this.serviceUrl}${referenceName}/`,
       {
         method: 'DELETE',
         headers: {
@@ -80,6 +81,26 @@ class ScreenshotRequestor {
     fs.removeSync(archiveName);
   }
 
+  /**
+   * Makes string to return for the screenshots to save and download
+   * Naming convention is {theme}-{locale}-{browser}-{formfactor} and if provided string is empty does not add it.
+   * @param {string} locale - the locale to use when downloading
+   * @param {string} theme - the theme to use when downloading
+   * @param {string} formFactor - the formFactor to use when downloading
+   * @param {string} browser - the browser to use when uploading
+   */
+  static makeReferenceName(locale, theme, formFactor, browser) {
+    const referenceName = [theme, locale, browser, formFactor].filter(
+      (str) => {
+        if (str !== '' || str !== undefined) {
+          return str;
+        }
+        return false;
+      },
+    ).join('-');
+    return referenceName;
+  }
+
   /**
    * Zips the latest screenshots.
    */
@@ -104,7 +125,7 @@ class ScreenshotRequestor {
 
     const archiveName = path.join(this.zipFilePath, 'latest.zip');
 
-    // Name the uploaded file reference.zip since the latest screenshots will now be used as the reference screenshots.
+    // Name the uploaded file the passed referenceName since the latest screenshots will now be used as the reference screenshots.
     archive.file(archiveName, { name: 'reference.zip' });
 
     await archive.finalize();
@@ -115,20 +136,21 @@ class ScreenshotRequestor {
 
   /**
    * Downloads the screenshots and unzip it to the reference screenshot directory defined by referenceScreenshotsPath.
+   * @param {string} referenceName - the name of the reference screenshots file to download
    */
-  async downloadScreenshots() {
+  async downloadScreenshots(referenceName) {
     let archiveUrl;
     let fetchOptions;
     if (this.serviceAuthHeader !== undefined) {
-      archiveUrl = `${this.serviceUrl}/reference.zip`;
+      archiveUrl = `${this.serviceUrl}${referenceName}/reference.zip`;
       fetchOptions = {
         method: 'GET',
         headers: {
           Authorization: this.serviceAuthHeader,
         },
       };
     } else {
-      archiveUrl = `${this.url}/reference.zip`;
+      archiveUrl = `${this.url}${referenceName}/reference.zip`;
       fetchOptions = {
         method: 'GET',
       };
@@ -139,7 +161,7 @@ class ScreenshotRequestor {
     );
 
     if (response.status === 404) {
-      logger.info(`No screenshots downloaded from ${this.url}. Either the URL is invalid or no screenshots were previously uploaded.`);
+      logger.info(`No screenshots downloaded from ${this.url}${referenceName}. Either the URL is invalid or no screenshots were previously uploaded.`);
       return;
     }
 
@@ -153,7 +175,7 @@ class ScreenshotRequestor {
         writeStream.on('finish', async () => {
           await extract('terra-wdio-screenshots.zip', { dir: this.referenceScreenshotsPath });
           fs.removeSync('terra-wdio-screenshots.zip');
-          logger.info(`Screenshots downloaded from ${this.url}`);
+          logger.info(`Screenshots downloaded from ${this.url}${referenceName}`);
           resolve();
         });
       } catch (error) {
@@ -167,12 +189,13 @@ class ScreenshotRequestor {
   /**
    * Uploads the site zip contained in memoryStream
    * @param {MemoryStream} memoryStream - the MemoryStream to use when uploading
+   * @param {string} referenceName - the name of the reference zip to use when uploading
    */
-  async uploadScreenshots(memoryStream) {
+  async uploadScreenshots(memoryStream, referenceName) {
     const formData = new FormData();
     formData.append('file', memoryStream, { filename: 'reference.zip', knownLength: memoryStream.length });
     const response = await fetch(
-      this.serviceUrl,
+      `${this.serviceUrl}${referenceName}/`,
       {
         method: 'PUT',
         headers: {
@@ -183,22 +206,32 @@ class ScreenshotRequestor {
     );
     ScreenshotRequestor.checkStatus(response);
 
-    logger.info(`Screenshots are uploaded to ${this.url}`);
+    logger.info(`Screenshots are uploaded to ${this.url}${referenceName}`);
   }
 
   /**
    * Downloads the screenshots.
+   * @param {string} locale - the locale to use when downloading
+   * @param {string} theme - the theme to use when downloading
+   * @param {string} formFactor - the formFactor to use when downloading
+   * @param {string} browser - the browser to use when uploading
    */
-  async download() {
-    await this.downloadScreenshots();
+  async download(locale, theme, formFactor, browser) {
+    const referenceName = ScreenshotRequestor.makeReferenceName(locale, theme, formFactor, browser);
+    await this.downloadScreenshots(referenceName);
   }
 
   /**
    * Uploads the screenshots by deleting the existing screenshots, zipping the new ones, and uploading it
+   * @param {string} locale - the locale to use when uploading
+   * @param {string} theme - the theme to use when uploading
+   * @param {string} formFactor - the formFactor to use when uploading
+   * @param {string} browser - the browser to use when uploading
    */
-  async upload() {
+  async upload(locale, theme, formFactor, browser) {
+    const referenceName = ScreenshotRequestor.makeReferenceName(locale, theme, formFactor, browser);
     // Delete the existing screenshots from the remote repository because new screenshots will be uploaded.
-    await this.deleteExistingScreenshots();
+    await this.deleteExistingScreenshots(referenceName);
 
     // Zip up the existing latest screenshots
     await this.zipLatestScreenshots();
@@ -207,7 +240,7 @@ class ScreenshotRequestor {
     const memoryStream = await this.zipDirectoryToMemory();
 
     // Upload the screenshots to the remote repository
-    await this.uploadScreenshots(memoryStream);
+    await this.uploadScreenshots(memoryStream, referenceName);
 
     // The zipped latest screenshots can now be safely deleted.
     this.deleteZippedLatestScreenshots();

packages/terra-functional-testing/src/commands/validates/screenshot.js

@@ -22,7 +22,7 @@ const screenshot = (testName, options = {}) => {
 
   const screenshotResult = global.browser.checkElement(selector || global.Terra.serviceOptions.selector, wrappedOptions);
 
-  global.expect(screenshotResult).toMatchReference();
+  global.expect(screenshotResult).toMatchReference(testName);
 };
 
 module.exports = screenshot;

packages/terra-functional-testing/src/services/wdio-terra-service/WDIOTerraService.js

@@ -2,6 +2,7 @@ const path = require('path');
 const expect = require('expect');
 const fs = require('fs-extra');
 const { SevereServiceError } = require('webdriverio');
+const getOutputDir = require('../../reporters/spec-reporter/get-output-dir');
 const { accessibility, element, screenshot } = require('../../commands/validates');
 const { toBeAccessible, toMatchReference } = require('../../commands/expect');
 const {
@@ -64,6 +65,10 @@ class WDIOTerraService {
       gitToken,
       issueNumber,
       useRemoteReferenceScreenshots,
+      locale,
+      theme,
+      formFactor,
+      browser,
     } = this.serviceOptions;
 
     if (!useRemoteReferenceScreenshots) {
@@ -90,7 +95,7 @@ class WDIOTerraService {
         screenshotConfig = this.getRemoteScreenshotConfiguration(this.screenshotsSites, buildBranch);
       }
       const screenshotRequestor = new ScreenshotRequestor(screenshotConfig.publishScreenshotConfiguration);
-      await screenshotRequestor.download();
+      await screenshotRequestor.download(locale, theme, formFactor, browser);
     } catch (error) {
       throw new SevereServiceError(error);
     }
@@ -211,9 +216,7 @@ class WDIOTerraService {
       ':warning: :bangbang: **WDIO MISMATCH**\n\n',
       `Check that screenshot change is intended at: ${buildUrl}\n\n`,
       'If screenshot change is intended, remote reference screenshots will be updated upon PR merge.\n',
-      'If screenshot change is unintended, please fix screenshot issues before PR merge to prevent them from being uploaded.\n\n',
-      'Note: This comment only appears the first time a screenshot mismatch is detected on a PR build, ',
-      'future builds will need to be checked for unintended screenshot mismatches.',
+      'If screenshot change is unintended, please fix screenshot issues before PR merge to prevent them from being uploaded.',
     ].join('');
 
     const comments = await issue.getComments();
@@ -230,12 +233,16 @@ class WDIOTerraService {
   async uploadBuildBranchScreenshots() {
     const {
       buildBranch,
+      locale,
+      theme,
+      formFactor,
+      browser,
     } = this.serviceOptions;
 
     try {
       const screenshotConfig = this.getRemoteScreenshotConfiguration(this.screenshotsSites, buildBranch);
       const screenshotRequestor = new ScreenshotRequestor(screenshotConfig.publishScreenshotConfiguration);
-      await screenshotRequestor.upload();
+      await screenshotRequestor.upload(locale, theme, formFactor, browser);
     } catch (err) {
       throw new SevereServiceError(err);
     }
@@ -256,13 +263,21 @@ class WDIOTerraService {
       return;
     }
 
+    const fileName = path.join(getOutputDir(), 'ignored-mismatch.json');
+
     try {
-      if (process.env.SCREENSHOT_MISMATCH_CHECK === 'true' && buildBranch.match(BUILD_BRANCH.pullRequest)) {
+      if (fs.existsSync(fileName) && buildBranch.match(BUILD_BRANCH.pullRequest)) {
         // We found a screenshot mismatch during our build of this PR branch.
         await this.postMismatchWarningOnce();
+        // Remove mismatch flag file after running
+        fs.removeSync(fileName);
       } else if (!buildBranch.match(BUILD_BRANCH.pullRequest) && buildType === BUILD_TYPE.branchEventCause) {
         // This non-PR branch is being merged or someone pushed code into it directly.
         await this.uploadBuildBranchScreenshots();
+        // Remove mismatch flag file after running if it exists
+        if (fs.existsSync(fileName)) {
+          fs.removeSync(fileName);
+        }
       }
     } catch (err) {
       // The service will stop only if a SevereServiceError is thrown.