fix(action): delete S3 images for hash when visual tests pass on retry (#759)

Author: danadajian

action/dist/main.js

@@ -81447,6 +81447,9 @@ function putObject(input) {
 function copyObject(input) {
   return s3Client.send(new import_client_s3.CopyObjectCommand(input));
 }
+function deleteObjects(input) {
+  return s3Client.send(new import_client_s3.DeleteObjectsCommand(input));
+}
 function encodeS3CopySource(bucket, key) {
   return `${bucket}/${key.split("/").map(encodeURIComponent).join("/")}`;
 }
@@ -156282,6 +156285,26 @@ var uploadOriginalNewImages = async (hash) => {
   }
   return uploadOriginalNewPngs(screenshotsDirectory, bucketName, `${ORIGINAL_NEW_IMAGES_DIRECTORY}/${hash}/`);
 };
+var deleteHashImages = async (hash) => {
+  const bucketName = getInput("bucket-name", { required: true });
+  const [newImageKeys, originalImageKeys] = await Promise.all([
+    getKeysFromS3(NEW_IMAGES_DIRECTORY, hash, bucketName),
+    getKeysFromS3(ORIGINAL_NEW_IMAGES_DIRECTORY, hash, bucketName)
+  ]);
+  const keysToDelete = [...newImageKeys, ...originalImageKeys];
+  if (!keysToDelete.length) {
+    info(`No images found in S3 for hash ${hash}. Skipping deletion.`);
+    return;
+  }
+  await deleteObjects({
+    Bucket: bucketName,
+    Delete: {
+      Objects: keysToDelete.map((Key) => ({ Key })),
+      Quiet: true
+    }
+  });
+  info(`Deleted ${keysToDelete.length} image(s) for ${hash}`);
+};
 
 // ../node_modules/@actions/github/lib/context.js
 import { readFileSync, existsSync as existsSync2 } from "fs";
@@ -160257,8 +160280,12 @@ var run = async () => {
   }
   const latestVisualRegressionStatus = commitHash ? await getLatestVisualRegressionStatus(commitHash) : null;
   const isRetry = context2.runAttempt > 1;
-  if (diffFileCount === 0 && newFileCount === 0) {
+  const testsPassed = diffFileCount === 0 && newFileCount === 0;
+  if (testsPassed) {
     info("All visual tests passed, and no diffs found!");
+    if (isRetry) {
+      await deleteHashImages(hash);
+    }
     if (!commitHash)
       return;
     if (isRetry) {
@@ -160323,5 +160350,5 @@ var run = async () => {
 // src/main.ts
 run();
 
-//# debugId=0149D6459182F2B064756E2164756E21
+//# debugId=47BE8D1DB8D72B8E64756E2164756E21
 //# sourceMappingURL=main.js.map

action/dist/main.js.map

@@ -7,6 +7,7 @@ import {
   warning
 } from '@actions/core';
 import {
+  deleteHashImages,
   downloadBaseImages,
   uploadAllImages,
   uploadOriginalNewImages
@@ -127,9 +128,14 @@ export const run = async () => {
 
   const isRetry = context.runAttempt > 1;
 
-  if (diffFileCount === 0 && newFileCount === 0) {
+  const testsPassed = diffFileCount === 0 && newFileCount === 0;
+  if (testsPassed) {
     info('All visual tests passed, and no diffs found!');
 
+    if (isRetry) {
+      await deleteHashImages(hash);
+    }
+
     if (!commitHash) return;
     if (isRetry) {
       warning(

action/src/run.ts

@@ -6,7 +6,14 @@ import {
   NEW_IMAGE_NAME,
   ORIGINAL_NEW_IMAGES_DIRECTORY
 } from 'shared/constants';
-import { getObject, listAllObjects, listObjects, putObject } from 'shared/s3';
+import {
+  deleteObjects,
+  getKeysFromS3,
+  getObject,
+  listAllObjects,
+  listObjects,
+  putObject
+} from 'shared/s3';
 import { map } from 'bluebird';
 import * as path from 'path';
 import * as fs from 'fs';
@@ -242,6 +249,32 @@ export const uploadOriginalNewImages = async (hash: string) => {
   );
 };
 
+export const deleteHashImages = async (hash: string) => {
+  const bucketName = getInput('bucket-name', { required: true });
+
+  const [newImageKeys, originalImageKeys] = await Promise.all([
+    getKeysFromS3(NEW_IMAGES_DIRECTORY, hash, bucketName),
+    getKeysFromS3(ORIGINAL_NEW_IMAGES_DIRECTORY, hash, bucketName)
+  ]);
+
+  const keysToDelete = [...newImageKeys, ...originalImageKeys];
+
+  if (!keysToDelete.length) {
+    info(`No images found in S3 for hash ${hash}. Skipping deletion.`);
+    return;
+  }
+
+  await deleteObjects({
+    Bucket: bucketName,
+    Delete: {
+      Objects: keysToDelete.map(Key => ({ Key })),
+      Quiet: true
+    }
+  });
+
+  info(`Deleted ${keysToDelete.length} image(s) for ${hash}`);
+};
+
 export const uploadBaseImages = async (newFilePaths: string[]) => {
   info(`Uploading ${newFilePaths.length} base image(s)`);
   return map(newFilePaths, newFilePath =>

action/src/s3-operations.ts

@@ -63,6 +63,8 @@ const listObjectsMock = mock();
 const getObjectMock = mock();
 const putObjectMock = mock();
 const copyObjectMock = mock();
+const deleteObjectsMock = mock();
+const getKeysFromS3Mock = mock();
 const updateBaseImagesMock = mock();
 async function listAllObjects(
   input: { Bucket: string; Prefix: string },
@@ -83,11 +85,12 @@ mock.module('shared/s3', () => ({
   s3Client: {},
   listObjects: listObjectsMock,
   listAllObjects,
-  getKeysFromS3: mock(),
+  getKeysFromS3: getKeysFromS3Mock,
   updateBaseImages: updateBaseImagesMock,
   getObject: getObjectMock,
   putObject: putObjectMock,
-  copyObject: copyObjectMock
+  copyObject: copyObjectMock,
+  deleteObjects: deleteObjectsMock
 }));
 
 const jimpImageMock = {
@@ -215,6 +218,8 @@ describe('main', () => {
     getObjectMock.mockResolvedValue({ Body: null });
     putObjectMock.mockResolvedValue({});
     copyObjectMock.mockResolvedValue({});
+    deleteObjectsMock.mockResolvedValue({});
+    getKeysFromS3Mock.mockResolvedValue([]);
     updateBaseImagesMock.mockResolvedValue(undefined);
     mkdirMock.mockResolvedValue(undefined);
     readFileMock.mockResolvedValue(Buffer.from('image-data'));
@@ -750,6 +755,59 @@ describe('main', () => {
     expect(createCommitStatusMock).toHaveBeenCalled();
   });
 
+  it('should delete S3 images for hash when tests pass on retry', async () => {
+    githubContext.runAttempt = 2;
+    execMock.mockResolvedValue(0);
+    globMock.mockResolvedValue(['path/to/screenshots/base.png']);
+    getKeysFromS3Mock.mockResolvedValueOnce([
+      'new-images/sha/component/new.png'
+    ]);
+    getKeysFromS3Mock.mockResolvedValueOnce([]);
+    await runAction();
+    expect(deleteObjectsMock).toHaveBeenCalledWith({
+      Bucket: 'some-bucket',
+      Delete: {
+        Objects: [{ Key: 'new-images/sha/component/new.png' }],
+        Quiet: true
+      }
+    });
+  });
+
+  it('should delete S3 images for hash when tests pass on retry with diff-id input', async () => {
+    githubContext.runAttempt = 2;
+    getInputMock.mockImplementation(name => diffIdInputMap[name]);
+    execMock.mockResolvedValue(0);
+    globMock.mockResolvedValue(['path/to/screenshots/base.png']);
+    getKeysFromS3Mock.mockResolvedValueOnce([
+      'new-images/uniqueId/component/new.png'
+    ]);
+    getKeysFromS3Mock.mockResolvedValueOnce([]);
+    await runAction();
+    expect(deleteObjectsMock).toHaveBeenCalledWith({
+      Bucket: 'some-bucket',
+      Delete: {
+        Objects: [{ Key: 'new-images/uniqueId/component/new.png' }],
+        Quiet: true
+      }
+    });
+  });
+
+  it('should not delete S3 images when tests pass on first attempt', async () => {
+    execMock.mockResolvedValue(0);
+    globMock.mockResolvedValue(['path/to/screenshots/base.png']);
+    await runAction();
+    expect(deleteObjectsMock).not.toHaveBeenCalled();
+  });
+
+  it('should skip deletion when no images exist in S3 on retry', async () => {
+    githubContext.runAttempt = 2;
+    execMock.mockResolvedValue(0);
+    globMock.mockResolvedValue(['path/to/screenshots/base.png']);
+    getKeysFromS3Mock.mockResolvedValue([]);
+    await runAction();
+    expect(deleteObjectsMock).not.toHaveBeenCalled();
+  });
+
   it('should call setFailed with the diff message when visual-test-command-fails-on-diff is true', async () => {
     execMock.mockResolvedValue(0);
     getBooleanInputMock.mockImplementation(name =>

action/test/run.test.ts

@@ -11,7 +11,10 @@ import {
   PutObjectCommandOutput,
   CopyObjectCommand,
   CopyObjectCommandInput,
-  CopyObjectCommandOutput
+  CopyObjectCommandOutput,
+  DeleteObjectsCommand,
+  DeleteObjectsCommandInput,
+  DeleteObjectsCommandOutput
 } from '@aws-sdk/client-s3';
 import {
   BASE_IMAGES_DIRECTORY,
@@ -61,6 +64,12 @@ export function copyObject(
   return s3Client.send(new CopyObjectCommand(input));
 }
 
+export function deleteObjects(
+  input: DeleteObjectsCommandInput
+): Promise<DeleteObjectsCommandOutput> {
+  return s3Client.send(new DeleteObjectsCommand(input));
+}
+
 function encodeS3CopySource(bucket: string, key: string): string {
   return `${bucket}/${key.split('/').map(encodeURIComponent).join('/')}`;
 }

shared/s3.ts

@@ -37,7 +37,8 @@ mock.module('@aws-sdk/client-s3', () => ({
   ListObjectsV2Command,
   GetObjectCommand: class {},
   PutObjectCommand: class {},
-  CopyObjectCommand
+  CopyObjectCommand,
+  DeleteObjectsCommand: class {}
 }));
 
 const {