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));