From c994809c148df4282c6eb297a7069a6147801d32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Beteg=C3=B3n?= Date: Mon, 10 Aug 2026 10:57:01 +0200 Subject: [PATCH] fix(snapshots): create all-unchanged selective uploads Selective CI uploads can legitimately have no changed image files while still providing the full suite name list. Build and post an empty manifest in that case, while preserving the ordinary empty-directory no-op and avoiding objectstore requests. --- packages/cli/src/commands/snapshots/upload.ts | 36 +++-- .../test/commands/snapshots/upload.test.ts | 131 +++++++++++++++++- 2 files changed, 150 insertions(+), 17 deletions(-) diff --git a/packages/cli/src/commands/snapshots/upload.ts b/packages/cli/src/commands/snapshots/upload.ts index 145be2cd2b..600ccfe531 100644 --- a/packages/cli/src/commands/snapshots/upload.ts +++ b/packages/cli/src/commands/snapshots/upload.ts @@ -68,7 +68,7 @@ type SnapshotUploadResult = { uploaded: number; /** Number of images skipped (already present in objectstore). */ skipped: number; - /** The created snapshot, or `null` when there were no images. */ + /** The created snapshot, or `null` when there was nothing to record. */ snapshot: CreateSnapshotResponse | null; }; @@ -97,7 +97,7 @@ function parsePrNumber(value: string): number { async function resolveAllImageNames( flags: UploadFlags ): Promise { - if (flags["all-image-file-names"]) { + if (flags["all-image-file-names"] !== undefined) { const names = normalizeImageNames( splitAndTrim(flags["all-image-file-names"], ",") ); @@ -109,7 +109,7 @@ async function resolveAllImageNames( } return names; } - if (flags["all-image-file-names-file"]) { + if (flags["all-image-file-names-file"] !== undefined) { const path = flags["all-image-file-names-file"]; let content: string; try { @@ -475,7 +475,10 @@ export const uploadCommand = buildCommand({ } const { org, project } = resolved; - if (flags["all-image-file-names"] && flags["all-image-file-names-file"]) { + if ( + flags["all-image-file-names"] !== undefined && + flags["all-image-file-names-file"] !== undefined + ) { throw new ValidationError( "--all-image-file-names and --all-image-file-names-file cannot be used together", "all-image-file-names" @@ -484,7 +487,11 @@ export const uploadCommand = buildCommand({ const vcs = collectVcs(flags, this.cwd, this.env); const images = await collectImages(dir); - if (images.length === 0) { + validateImageSizes(images); + + const { selective, allImageNames } = await resolveSelective(flags, images); + // A complete name list makes zero uploaded images a valid all-unchanged run. + if (images.length === 0 && allImageNames === undefined) { yield new CommandOutput({ imagesFound: 0, uploaded: 0, @@ -493,16 +500,17 @@ export const uploadCommand = buildCommand({ }); return { hint: "No image files found." }; } - validateImageSizes(images); - const { selective, allImageNames } = await resolveSelective(flags, images); - - log.info(`Uploading ${images.length} image(s)...`); - const { entries, uploaded, skipped } = await uploadImages( - org, - project, - images - ); + let uploadResult: UploadImagesResult = { + entries: {}, + uploaded: 0, + skipped: 0, + }; + if (images.length > 0) { + log.info(`Uploading ${images.length} image(s)...`); + uploadResult = await uploadImages(org, project, images); + } + const { entries, uploaded, skipped } = uploadResult; const manifest = buildManifest({ appId: flags["app-id"], diff --git a/packages/cli/test/commands/snapshots/upload.test.ts b/packages/cli/test/commands/snapshots/upload.test.ts index 64ff3c00f3..5fecc2d3d2 100644 --- a/packages/cli/test/commands/snapshots/upload.test.ts +++ b/packages/cli/test/commands/snapshots/upload.test.ts @@ -68,6 +68,7 @@ describe("snapshots upload", () => { let existsSpy: ReturnType; let putSpy: ReturnType; let createSpy: ReturnType; + let uploadOptionsSpy: ReturnType; beforeEach(async () => { tmpDir = await mkdtemp(join(tmpdir(), "snap-up-")); @@ -75,9 +76,9 @@ describe("snapshots upload", () => { org: "test-org", project: "test-project", }); - vi.spyOn(preprod, "fetchSnapshotsUploadOptions").mockResolvedValue( - UPLOAD_OPTIONS - ); + uploadOptionsSpy = vi + .spyOn(preprod, "fetchSnapshotsUploadOptions") + .mockResolvedValue(UPLOAD_OPTIONS); existsSpy = vi.spyOn(objectstore, "objectExists").mockResolvedValue(false); putSpy = vi.spyOn(objectstore, "putObject").mockResolvedValue(undefined); createSpy = vi.spyOn(preprod, "createPreprodSnapshot").mockResolvedValue({ @@ -215,6 +216,130 @@ describe("snapshots upload", () => { expect(harness.output()).toContain("No image files found"); }); + test("creates an all-unchanged snapshot from an inline image-name list", async () => { + const dir = join(tmpDir, "empty"); + await mkdir(dir); + createSpy.mockResolvedValue({ + artifactId: "snap-empty", + imageCount: 0, + snapshotUrl: "https://sentry.io/snap-empty", + }); + const harness = createContext(); + const func = await uploadCommand.loader(); + + await func.call( + harness.context, + { + "app-id": "app", + "all-image-file-names": "./a.png,sub\\b.jpg", + }, + dir + ); + + expect(uploadOptionsSpy).not.toHaveBeenCalled(); + expect(existsSpy).not.toHaveBeenCalled(); + expect(putSpy).not.toHaveBeenCalled(); + expect(createSpy).toHaveBeenCalledTimes(1); + const manifest = createSpy.mock.calls[0]?.[2]; + expect(manifest).toMatchObject({ + app_id: "app", + selective: true, + all_image_file_names: ["a.png", "sub/b.jpg"], + }); + expect(manifest?.images).toEqual({}); + expect(harness.output()).toContain("snap-empty"); + }); + + test("creates an all-unchanged snapshot from an image-name file", async () => { + const dir = join(tmpDir, "empty"); + const namesFile = join(tmpDir, "all-images.txt"); + await mkdir(dir); + await writeFile(namesFile, "a.png\nsub/b.png\n"); + createSpy.mockResolvedValue({ + artifactId: "snap-empty", + imageCount: 0, + snapshotUrl: null, + }); + const func = await uploadCommand.loader(); + + await func.call( + createContext().context, + { "app-id": "app", "all-image-file-names-file": namesFile }, + dir + ); + + expect(uploadOptionsSpy).not.toHaveBeenCalled(); + expect(createSpy).toHaveBeenCalledTimes(1); + const manifest = createSpy.mock.calls[0]?.[2]; + expect(manifest).toMatchObject({ + selective: true, + all_image_file_names: ["a.png", "sub/b.png"], + }); + expect(manifest?.images).toEqual({}); + }); + + test("keeps an empty --selective-only upload as a no-op", async () => { + const dir = join(tmpDir, "empty"); + await mkdir(dir); + const harness = createContext(); + const func = await uploadCommand.loader(); + + await func.call(harness.context, { "app-id": "app", selective: true }, dir); + + expect(createSpy).not.toHaveBeenCalled(); + expect(uploadOptionsSpy).not.toHaveBeenCalled(); + expect(harness.output()).toContain("No image files found"); + }); + + test("rejects an unreadable image-name file for an empty upload", async () => { + const dir = join(tmpDir, "empty"); + await mkdir(dir); + const func = await uploadCommand.loader(); + + await expect( + func.call( + createContext().context, + { + "app-id": "app", + "all-image-file-names-file": join(tmpDir, "missing.txt"), + }, + dir + ) + ).rejects.toThrow(ValidationError); + expect(createSpy).not.toHaveBeenCalled(); + }); + + test("rejects an empty image-name list for an empty upload", async () => { + const dir = join(tmpDir, "empty"); + const namesFile = join(tmpDir, "all-images.txt"); + await mkdir(dir); + await writeFile(namesFile, " \n\n"); + const func = await uploadCommand.loader(); + + await expect( + func.call( + createContext().context, + { "app-id": "app", "all-image-file-names-file": namesFile }, + dir + ) + ).rejects.toThrow(ValidationError); + expect(createSpy).not.toHaveBeenCalled(); + }); + + test.each([ + { "all-image-file-names": "" }, + { "all-image-file-names-file": "" }, + ])("rejects an explicitly empty image-name flag", async (flag) => { + const dir = join(tmpDir, "empty"); + await mkdir(dir); + const func = await uploadCommand.loader(); + + await expect( + func.call(createContext().context, { "app-id": "app", ...flag }, dir) + ).rejects.toThrow(ValidationError); + expect(createSpy).not.toHaveBeenCalled(); + }); + test("rejects a path that is not a directory", async () => { const file = join(tmpDir, "a.png"); await writeFile(file, pngBytes(1, 1));