Skip to content

Commit cafc4f5

Browse files
aga-szczepanskaaga-szczepanska
authored
[no-ticket] Do not create a build when there are no files to process (#226)
1 parent 3528ea0 commit cafc4f5

15 files changed

+257
-69
lines changed

‎visual-js/.changeset/forty-bees-sin.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@saucelabs/visual-snapshots": minor
3+
---
4+
5+
Reasonable defaults for build name, suite name, test name, and snapshot name

‎visual-js/visual-snapshots/src/app/pdf-handler.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ import {
33
VisualSnapshotsApi,
44
} from "../api/visual-snapshots-api.js";
55
import { VisualConfig } from "@saucelabs/visual";
6-
import { getFiles } from "../utils/glob.js";
76
import { PdfSnapshotUploader } from "./pdf-files-snapshot-uploader.js";
7+
import { logger as defaultLogger } from "../logger.js";
8+
import { FileExtractor } from "../utils/glob.js";
9+
import { Logger } from "pino";
810

911
export interface PdfCommandParams
1012
extends VisualConfig,
@@ -15,14 +17,32 @@ export interface PdfCommandParams
1517
export class PdfCommandHandler {
1618
constructor(
1719
private readonly visualSnapshotsApi: VisualSnapshotsApi,
18-
private readonly pdfSnapshotUploader: PdfSnapshotUploader
20+
private readonly pdfSnapshotUploader: PdfSnapshotUploader,
21+
private readonly fileExtractor: FileExtractor,
22+
private readonly logger: Logger = defaultLogger
1923
) {}
2024

2125
public async handle(
2226
globsOrDirs: string[],
2327
params: PdfCommandParams
2428
): Promise<void> {
25-
const pdfFilePaths = await getFiles(globsOrDirs, "*.pdf");
29+
const pdfFilePaths = await this.fileExtractor.getFiles(
30+
globsOrDirs,
31+
"*.pdf"
32+
);
33+
34+
if (pdfFilePaths.length === 0) {
35+
this.logger.warn("No PDF files found to process.");
36+
return;
37+
}
38+
39+
this.logger.info(
40+
{
41+
count: pdfFilePaths.length,
42+
paths: pdfFilePaths,
43+
},
44+
`Found PDF file${pdfFilePaths.length > 1 ? "s" : ""} to process`
45+
);
2646

2747
const buildId =
2848
params.buildId ?? (await this.visualSnapshotsApi.createBuild(params));

‎visual-js/visual-snapshots/src/commands/pdf.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { initializeVisualApi } from "../api/visual-client.js";
2020
import { WorkerPoolPdfSnapshotUploader } from "../app/worker/worker-pool-pdf-snapshot-uploader.js";
2121
import { LibPdfFileLoader } from "../app/pdf-file-loader.js";
2222
import { logger } from "../logger.js";
23+
import { GlobFileExtractor } from "../utils/glob.js";
2324

2425
export const pdfCommand = (clientVersion: string) => {
2526
return new Command()
@@ -52,12 +53,14 @@ export const pdfCommand = (clientVersion: string) => {
5253
maxWorkers: params.concurrency,
5354
}
5455
);
56+
const globFileExtractor = new GlobFileExtractor();
5557

56-
new PdfCommandHandler(visualSnapshotsApi, pdfSnapshotUploader)
58+
new PdfCommandHandler(
59+
visualSnapshotsApi,
60+
pdfSnapshotUploader,
61+
globFileExtractor
62+
)
5763
.handle(globsOrDirs, params)
58-
.then(() => {
59-
logger.info("Successfully created PDF snapshots.");
60-
})
6164
.catch((err) => {
6265
logger.error(err, "At least one PDF snapshot creation failed.");
6366
});

‎visual-js/visual-snapshots/src/utils/glob.ts

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,35 @@ import { glob } from "glob";
22
import fs from "fs/promises";
33
import path from "path";
44

5-
/**
6-
* Returns all files matched by globs, or if path is a directory, matched by `dirGlob`.
7-
* @param globOrDirs Globs or dirs to get files from.
8-
* @param dirGlob Glob to append to directory path.
9-
* @returns Matched files.
10-
*/
11-
export async function getFiles(globOrDirs: string[], dirGlob: string) {
12-
const globs = await Promise.all(
13-
globOrDirs.map((g) =>
14-
isDirectory(g).then((result) => (result ? path.join(g, dirGlob) : g))
15-
)
16-
);
17-
18-
return await glob(globs);
5+
export interface FileExtractor {
6+
getFiles(globOrDirs: string[], dirGlob: string): Promise<string[]>;
197
}
208

21-
async function isDirectory(path: string) {
22-
try {
23-
const stat = await fs.stat(path);
24-
return stat.isDirectory();
25-
} catch {
26-
return false;
9+
export class GlobFileExtractor implements FileExtractor {
10+
/**
11+
* Returns all files matched by globs, or if path is a directory, matched by `dirGlob`.
12+
* @param globOrDirs Globs or dirs to get files from.
13+
* @param dirGlob Glob to append to directory path.
14+
* @returns Matched files.
15+
*/
16+
public async getFiles(globOrDirs: string[], dirGlob: string) {
17+
const globs = await Promise.all(
18+
globOrDirs.map((g) =>
19+
this.isDirectory(g).then((result) =>
20+
result ? path.join(g, dirGlob) : g
21+
)
22+
)
23+
);
24+
25+
return await glob(globs);
26+
}
27+
28+
private async isDirectory(path: string) {
29+
try {
30+
const stat = await fs.stat(path);
31+
return stat.isDirectory();
32+
} catch {
33+
return false;
34+
}
2735
}
2836
}

‎visual-js/visual-snapshots/test/api/visual-api.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {
44
UploadSnapshotParams,
55
VisualSnapshotsApi,
66
} from "../../src/api/visual-snapshots-api.js";
7-
import { mockLogger } from "../helpers.js";
7+
import { mockLogger } from "../mock-logger.js";
88

99
describe("VisualSnapshots", () => {
1010
const { logger, logged, reset: resetLogger } = mockLogger();

‎visual-js/visual-snapshots/test/api/worker/pdf-page-snapshot-uploader.spec.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@ function createUploadId(content: Buffer) {
1717

1818
describe("PdfPageSnapshotUploader", () => {
1919
describe("uploadPageSnapshot", () => {
20-
const consoleInfoSpy = jest
21-
.spyOn(console, "info")
22-
.mockImplementation(() => undefined);
23-
2420
const uploadImageAndCreateSnapshot = jest.fn<
2521
ReturnType<VisualSnapshotsApi["uploadImageAndCreateSnapshot"]>,
2622
Parameters<VisualSnapshotsApi["uploadImageAndCreateSnapshot"]>
@@ -59,8 +55,6 @@ describe("PdfPageSnapshotUploader", () => {
5955
uploadImageAndCreateSnapshot.mockImplementation(({ snapshot }) =>
6056
Promise.resolve(createUploadId(snapshot))
6157
);
62-
63-
consoleInfoSpy.mockReset();
6458
});
6559

6660
it("should call uploadImageAndCreateSnapshot", async () => {

‎visual-js/visual-snapshots/test/app/__snapshots__/pdf-handler.spec.ts.snap

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`pdf-handler creating build should create a build when buildId is not passed 1`] = `
4+
[
5+
{
6+
"count": 2,
7+
"level": 30,
8+
"msg": "Found PDF files to process",
9+
"paths": [
10+
"/absolute/path/to/files/1.pdf",
11+
"/absolute/path/to/files/2.pdf",
12+
],
13+
},
14+
]
15+
`;
16+
17+
exports[`pdf-handler creating build should not create a build when buildId is passed 1`] = `
18+
[
19+
{
20+
"count": 1,
21+
"level": 30,
22+
"msg": "Found PDF file to process",
23+
"paths": [
24+
"/absolute/path/to/files/1.pdf",
25+
],
26+
},
27+
]
28+
`;
29+
30+
exports[`pdf-handler creating build should not create a build when there are no files to process 1`] = `
31+
[
32+
{
33+
"level": 40,
34+
"msg": "No PDF files found to process.",
35+
},
36+
]
37+
`;

‎visual-js/visual-snapshots/test/app/pdf-file-loader.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { LibPdfFileLoader } from "../../src/app/pdf-file-loader.js";
2-
import { __dirname } from "../helpers.js";
2+
import { __dirname } from "../system-helpers.js";
33

44
describe("LibPdfFileLoader", () => {
55
it("should call library with path and scale", async () => {

‎visual-js/visual-snapshots/test/app/pdf-handler.spec.ts

Lines changed: 90 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ import {
44
PdfCommandHandler,
55
PdfCommandParams,
66
} from "../../src/app/pdf-handler.js";
7-
import { __dirname } from "../helpers.js";
7+
import { MockFileExtractor } from "../mock-file-extractor.js";
8+
import { mockLogger } from "../mock-logger.js";
9+
import { __dirname } from "../system-helpers.js";
810
import path from "path";
911

1012
describe("pdf-handler", () => {
@@ -36,15 +38,28 @@ describe("pdf-handler", () => {
3638
const snapshotName = "snapshotName";
3739
const buildId = "buildId";
3840

41+
const mockFileExtractor = new MockFileExtractor();
42+
43+
const { logger, logged, reset: resetLogger } = mockLogger();
44+
3945
beforeEach(() => {
4046
jest.resetAllMocks();
47+
resetLogger();
48+
mockFileExtractor.reset();
4149
});
4250

4351
describe("creating build", () => {
4452
it("should create a build when buildId is not passed", async () => {
53+
mockFileExtractor.setFilesToReturn([
54+
"/absolute/path/to/files/1.pdf",
55+
"/absolute/path/to/files/2.pdf",
56+
]);
57+
4558
const handler = new PdfCommandHandler(
4659
visualSnapshotsApi,
47-
pdfSnapshotUploaderMock
60+
pdfSnapshotUploaderMock,
61+
mockFileExtractor,
62+
logger
4863
);
4964

5065
const params: PdfCommandParams = {
@@ -55,17 +70,61 @@ describe("pdf-handler", () => {
5570
};
5671

5772
await handler.handle(
58-
[path.join(__dirname(import.meta), "../files/1.pdf")],
73+
[path.join(__dirname(import.meta), "../files/")],
5974
params
6075
);
6176

6277
expect(createBuildMock).toHaveBeenCalledWith(params);
78+
expect(mockFileExtractor.calls()).toEqual([
79+
{
80+
globOrDirs: [path.join(__dirname(import.meta), "../files/")],
81+
dirGlob: "*.pdf",
82+
},
83+
]);
84+
expect(logged).toMatchSnapshot();
6385
});
6486

6587
it("should not create a build when buildId is passed", async () => {
88+
mockFileExtractor.setFilesToReturn(["/absolute/path/to/files/1.pdf"]);
89+
90+
const handler = new PdfCommandHandler(
91+
visualSnapshotsApi,
92+
pdfSnapshotUploaderMock,
93+
mockFileExtractor,
94+
logger
95+
);
96+
97+
const params: PdfCommandParams = {
98+
suiteName,
99+
testName,
100+
snapshotName,
101+
buildId,
102+
concurrency: 1,
103+
};
104+
105+
await handler.handle(
106+
[path.join(__dirname(import.meta), "../files/1.pdf")],
107+
params
108+
);
109+
110+
expect(createBuildMock).not.toHaveBeenCalled();
111+
expect(mockFileExtractor.calls()).toEqual([
112+
{
113+
globOrDirs: [path.join(__dirname(import.meta), "../files/1.pdf")],
114+
dirGlob: "*.pdf",
115+
},
116+
]);
117+
expect(logged).toMatchSnapshot();
118+
});
119+
120+
it("should not create a build when there are no files to process", async () => {
121+
mockFileExtractor.setFilesToReturn([]);
122+
66123
const handler = new PdfCommandHandler(
67124
visualSnapshotsApi,
68-
pdfSnapshotUploaderMock
125+
pdfSnapshotUploaderMock,
126+
mockFileExtractor,
127+
logger
69128
);
70129

71130
const params: PdfCommandParams = {
@@ -82,14 +141,25 @@ describe("pdf-handler", () => {
82141
);
83142

84143
expect(createBuildMock).not.toHaveBeenCalled();
144+
expect(mockFileExtractor.calls()).toEqual([
145+
{
146+
globOrDirs: [path.join(__dirname(import.meta), "../files/1.pdf")],
147+
dirGlob: "*.pdf",
148+
},
149+
]);
150+
expect(logged).toMatchSnapshot();
85151
});
86152
});
87153

88154
describe("uploading snapshots", () => {
89155
it("should call uploadSnapshots with created build ID", async () => {
156+
mockFileExtractor.setFilesToReturn(["/absolute/path/to/files/1.pdf"]);
157+
90158
const handler = new PdfCommandHandler(
91159
visualSnapshotsApi,
92-
pdfSnapshotUploaderMock
160+
pdfSnapshotUploaderMock,
161+
mockFileExtractor,
162+
logger
93163
);
94164

95165
const params: PdfCommandParams = {
@@ -117,9 +187,13 @@ describe("pdf-handler", () => {
117187
});
118188

119189
it("should call uploadSnapshots with provided build ID", async () => {
190+
mockFileExtractor.setFilesToReturn(["/absolute/path/to/files/1.pdf"]);
191+
120192
const handler = new PdfCommandHandler(
121193
visualSnapshotsApi,
122-
pdfSnapshotUploaderMock
194+
pdfSnapshotUploaderMock,
195+
mockFileExtractor,
196+
logger
123197
);
124198

125199
const params: PdfCommandParams = {
@@ -150,9 +224,13 @@ describe("pdf-handler", () => {
150224
it("should finish build when buildId is not passed, using created build ID", async () => {
151225
createBuildMock.mockResolvedValue(buildId);
152226

227+
mockFileExtractor.setFilesToReturn(["/absolute/path/to/files/1.pdf"]);
228+
153229
const handler = new PdfCommandHandler(
154230
visualSnapshotsApi,
155-
pdfSnapshotUploaderMock
231+
pdfSnapshotUploaderMock,
232+
mockFileExtractor,
233+
logger
156234
);
157235

158236
const params: PdfCommandParams = {
@@ -171,9 +249,13 @@ describe("pdf-handler", () => {
171249
});
172250

173251
it("should not finish build when buildId is passed", async () => {
252+
mockFileExtractor.setFilesToReturn(["/absolute/path/to/files/1.pdf"]);
253+
174254
const handler = new PdfCommandHandler(
175255
visualSnapshotsApi,
176-
pdfSnapshotUploaderMock
256+
pdfSnapshotUploaderMock,
257+
mockFileExtractor,
258+
logger
177259
);
178260

179261
const params: PdfCommandParams = {

0 commit comments

Comments
 (0)