[no-ticket] Do not create a build when there are no files to process

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

@@ -0,0 +1,5 @@
+---
+"@saucelabs/visual-snapshots": minor
+---
+
+Reasonable defaults for build name, suite name, test name, and snapshot name

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

@@ -3,8 +3,10 @@ import {
   VisualSnapshotsApi,
 } from "../api/visual-snapshots-api.js";
 import { VisualConfig } from "@saucelabs/visual";
-import { getFiles } from "../utils/glob.js";
 import { PdfSnapshotUploader } from "./pdf-files-snapshot-uploader.js";
+import { logger as defaultLogger } from "../logger.js";
+import { FileExtractor } from "../utils/glob.js";
+import { Logger } from "pino";
 
 export interface PdfCommandParams
   extends VisualConfig,
@@ -15,14 +17,30 @@ export interface PdfCommandParams
 export class PdfCommandHandler {
   constructor(
     private readonly visualSnapshotsApi: VisualSnapshotsApi,
-    private readonly pdfSnapshotUploader: PdfSnapshotUploader
+    private readonly pdfSnapshotUploader: PdfSnapshotUploader,
+    private readonly fileExtractor: FileExtractor,
+    private readonly logger: Logger = defaultLogger
   ) {}
 
   public async handle(
     globsOrDirs: string[],
     params: PdfCommandParams
   ): Promise<void> {
-    const pdfFilePaths = await getFiles(globsOrDirs, "*.pdf");
+    const pdfFilePaths = await this.fileExtractor.getFiles(
+      globsOrDirs,
+      "*.pdf"
+    );
+
+    if (pdfFilePaths.length === 0) {
+      this.logger.warn("No PDF files found to process.");
+      return;
+    }
+
+    this.logger.info(
+      `Found ${pdfFilePaths.length} PDF file${
+        pdfFilePaths.length > 1 ? "s" : ""
+      } to process: ${pdfFilePaths.join(", ")}`
+    );
 
     const buildId =
       params.buildId ?? (await this.visualSnapshotsApi.createBuild(params));

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

@@ -20,6 +20,7 @@ import { initializeVisualApi } from "../api/visual-client.js";
 import { WorkerPoolPdfSnapshotUploader } from "../app/worker/worker-pool-pdf-snapshot-uploader.js";
 import { LibPdfFileLoader } from "../app/pdf-file-loader.js";
 import { logger } from "../logger.js";
+import { GlobFileExtractor } from "../utils/glob.js";
 
 export const pdfCommand = (clientVersion: string) => {
   return new Command()
@@ -52,12 +53,14 @@ export const pdfCommand = (clientVersion: string) => {
           maxWorkers: params.concurrency,
         }
       );
+      const globFileExtractor = new GlobFileExtractor();
 
-      new PdfCommandHandler(visualSnapshotsApi, pdfSnapshotUploader)
+      new PdfCommandHandler(
+        visualSnapshotsApi,
+        pdfSnapshotUploader,
+        globFileExtractor
+      )
         .handle(globsOrDirs, params)
-        .then(() => {
-          logger.info("Successfully created PDF snapshots.");
-        })
         .catch((err) => {
           logger.error(err, "At least one PDF snapshot creation failed.");
         });

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

@@ -2,27 +2,35 @@ import { glob } from "glob";
 import fs from "fs/promises";
 import path from "path";
 
-/**
- * Returns all files matched by globs, or if path is a directory, matched by `dirGlob`.
- * @param globOrDirs Globs or dirs to get files from.
- * @param dirGlob Glob to append to directory path.
- * @returns Matched files.
- */
-export async function getFiles(globOrDirs: string[], dirGlob: string) {
-  const globs = await Promise.all(
-    globOrDirs.map((g) =>
-      isDirectory(g).then((result) => (result ? path.join(g, dirGlob) : g))
-    )
-  );
-
-  return await glob(globs);
+export interface FileExtractor {
+  getFiles(globOrDirs: string[], dirGlob: string): Promise<string[]>;
 }
 
-async function isDirectory(path: string) {
-  try {
-    const stat = await fs.stat(path);
-    return stat.isDirectory();
-  } catch {
-    return false;
+export class GlobFileExtractor implements FileExtractor {
+  /**
+   * Returns all files matched by globs, or if path is a directory, matched by `dirGlob`.
+   * @param globOrDirs Globs or dirs to get files from.
+   * @param dirGlob Glob to append to directory path.
+   * @returns Matched files.
+   */
+  public async getFiles(globOrDirs: string[], dirGlob: string) {
+    const globs = await Promise.all(
+      globOrDirs.map((g) =>
+        this.isDirectory(g).then((result) =>
+          result ? path.join(g, dirGlob) : g
+        )
+      )
+    );
+
+    return await glob(globs);
+  }
+
+  private async isDirectory(path: string) {
+    try {
+      const stat = await fs.stat(path);
+      return stat.isDirectory();
+    } catch {
+      return false;
+    }
   }
 }

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

@@ -4,7 +4,7 @@ import {
   UploadSnapshotParams,
   VisualSnapshotsApi,
 } from "../../src/api/visual-snapshots-api.js";
-import { mockLogger } from "../helpers.js";
+import { mockLogger } from "../mock-logger.js";
 
 describe("VisualSnapshots", () => {
   const { logger, logged, reset: resetLogger } = mockLogger();

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

@@ -17,10 +17,6 @@ function createUploadId(content: Buffer) {
 
 describe("PdfPageSnapshotUploader", () => {
   describe("uploadPageSnapshot", () => {
-    const consoleInfoSpy = jest
-      .spyOn(console, "info")
-      .mockImplementation(() => undefined);
-
     const uploadImageAndCreateSnapshot = jest.fn<
       ReturnType<VisualSnapshotsApi["uploadImageAndCreateSnapshot"]>,
       Parameters<VisualSnapshotsApi["uploadImageAndCreateSnapshot"]>
@@ -59,8 +55,6 @@ describe("PdfPageSnapshotUploader", () => {
       uploadImageAndCreateSnapshot.mockImplementation(({ snapshot }) =>
         Promise.resolve(createUploadId(snapshot))
       );
-
-      consoleInfoSpy.mockReset();
     });
 
     it("should call uploadImageAndCreateSnapshot", async () => {

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

@@ -1,5 +1,5 @@
 import { LibPdfFileLoader } from "../../src/app/pdf-file-loader.js";
-import { __dirname } from "../helpers.js";
+import { __dirname } from "../system-helpers.js";
 
 describe("LibPdfFileLoader", () => {
   it("should call library with path and scale", async () => {

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

@@ -4,7 +4,9 @@ import {
   PdfCommandHandler,
   PdfCommandParams,
 } from "../../src/app/pdf-handler.js";
-import { __dirname } from "../helpers.js";
+import { MockFileExtractor } from "../mock-file-extractor.js";
+import { mockLogger } from "../mock-logger.js";
+import { __dirname } from "../system-helpers.js";
 import path from "path";
 
 describe("pdf-handler", () => {
@@ -36,15 +38,28 @@ describe("pdf-handler", () => {
   const snapshotName = "snapshotName";
   const buildId = "buildId";
 
+  const mockFileExtractor = new MockFileExtractor();
+
+  const { logger, logged, reset: resetLogger } = mockLogger();
+
   beforeEach(() => {
     jest.resetAllMocks();
+    resetLogger();
+    mockFileExtractor.reset();
   });
 
   describe("creating build", () => {
     it("should create a build when buildId is not passed", async () => {
+      mockFileExtractor.setFilesToReturn([
+        "/absolute/path/to/files/1.pdf",
+        "/absolute/path/to/files/2.pdf",
+      ]);
+
       const handler = new PdfCommandHandler(
         visualSnapshotsApi,
-        pdfSnapshotUploaderMock
+        pdfSnapshotUploaderMock,
+        mockFileExtractor,
+        logger
       );
 
       const params: PdfCommandParams = {
@@ -55,17 +70,71 @@ describe("pdf-handler", () => {
       };
 
       await handler.handle(
-        [path.join(__dirname(import.meta), "../files/1.pdf")],
+        [path.join(__dirname(import.meta), "../files/")],
         params
       );
 
       expect(createBuildMock).toHaveBeenCalledWith(params);
+      expect(mockFileExtractor.calls()).toEqual([
+        {
+          globOrDirs: [path.join(__dirname(import.meta), "../files/")],
+          dirGlob: "*.pdf",
+        },
+      ]);
+      expect(logged).toEqual([
+        {
+          level: 30,
+          msg: "Found 2 PDF files to process: /absolute/path/to/files/1.pdf, /absolute/path/to/files/2.pdf",
+        },
+      ]);
     });
 
     it("should not create a build when buildId is passed", async () => {
+      mockFileExtractor.setFilesToReturn(["/absolute/path/to/files/1.pdf"]);
+
+      const handler = new PdfCommandHandler(
+        visualSnapshotsApi,
+        pdfSnapshotUploaderMock,
+        mockFileExtractor,
+        logger
+      );
+
+      const params: PdfCommandParams = {
+        suiteName,
+        testName,
+        snapshotName,
+        buildId,
+        concurrency: 1,
+      };
+
+      await handler.handle(
+        [path.join(__dirname(import.meta), "../files/1.pdf")],
+        params
+      );
+
+      expect(createBuildMock).not.toHaveBeenCalled();
+      expect(mockFileExtractor.calls()).toEqual([
+        {
+          globOrDirs: [path.join(__dirname(import.meta), "../files/1.pdf")],
+          dirGlob: "*.pdf",
+        },
+      ]);
+      expect(logged).toEqual([
+        {
+          level: 30,
+          msg: "Found 1 PDF file to process: /absolute/path/to/files/1.pdf",
+        },
+      ]);
+    });
+
+    it("should not create a build when there are no files to process", async () => {
+      mockFileExtractor.setFilesToReturn([]);
+
       const handler = new PdfCommandHandler(
         visualSnapshotsApi,
-        pdfSnapshotUploaderMock
+        pdfSnapshotUploaderMock,
+        mockFileExtractor,
+        logger
       );
 
       const params: PdfCommandParams = {
@@ -82,14 +151,27 @@ describe("pdf-handler", () => {
       );
 
       expect(createBuildMock).not.toHaveBeenCalled();
+      expect(mockFileExtractor.calls()).toEqual([
+        {
+          globOrDirs: [path.join(__dirname(import.meta), "../files/1.pdf")],
+          dirGlob: "*.pdf",
+        },
+      ]);
+      expect(logged).toEqual([
+        { level: 40, msg: "No PDF files found to process." },
+      ]);
     });
   });
 
   describe("uploading snapshots", () => {
     it("should call uploadSnapshots with created build ID", async () => {
+      mockFileExtractor.setFilesToReturn(["/absolute/path/to/files/1.pdf"]);
+
       const handler = new PdfCommandHandler(
         visualSnapshotsApi,
-        pdfSnapshotUploaderMock
+        pdfSnapshotUploaderMock,
+        mockFileExtractor,
+        logger
       );
 
       const params: PdfCommandParams = {
@@ -117,9 +199,13 @@ describe("pdf-handler", () => {
     });
 
     it("should call uploadSnapshots with provided build ID", async () => {
+      mockFileExtractor.setFilesToReturn(["/absolute/path/to/files/1.pdf"]);
+
       const handler = new PdfCommandHandler(
         visualSnapshotsApi,
-        pdfSnapshotUploaderMock
+        pdfSnapshotUploaderMock,
+        mockFileExtractor,
+        logger
       );
 
       const params: PdfCommandParams = {
@@ -150,9 +236,13 @@ describe("pdf-handler", () => {
     it("should finish build when buildId is not passed, using created build ID", async () => {
       createBuildMock.mockResolvedValue(buildId);
 
+      mockFileExtractor.setFilesToReturn(["/absolute/path/to/files/1.pdf"]);
+
       const handler = new PdfCommandHandler(
         visualSnapshotsApi,
-        pdfSnapshotUploaderMock
+        pdfSnapshotUploaderMock,
+        mockFileExtractor,
+        logger
       );
 
       const params: PdfCommandParams = {
@@ -171,9 +261,13 @@ describe("pdf-handler", () => {
     });
 
     it("should not finish build when buildId is passed", async () => {
+      mockFileExtractor.setFilesToReturn(["/absolute/path/to/files/1.pdf"]);
+
       const handler = new PdfCommandHandler(
         visualSnapshotsApi,
-        pdfSnapshotUploaderMock
+        pdfSnapshotUploaderMock,
+        mockFileExtractor,
+        logger
       );
 
       const params: PdfCommandParams = {

visual-js/visual-snapshots/test/mock-file-extractor.ts

@@ -0,0 +1,24 @@
+import { FileExtractor } from "../src/utils/glob.js";
+
+export class MockFileExtractor implements FileExtractor {
+  private files: string[] = [];
+  private calledWith: { globOrDirs: string[]; dirGlob: string }[] = [];
+
+  public setFilesToReturn(files: string[]) {
+    this.files = files;
+  }
+
+  public async getFiles(globOrDirs: string[], dirGlob: string) {
+    this.calledWith.push({ globOrDirs, dirGlob });
+    return this.files;
+  }
+
+  public reset() {
+    this.files = [];
+    this.calledWith = [];
+  }
+
+  public calls() {
+    return this.calledWith;
+  }
+}