Skip to content

Commit f050c38

Browse files
authored
feat: use-base-images-param (#640)
Co-authored-by: danadajian <danadajian@users.noreply.github.com>
1 parent ee11f48 commit f050c38

File tree

11 files changed

+85
-17
lines changed

11 files changed

+85
-17
lines changed

action.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ inputs:
1414
diff-id:
1515
description: 'Alternative to commit-hash as a unique identifier for visual tests. GitHub integration will be disabled if diff-id is used.'
1616
required: false
17-
download-base-images:
18-
description: 'optionally enable downloading of base images from S3 to be skipped'
17+
use-base-images:
18+
description: 'Optionally disables base images from being used'
1919
required: false
2020
default: 'true'
2121
screenshots-directory:

action/dist/main.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19712,7 +19712,7 @@ var require_core = __commonJS({
1971219712
return inputs.map((input) => input.trim());
1971319713
}
1971419714
exports2.getMultilineInput = getMultilineInput2;
19715-
function getBooleanInput2(name, options) {
19715+
function getBooleanInput3(name, options) {
1971619716
const trueValue = ["true", "True", "TRUE"];
1971719717
const falseValue = ["false", "False", "FALSE"];
1971819718
const val = getInput6(name, options);
@@ -19723,7 +19723,7 @@ var require_core = __commonJS({
1972319723
throw new TypeError(`Input does not meet YAML 1.2 "Core Schema" specification: ${name}
1972419724
Support boolean input list: \`true | True | TRUE | false | False | FALSE\``);
1972519725
}
19726-
exports2.getBooleanInput = getBooleanInput2;
19726+
exports2.getBooleanInput = getBooleanInput3;
1972719727
function setOutput(name, value) {
1972819728
const filePath = process.env["GITHUB_OUTPUT"] || "";
1972919729
if (filePath) {
@@ -35903,8 +35903,9 @@ var buildComparadiseUrl = () => {
3590335903
const commitHash = (0, import_core3.getInput)("commit-hash");
3590435904
const diffId = (0, import_core3.getInput)("diff-id");
3590535905
const hashParam = commitHash ? `commitHash=${commitHash}` : `diffId=${diffId}`;
35906+
const useBaseImages = (0, import_core3.getBooleanInput)("use-base-images") ?? true;
3590635907
const { owner, repo } = import_github2.context.repo;
35907-
return `${comparadiseHost}/?${hashParam}&owner=${owner}&repo=${repo}&bucket=${bucketName}`;
35908+
return `${comparadiseHost}/?${hashParam}&owner=${owner}&repo=${repo}&bucket=${bucketName}&useBaseImages=${useBaseImages}`;
3590835909
};
3590935910

3591035911
// src/comment.ts
@@ -35996,8 +35997,8 @@ var run = async () => {
3599635997
}
3599735998
const hash = commitHash || diffId;
3599835999
const screenshotsDirectory = (0, import_core6.getInput)("screenshots-directory");
35999-
const downloadImages = (0, import_core6.getBooleanInput)("download-base-images") ?? true;
36000-
if (downloadImages) {
36000+
const useBaseImages = (0, import_core6.getBooleanInput)("use-base-images") ?? true;
36001+
if (useBaseImages) {
3600136002
await downloadBaseImages();
3600236003
}
3600336004
const visualTestExitCode = await Promise.all(

action/dist/main.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

action/src/build-comparadise-url.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getInput } from '@actions/core';
1+
import { getBooleanInput, getInput } from '@actions/core';
22
import { context } from '@actions/github';
33

44
export const buildComparadiseUrl = () => {
@@ -9,7 +9,8 @@ export const buildComparadiseUrl = () => {
99
const hashParam = commitHash
1010
? `commitHash=${commitHash}`
1111
: `diffId=${diffId}`;
12+
const useBaseImages = getBooleanInput('use-base-images') ?? true;
1213
const { owner, repo } = context.repo;
1314

14-
return `${comparadiseHost}/?${hashParam}&owner=${owner}&repo=${repo}&bucket=${bucketName}`;
15+
return `${comparadiseHost}/?${hashParam}&owner=${owner}&repo=${repo}&bucket=${bucketName}&useBaseImages=${useBaseImages}`;
1516
};

action/src/run.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ export const run = async () => {
4343

4444
const screenshotsDirectory = getInput('screenshots-directory');
4545

46-
const downloadImages = getBooleanInput('download-base-images') ?? true;
47-
if (downloadImages) {
46+
const useBaseImages = getBooleanInput('use-base-images') ?? true;
47+
if (useBaseImages) {
4848
await downloadBaseImages();
4949
}
5050

action/test/run.test.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ describe('main', () => {
205205
state: 'failure',
206206
description: 'A visual regression was detected. Check Comparadise!',
207207
target_url:
208-
'https://comparadise.app/?commitHash=sha&owner=owner&repo=repo&bucket=some-bucket'
208+
'https://comparadise.app/?commitHash=sha&owner=owner&repo=repo&bucket=some-bucket&useBaseImages=true'
209209
});
210210
expect(octokit.rest.issues.createComment).toHaveBeenCalled();
211211
});
@@ -412,7 +412,7 @@ describe('main', () => {
412412
assertNoOctokitCalls();
413413
});
414414

415-
it('should download base images if download-base-images specified as true', async () => {
415+
it('should download base images if use-base-images specified as true', async () => {
416416
const downloadBaseImages = true;
417417
(exec as jest.Mock).mockResolvedValue(0);
418418
(getInput as jest.Mock).mockImplementation(name => inputMap[name]);
@@ -428,7 +428,7 @@ describe('main', () => {
428428
);
429429
});
430430

431-
it('should not download base images if download-base-images specified as false', async () => {
431+
it('should not download base images if use-base-images specified as false and set URL param', async () => {
432432
const downloadBaseImages = false;
433433
(exec as jest.Mock).mockResolvedValue(0);
434434
(getInput as jest.Mock).mockImplementation(name => inputMap[name]);
@@ -442,6 +442,16 @@ describe('main', () => {
442442
expect(exec).not.toHaveBeenCalledWith(
443443
`aws s3 cp s3://some-bucket/${BASE_IMAGES_DIRECTORY} path/to/screenshots --recursive`
444444
);
445+
expect(octokit.rest.repos.createCommitStatus).toHaveBeenCalledWith({
446+
owner: 'owner',
447+
repo: 'repo',
448+
sha: 'sha',
449+
context: VISUAL_REGRESSION_CONTEXT,
450+
state: 'failure',
451+
description: 'A visual regression was detected. Check Comparadise!',
452+
target_url:
453+
'https://comparadise.app/?commitHash=sha&owner=owner&repo=repo&bucket=some-bucket&useBaseImages=false'
454+
});
445455
});
446456

447457
it('should not download base images if aws ls call fails', async () => {

app/backend/src/acceptVisualChanges.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { updateCommitStatus } from './updateCommitStatus';
1414
export const acceptVisualChanges = async ({
1515
commitHash,
1616
diffId,
17+
useBaseImages,
1718
bucket,
1819
owner,
1920
repo
@@ -35,8 +36,10 @@ export const acceptVisualChanges = async ({
3536
});
3637
}
3738

38-
const s3Paths = await getKeysFromS3(hash, bucket);
39-
await updateBaseImages(s3Paths, bucket);
39+
if (useBaseImages) {
40+
const s3Paths = await getKeysFromS3(hash, bucket);
41+
await updateBaseImages(s3Paths, bucket);
42+
}
4043
if (commitHash) {
4144
await updateCommitStatus({ owner, repo, commitHash });
4245
}

app/backend/src/schema.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export const fetchCurrentPageInputSchema = z.object({
88
export const acceptVisualChangesInputSchema = z.object({
99
commitHash: z.string().min(1).optional(),
1010
diffId: z.string().min(1).optional(),
11+
useBaseImages: z.boolean().optional().default(true),
1112
bucket: z.string().min(1),
1213
repo: z.string().min(1),
1314
owner: z.string().min(1)

app/backend/test/acceptVisualChanges.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ describe('acceptVisualChanges', () => {
9191
acceptVisualChanges({
9292
commitHash: '030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5',
9393
bucket: expectedBucket,
94+
useBaseImages: true,
9495
repo: 'repo',
9596
owner: 'owner'
9697
})
@@ -100,4 +101,22 @@ describe('acceptVisualChanges', () => {
100101
expect(S3Client.copyObject).not.toHaveBeenCalled();
101102
expect(updateCommitStatus).not.toHaveBeenCalled();
102103
});
104+
105+
it('should update commit status but not base images if useBaseImages is false', async () => {
106+
(findReasonToPreventVisualChangeAcceptance as jest.Mock).mockResolvedValue(
107+
undefined
108+
);
109+
const expectedBucket = 'expected-bucket-name';
110+
await acceptVisualChanges({
111+
commitHash: '030928b2c4b48ab4d3b57c8e0b0f7a56db768ef5',
112+
bucket: expectedBucket,
113+
useBaseImages: false,
114+
repo: 'repo',
115+
owner: 'owner'
116+
});
117+
118+
expect(S3Client.listObjectsV2).not.toHaveBeenCalled();
119+
expect(S3Client.copyObject).not.toHaveBeenCalled();
120+
expect(updateCommitStatus).toHaveBeenCalled();
121+
});
103122
});

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,14 @@ export const UpdateImagesButton: React.FC<{ disabled: boolean }> = ({
5050
setDialogIsOpen(false);
5151
};
5252

53+
const useBaseImages = params.useBaseImages
54+
? params.useBaseImages === 'true'
55+
: undefined;
5356
const handleUpdate = () =>
5457
acceptVisualChanges({
5558
commitHash,
5659
diffId,
60+
useBaseImages,
5761
bucket,
5862
owner,
5963
repo

0 commit comments

Comments
 (0)