refactor: remove unnecessary base images directory option (#215)

README.md

@@ -28,6 +28,8 @@ and when you use the Comparadise app to review changes.
 
 ## Writing Visual Tests
 
+Comparadise relies on Cypress for running tests against web components in the browser.
+
 Install the [comparadise-utils](https://www.npmjs.com/package/comparadise-utils) package using your favorite node package manager.
 
 In `cypress.config.ts`:
@@ -64,6 +66,30 @@ In a Cypress test that renders your component or visits your site, use `cy.match
 This will take a screenshot of whatever Cypress is currently displaying, compare it to a `base.png` that was
 previously downloaded from S3, and output a `diff.png` and `new.png` if there is a visual change.
 
+Example test:
+
+```tsx
+describe('MyComponent visual test', () => {
+  it('should verify MyComponent looks the same', () => {
+    cy.mount(<MyComponent inputs={mockInputs} />);
+
+    cy.matchScreenshot();
+  });
+});
+```
+
+### matchScreenshot Arguments
+
+#### rawName - optional (String)
+
+By default, `matchScreenshot` will infer the name of your test from the name of your file and create a folder to save the base, new, and diff images to.
+
+However, if you have multiple visual tests in a single file, you should pass a different `rawName` for each test to vary the paths where screenshots will be saved.
+
+#### options - optional (Cypress.ScreenshotOptions)
+
+Learn more about the `options` argument definition in [Cypress' official docs](https://docs.cypress.io/api/commands/screenshot.html#Arguments).
+
 ## Executing Your Visual Tests
 
 Usage:

action.yml

@@ -18,10 +18,6 @@ inputs:
   package-paths:
     description: 'Comma separated list of package paths for monorepos'
     required: false
-  base-images-directory:
-    description: 'The comment you would like to leave on PRs when visual tests fail'
-    required: false
-    default: 'base-images'
   comparadise-host:
     description: 'The URL at which you are hosting Comparadise'
     required: false

action/dist/index.js

@@ -12028,15 +12028,15 @@ Object.defineProperty(exports, "__esModule", ({ value: true }));
 exports.uploadBaseImages = exports.downloadBaseImages = void 0;
 const exec_1 = __nccwpck_require__(5045);
 const core_1 = __nccwpck_require__(3610);
+const shared_1 = __nccwpck_require__(9201);
 const downloadBaseImages = async () => {
     const bucketName = (0, core_1.getInput)('bucket-name', { required: true });
     const screenshotsDirectory = (0, core_1.getInput)('screenshots-directory');
-    const baseImagesDirectory = (0, core_1.getInput)('base-images-directory');
     const packagePaths = (0, core_1.getInput)('package-paths')?.split(',');
     if (packagePaths) {
-        return Promise.all(packagePaths.map(packagePath => (0, exec_1.exec)(`aws s3 cp s3://${bucketName}/${baseImagesDirectory}/${packagePath} ${screenshotsDirectory}/${packagePath} --recursive`)));
+        return Promise.all(packagePaths.map(packagePath => (0, exec_1.exec)(`aws s3 cp s3://${bucketName}/${shared_1.BASE_IMAGES_DIRECTORY}/${packagePath} ${screenshotsDirectory}/${packagePath} --recursive`)));
     }
-    return (0, exec_1.exec)(`aws s3 cp s3://${bucketName}/${baseImagesDirectory} ${screenshotsDirectory} --recursive`);
+    return (0, exec_1.exec)(`aws s3 cp s3://${bucketName}/${shared_1.BASE_IMAGES_DIRECTORY} ${screenshotsDirectory} --recursive`);
 };
 exports.downloadBaseImages = downloadBaseImages;
 const uploadBaseImages = async () => {

action/src/s3-operations.ts

@@ -1,23 +1,23 @@
 import { exec } from '@actions/exec';
 import { getInput } from '@actions/core';
+import { BASE_IMAGES_DIRECTORY } from 'shared';
 
 export const downloadBaseImages = async () => {
   const bucketName = getInput('bucket-name', { required: true });
   const screenshotsDirectory = getInput('screenshots-directory');
-  const baseImagesDirectory = getInput('base-images-directory');
   const packagePaths = getInput('package-paths')?.split(',');
   if (packagePaths) {
     return Promise.all(
       packagePaths.map(packagePath =>
         exec(
-          `aws s3 cp s3://${bucketName}/${baseImagesDirectory}/${packagePath} ${screenshotsDirectory}/${packagePath} --recursive`
+          `aws s3 cp s3://${bucketName}/${BASE_IMAGES_DIRECTORY}/${packagePath} ${screenshotsDirectory}/${packagePath} --recursive`
         )
       )
     );
   }
 
   return exec(
-    `aws s3 cp s3://${bucketName}/${baseImagesDirectory} ${screenshotsDirectory} --recursive`
+    `aws s3 cp s3://${bucketName}/${BASE_IMAGES_DIRECTORY} ${screenshotsDirectory} --recursive`
   );
 };
 

action/test/run.test.ts

@@ -3,7 +3,7 @@ import { exec } from '@actions/exec';
 import { getInput, getMultilineInput, setFailed } from '@actions/core';
 import { octokit } from '../src/octokit';
 import { sync } from 'glob';
-import { VISUAL_REGRESSION_CONTEXT } from 'shared';
+import { BASE_IMAGES_DIRECTORY, VISUAL_REGRESSION_CONTEXT } from 'shared';
 
 jest.mock('glob');
 jest.mock('@actions/core');
@@ -39,7 +39,6 @@ const inputMap: Record<string, string> = {
   'screenshots-directory': 'path/to/screenshots',
   'bucket-name': 'some-bucket',
   'commit-hash': 'sha',
-  'base-images-directory': 'base-images',
   'github-token': 'some-token',
 };
 (getInput as jest.Mock).mockImplementation(name => inputMap[name]);
@@ -135,13 +134,13 @@ describe('main', () => {
     ]);
     await run();
     expect(exec).toHaveBeenCalledWith(
-      'aws s3 cp s3://some-bucket/base-images/path/1 path/to/screenshots/path/1 --recursive'
+      `aws s3 cp s3://some-bucket/${BASE_IMAGES_DIRECTORY}/path/1 path/to/screenshots/path/1 --recursive`
     );
     expect(exec).toHaveBeenCalledWith(
-      'aws s3 cp s3://some-bucket/base-images/path/2 path/to/screenshots/path/2 --recursive'
+      `aws s3 cp s3://some-bucket/${BASE_IMAGES_DIRECTORY}/path/2 path/to/screenshots/path/2 --recursive`
     );
     expect(exec).not.toHaveBeenCalledWith(
-      'aws s3 cp s3://some-bucket/base-images path/to/screenshots --recursive'
+      `aws s3 cp s3://some-bucket/${BASE_IMAGES_DIRECTORY} path/to/screenshots --recursive`
     );
     expect(exec).toHaveBeenCalledWith(
       'aws s3 cp path/to/screenshots/path/1 s3://some-bucket/sha/path/1 --recursive'

app/backend/src/schema.ts

@@ -10,7 +10,6 @@ export const updateBaseImagesInputSchema = z.object({
   bucket: z.string().min(1),
   repo: z.string().min(1),
   owner: z.string().min(1),
-  baseImagesDirectory: z.string().nullish(),
 });
 export const updateCommitStatusInputSchema = z.object({
   hash: z.string().min(1),

app/backend/src/updateBaseImagesInS3.ts

@@ -15,7 +15,6 @@ export const updateBaseImagesInS3 = async ({
   bucket,
   owner,
   repo,
-  baseImagesDirectory,
 }: UpdateBaseImagesInput) => {
   if (!(await allNonVisualChecksHavePassed(owner, repo, hash))) {
     throw new TRPCError({
@@ -24,11 +23,7 @@ export const updateBaseImagesInS3 = async ({
     });
   }
   const s3Paths = await getKeysFromS3(hash, bucket);
-  return await replaceImagesInS3(
-    s3Paths,
-    bucket,
-    baseImagesDirectory || BASE_IMAGES_DIRECTORY
-  );
+  return await replaceImagesInS3(s3Paths, bucket);
 };
 
 export const filterNewImages = (s3Paths: string[]) => {
@@ -37,25 +32,18 @@ export const filterNewImages = (s3Paths: string[]) => {
   );
 };
 
-export const getBaseImagePaths = (
-  newImagePaths: string[],
-  baseImagesDirectory: string
-) => {
+export const getBaseImagePaths = (newImagePaths: string[]) => {
   return newImagePaths.map(path => {
     const commitHash = path.split('/')[0];
     return path
-      .replace(commitHash, baseImagesDirectory)
+      .replace(commitHash, BASE_IMAGES_DIRECTORY)
       .replace(NEW_IMAGE_NAME, BASE_IMAGE_NAME);
   });
 };
 
-export const replaceImagesInS3 = async (
-  s3Paths: string[],
-  bucket: string,
-  baseImagesDirectory: string
-) => {
+export const replaceImagesInS3 = async (s3Paths: string[], bucket: string) => {
   const newImagePaths = filterNewImages(s3Paths);
-  const baseImagePaths = getBaseImagePaths(newImagePaths, baseImagesDirectory);
+  const baseImagePaths = getBaseImagePaths(newImagePaths);
   return await Promise.all(
     baseImagePaths.map((path, index) =>
       S3Client.copyObject({

app/backend/test/updateBaseImagesInS3.test.ts

@@ -37,29 +37,17 @@ describe('filterNewImages', () => {
 });
 
 describe('getBaseImagesPaths', () => {
-  it(`should return the base image path given the new image path and default to ${BASE_IMAGES_DIRECTORY}`, () => {
+  it('should return the base image paths given the new image paths', () => {
     const paths = [
       '030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5/SMALL/pdpPage/new.png',
       '030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5/LARGE/srpPage/new.png',
     ];
-    const result = getBaseImagePaths(paths, BASE_IMAGES_DIRECTORY);
+    const result = getBaseImagePaths(paths);
     expect(result).toEqual([
       `${BASE_IMAGES_DIRECTORY}/SMALL/pdpPage/base.png`,
       `${BASE_IMAGES_DIRECTORY}/LARGE/srpPage/base.png`,
     ]);
   });
-
-  it('should return the base image *test* path given the new image path', () => {
-    const paths = [
-      '030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5/SMALL/pdpPage/new.png',
-      '030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5/LARGE/srpPage/new.png',
-    ];
-    const result = getBaseImagePaths(paths, 'base-images-test');
-    expect(result).toEqual([
-      `base-images-test/SMALL/pdpPage/base.png`,
-      `base-images-test/LARGE/srpPage/base.png`,
-    ]);
-  });
 });
 
 describe('updateBaseImagesInS3', () => {
@@ -72,8 +60,7 @@ describe('updateBaseImagesInS3', () => {
         '030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5/SMALL/srpPage/base.png',
         '030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5/SMALL/pdpPage/base.png',
       ],
-      expectedBucket,
-      BASE_IMAGES_DIRECTORY
+      expectedBucket
     );
     expect(S3Client.copyObject).toHaveBeenCalledWith({
       Bucket: expectedBucket,
@@ -89,32 +76,6 @@ describe('updateBaseImagesInS3', () => {
     });
   });
 
-  it('should fetch the images from S3 test directory', async () => {
-    const expectedBucket = 'expectedBucket';
-    await replaceImagesInS3(
-      [
-        '030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5/SMALL/pdpPage/new.png',
-        '030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5/SMALL/srpPage/new.png',
-        '030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5/SMALL/srpPage/base.png',
-        '030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5/SMALL/pdpPage/base.png',
-      ],
-      expectedBucket,
-      'base-images-test'
-    );
-    expect(S3Client.copyObject).toHaveBeenCalledWith({
-      Bucket: expectedBucket,
-      CopySource: `${expectedBucket}/030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5/SMALL/pdpPage/new.png`,
-      Key: `base-images-test/SMALL/pdpPage/base.png`,
-      ACL: 'bucket-owner-full-control',
-    });
-    expect(S3Client.copyObject).toHaveBeenCalledWith({
-      Bucket: expectedBucket,
-      CopySource: `${expectedBucket}/030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5/SMALL/srpPage/new.png`,
-      Key: `base-images-test/SMALL/srpPage/base.png`,
-      ACL: 'bucket-owner-full-control',
-    });
-  });
-
   it('should throw error if other required checks have not yet passed', async () => {
     (allNonVisualChecksHavePassed as jest.Mock).mockResolvedValue(false);
 

app/frontend/components/update-images-button.tsx

@@ -11,9 +11,6 @@ import { Dialog, Transition } from '@headlessui/react';
 import { PrimaryButton, TertiaryButton } from './buttons';
 import { useSearchParams } from 'react-router-dom';
 
-const UPDATE_TEXT =
-  'WARNING: This will update the base images in S3 and will set the visual regression status to passed. You can only do this if you are about to merge your PR and all other checks have passed.';
-
 export const UpdateImagesButton = () => {
   const [dialogIsOpen, setDialogIsOpen] = useState(false);
   const { baseImageState, setBaseImageState } = useContext(
@@ -29,7 +26,7 @@ export const UpdateImagesButton = () => {
   const params: Record<string, string | undefined> = Object.fromEntries(
     searchParams.entries()
   );
-  const { hash, bucket, repo, owner, baseImagesDirectory } = params;
+  const { hash, bucket, repo, owner } = params;
   if (!hash || !bucket || !owner || !repo) {
     return null;
   }
@@ -44,7 +41,7 @@ export const UpdateImagesButton = () => {
 
   const handleUpdate = async () => {
     setBaseImageState?.(UpdateBaseImagesTexts.UPDATING);
-    await updateBaseImages({ hash, bucket, owner, repo, baseImagesDirectory });
+    await updateBaseImages({ hash, bucket, owner, repo });
     await updateCommitStatus({ hash, owner, repo });
     setDialogIsOpen(false);
     setBaseImageState?.(UpdateBaseImagesTexts.UPDATED);
@@ -55,17 +52,16 @@ export const UpdateImagesButton = () => {
     setBaseImageState?.(UpdateBaseImagesTexts.ERROR);
   }
 
-  const dialogTitleText = baseImagesDirectory
-    ? `Custom base image directory in use. This will update the base images in ${baseImagesDirectory}`
-    : 'Are you sure you want to update the base images?';
-  const dialogDescriptionText = !baseImagesDirectory ? UPDATE_TEXT : undefined;
+  const dialogTitleText = 'Are you sure you want to update the base images?';
+  const updateText =
+    'WARNING: This will update the base images in S3 and will set the visual regression status to passed. You can only do this if you are about to merge your PR and all other checks have passed.';
   const dialogContent = (
     <>
       <Dialog.Title as="h3" className="mt-2 text-xl font-semibold leading-6">
         {dialogTitleText}
       </Dialog.Title>
       <Dialog.Description className="mt-5 text-lg font-semibold text-slate-500">
-        {dialogDescriptionText}
+        {updateText}
       </Dialog.Description>
 
       <div className="mt-5 flex justify-end">
@@ -123,9 +119,6 @@ export const UpdateImagesButton = () => {
       >
         {baseImageState}
       </PrimaryButton>
-      {baseImagesDirectory && (
-        <p>Custom base image directory {baseImagesDirectory} in use</p>
-      )}
       <Transition appear show={dialogIsOpen} as={Fragment}>
         <Dialog as="div" className="relative z-10" onClose={handleDialogClose}>
           <Transition.Child

pnpm-workspace.yaml

@@ -1,2 +1,8 @@
 packages:
-  - ./**/*
+  - .
+  - action
+  - app
+  - app/frontend
+  - app/backend
+  - comparadise-utils
+  - shared