diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d4d3dfc..9329627 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -23,3 +23,31 @@ jobs: - run: pnpm run lint - run: pnpm run build - run: pnpm test + + # Started as a step, not a service: `services:` cannot pass a command, and the + # minio image needs `server /data`. Without this the integration spec skips, and + # skip-by-default plus never-in-CI means it rots — which matters because mocked + # tests cannot catch an SDK behaviour change. + - name: Start MinIO + run: | + docker run -d --name ci-minio -p 9000:9000 \ + -e MINIO_ROOT_USER=minioadmin \ + -e MINIO_ROOT_PASSWORD=minioadmin \ + minio/minio:latest server /data + for i in $(seq 1 30); do + if curl -sf http://127.0.0.1:9000/minio/health/live; then + echo "minio ready"; exit 0 + fi + sleep 2 + done + echo "minio did not become healthy" >&2 + docker logs ci-minio >&2 + exit 1 + + - run: pnpm run test:integration + env: + MINIO_TEST_ENDPOINT: http://127.0.0.1:9000 + + - name: Stop MinIO + if: always() + run: docker rm -f ci-minio || true diff --git a/src/file-store.azure.spec.ts b/src/file-store.azure.spec.ts index 873a79a..5667670 100644 --- a/src/file-store.azure.spec.ts +++ b/src/file-store.azure.spec.ts @@ -10,6 +10,7 @@ describe("AzureFileStore", () => { let fastify: ReturnType; let mockBlobClient: any; let mockContainerClient: any; + let mockGetContainerClient: jest.Mock; let store: FileStore; beforeEach(async () => { @@ -30,8 +31,9 @@ describe("AzureFileStore", () => { getBlockBlobClient: jest.fn(() => mockBlobClient), }; + mockGetContainerClient = jest.fn(() => mockContainerClient); (BlobServiceClient as unknown as jest.Mock).mockImplementation(() => ({ - getContainerClient: jest.fn(() => mockContainerClient), + getContainerClient: mockGetContainerClient, })); process.env.AZURE_STORAGE_ACCOUNT_URL = @@ -58,6 +60,12 @@ describe("AzureFileStore", () => { expect(store).toBeDefined(); }); + // Nothing asserted the container name, so every operation could target the wrong + // container and the suite would stay green. + it("opens the container named by AZURE_STORAGE_CONTAINER", () => { + expect(mockGetContainerClient).toHaveBeenCalledWith("test-container"); + }); + it("should throw when AZURE_STORAGE_ACCOUNT_URL is missing", async () => { delete process.env.AZURE_STORAGE_ACCOUNT_URL; const f = Fastify(); @@ -102,6 +110,27 @@ describe("AzureFileStore", () => { ); }); + // Without this, changing the utf8 encoding to ascii mangles every non-ASCII byte + // and the suite stays green — all other payload fixtures are ASCII. + it("encodes a unicode payload as utf8", async () => { + mockBlobClient.uploadData.mockResolvedValueOnce({}); + + await store.save("u.txt", "text/plain", "héllo→世界"); + + expect(mockBlobClient.uploadData).toHaveBeenCalledWith( + Buffer.from("héllo→世界", "utf8"), + { blobHTTPHeaders: { blobContentType: "text/plain" } }, + ); + }); + + it("propagates a rejected upload, not just an errorCode", async () => { + mockBlobClient.uploadData.mockRejectedValueOnce(new Error("AuthFailure")); + + await expect(store.save("a.txt", "text/plain", "x")).rejects.toThrow( + "AuthFailure", + ); + }); + it("should pass a Buffer payload through untouched", async () => { mockBlobClient.uploadData.mockResolvedValueOnce({}); const data = Buffer.from([1, 2, 3]); @@ -140,6 +169,24 @@ describe("AzureFileStore", () => { }); }); + describe("write failures", () => { + it("propagates a rejected uploadFile", async () => { + mockBlobClient.uploadFile.mockRejectedValueOnce(new Error("io error")); + + await expect( + store.copyFromLocalFile("a.txt", "text/plain", "/tmp/x"), + ).rejects.toThrow("io error"); + }); + + it("propagates a rejected uploadStream", async () => { + mockBlobClient.uploadStream.mockRejectedValueOnce(new Error("aborted")); + + await expect( + store.copyFromStream("a.txt", "text/plain", Readable.from(["x"])), + ).rejects.toThrow("aborted"); + }); + }); + describe("copyFromLocalFile", () => { it("should upload the local file", async () => { mockBlobClient.uploadFile.mockResolvedValueOnce({}); diff --git a/src/file-store.gcs.spec.ts b/src/file-store.gcs.spec.ts index 7a48193..7af6b1f 100644 --- a/src/file-store.gcs.spec.ts +++ b/src/file-store.gcs.spec.ts @@ -88,6 +88,24 @@ describe("GCPFileStore", () => { contentType: "text/plain", }); }); + + it("propagates a failed save", async () => { + mockFile.save.mockRejectedValueOnce(new Error("quota exceeded")); + + await expect( + store.save("test.txt", "text/plain", "content"), + ).rejects.toThrow("quota exceeded"); + }); + + it("round-trips a unicode payload unchanged", async () => { + mockFile.save.mockResolvedValueOnce(undefined); + + await store.save("u.txt", "text/plain", "héllo→世界"); + + expect(mockFile.save).toHaveBeenCalledWith("héllo→世界", { + contentType: "text/plain", + }); + }); }); describe("getAsBuffer", () => { @@ -113,6 +131,14 @@ describe("GCPFileStore", () => { destination: "dest.txt", }); }); + + it("propagates a failed upload", async () => { + mockBucket.upload.mockRejectedValueOnce(new Error("upload failed")); + + await expect( + store.copyFromLocalFile("dest.txt", "text/plain", "/tmp/src.txt"), + ).rejects.toThrow("upload failed"); + }); }); describe("getAsStream", () => { @@ -159,6 +185,19 @@ describe("GCPFileStore", () => { }); expect(Buffer.concat(chunks).toString()).toBe("piped content"); }); + + it("propagates a write-stream failure", async () => { + const ws = new Writable({ + write(_chunk, _enc, cb) { + cb(new Error("disk full")); + }, + }); + mockFile.createWriteStream.mockReturnValueOnce(ws); + + await expect( + store.copyFromStream("test.txt", "text/plain", Readable.from(["x"])), + ).rejects.toThrow("disk full"); + }); }); describe("getInfo", () => { diff --git a/src/file-store.local.spec.ts b/src/file-store.local.spec.ts index afec7ac..7bf0318 100644 --- a/src/file-store.local.spec.ts +++ b/src/file-store.local.spec.ts @@ -227,6 +227,19 @@ describe("LocalFileStore", () => { await fs.promises.readFile(path.join(tempDir, "x/y/z.txt"), "utf8"), ).toBe("piped"); }); + + it("propagates a source stream error", async () => { + const store = await register(); + const bad = new Readable({ + read() { + this.destroy(new Error("source failed")); + }, + }); + + await expect( + store.copyFromStream("bad.txt", "text/plain", bad), + ).rejects.toThrow("source failed"); + }); }); }); diff --git a/src/file-store.minio.integration.spec.ts b/src/file-store.minio.integration.spec.ts index bd05aa8..2f86c0a 100644 --- a/src/file-store.minio.integration.spec.ts +++ b/src/file-store.minio.integration.spec.ts @@ -64,6 +64,26 @@ describeIf("MinIO integration", () => { expect(await store.getInfo(`missing-${Date.now()}.txt`)).toBeNull(); }); + // The real GetObject 404 path. Mocks cannot catch a change in this error shape — + // the NoSuchKey/NotFound mismatch fixed in #9 was invisible to them. + it("rejects getAsBuffer for a missing key", async () => { + await expect( + store.getAsBuffer(`missing-${Date.now()}.txt`), + ).rejects.toBeDefined(); + }); + + it("rejects getAsStream for a missing key", async () => { + await expect( + store.getAsStream(`missing-${Date.now()}.txt`), + ).rejects.toBeDefined(); + }); + + it("round-trips a unicode payload and key", async () => { + const key = `unicode-héllo-世界-${Date.now()}.txt`; + await store.save(key, "text/plain", "héllo→世界"); + expect((await store.getAsBuffer(key)).toString()).toBe("héllo→世界"); + }); + it("round-trips a string payload", async () => { const key = `round-trip-${Date.now()}.txt`; await store.save(key, "text/plain", "hello minio"); diff --git a/src/file-store.s3.spec.ts b/src/file-store.s3.spec.ts index c5fcb2b..598a19d 100644 --- a/src/file-store.s3.spec.ts +++ b/src/file-store.s3.spec.ts @@ -208,6 +208,16 @@ describe("S3FileStore", () => { }); expect(mockSend).toHaveBeenCalledWith(expect.any(S3.PutObjectCommand)); }); + + // Without this, deleting the await or attaching .catch(() => undefined) leaves + // the suite green while every write silently fails. + it("propagates a failed PutObject", async () => { + mockSend.mockRejectedValueOnce(new Error("AccessDenied")); + + await expect( + store.save("a/b.txt", "text/plain", "content"), + ).rejects.toThrow("AccessDenied"); + }); }); describe("getAsBuffer", () => { @@ -264,6 +274,14 @@ describe("S3FileStore", () => { params.params.Body.destroy(); }); + + it("propagates a failed upload", async () => { + mockDone.mockRejectedValueOnce(new Error("upload aborted")); + + await expect( + store.copyFromLocalFile("a/b.ts", "text/plain", __filename), + ).rejects.toThrow("upload aborted"); + }); }); describe("copyFromStream", () => { @@ -283,6 +301,14 @@ describe("S3FileStore", () => { }); expect(mockDone).toHaveBeenCalled(); }); + + it("propagates a failed upload", async () => { + mockDone.mockRejectedValueOnce(new Error("premature close")); + + await expect( + store.copyFromStream("a/b.txt", "text/plain", Readable.from(["x"])), + ).rejects.toThrow("premature close"); + }); }); describe("getInfo", () => {