This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6817-03a0e3a3b12424b99fa829d56fdb0a7512c6f173 in repository https://gitbox.apache.org/repos/asf/texera.git
commit cb59021826554383bdf6dd6ea7bd138e503a879b Author: Xinyuan Lin <[email protected]> AuthorDate: Sun Jul 26 05:34:05 2026 -0700 test(frontend): extend DownloadService coverage; fix jszip default import (#6817) ### What changes were proposed in this PR? Extends `download.service.spec.ts` with 4 tests (19 -> 23) and un-skips the previously-skipped zip tests, reaching 100% coverage of `download.service.ts`. New/strengthened tests cover the multi-file operators-result zip branch (loads the produced blob back and asserts entries), the `flat()` merge routing, `createWorkflowsZip` assembly (one JSON per workflow), `nameWorkflow` collision de-duplication, and the `exportWorkflowResultToLocal` empty-token fallback. **Source change:** `import * as JSZip` -> `import JSZip from "jszip"` in `download.service.ts`. The namespace import makes `new JSZip()` throw "not a constructor" under esbuild/Vitest (and esbuild warns it will crash at runtime), which had forced every zip path to be skipped. The default import matches the existing usage in `user-workflow.component.ts` (already shipped in the prod build) and is safe for the webpack prod build — jszip is CommonJS (`module.exports = JSZip`) and `allowSyntheticDefaultImports` is set. ### Any related issues, documentation, discussions? Closes #6813. ### How was this PR tested? `ng test --include='**/download.service.spec.ts'` -> 23/23 passing (0 skipped). `yarn format:ci` passes. The default-import pattern is already used and shipped in `user-workflow.component.ts`. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) Co-authored-by: Yicong Huang <[email protected]> Co-authored-by: Meng Wang <[email protected]> --- .../service/user/download/download.service.spec.ts | 102 +++++++++++++++++++-- .../service/user/download/download.service.ts | 2 +- 2 files changed, 95 insertions(+), 9 deletions(-) diff --git a/frontend/src/app/dashboard/service/user/download/download.service.spec.ts b/frontend/src/app/dashboard/service/user/download/download.service.spec.ts index 30bd053381..7bc180e737 100644 --- a/frontend/src/app/dashboard/service/user/download/download.service.spec.ts +++ b/frontend/src/app/dashboard/service/user/download/download.service.spec.ts @@ -29,6 +29,7 @@ import { commonTestProviders } from "../../../../common/testing/test-utils"; import type { Mocked } from "vitest"; import { WORKFLOW_EXECUTIONS_API_BASE_URL } from "../workflow-executions/workflow-executions.service"; import { DashboardWorkflowComputingUnit } from "../../../../common/type/workflow-computing-unit"; +import JSZip from "jszip"; function computingUnit(type: string, cuid: number): DashboardWorkflowComputingUnit { return { computingUnit: { cuid, type } } as unknown as DashboardWorkflowComputingUnit; @@ -224,6 +225,47 @@ describe("DownloadService", () => { expect(notificationServiceSpy.error).toHaveBeenCalledWith("Error downloading workflows as ZIP"); }); + // ─── createWorkflowsZip / nameWorkflow (real zip assembly) ──────────────── + // These drive downloadWorkflowsAsZip through the real (un-mocked) private + // createWorkflowsZip → downloadWorkflow → nameWorkflow chain, so the produced + // blob is a genuine zip we can load back and inspect. + + it("assembles a real zip with one JSON entry per workflow", async () => { + workflowPersistServiceSpy.retrieveWorkflow.mockReturnValue(of({ content: { op: "x" } } as any)); + + const result = await firstValueFrom( + downloadService.downloadWorkflowsAsZip([ + { id: 1, name: "Alpha" }, + { id: 2, name: "Beta" }, + ]) + ); + + expect(result).toBeInstanceOf(Blob); + expect(workflowPersistServiceSpy.retrieveWorkflow).toHaveBeenCalledWith(1); + expect(workflowPersistServiceSpy.retrieveWorkflow).toHaveBeenCalledWith(2); + expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(result, expect.stringMatching(/^workflowExports-.*\.zip$/)); + expect(notificationServiceSpy.success).toHaveBeenCalledWith("Workflows have been downloaded as ZIP"); + + const loaded = await JSZip.loadAsync(result); + expect(Object.keys(loaded.files).sort()).toEqual(["Alpha.json", "Beta.json"]); + }); + + it("de-duplicates colliding workflow filenames inside the zip", async () => { + workflowPersistServiceSpy.retrieveWorkflow.mockReturnValue(of({ content: { op: "x" } } as any)); + + const result = await firstValueFrom( + downloadService.downloadWorkflowsAsZip([ + { id: 1, name: "Dup" }, + { id: 2, name: "Dup" }, + { id: 3, name: "Dup" }, + ]) + ); + + // nameWorkflow appends -1, -2, ... on each collision so no entry is lost. + const loaded = await JSZip.loadAsync(result); + expect(Object.keys(loaded.files).sort()).toEqual(["Dup-1.json", "Dup-2.json", "Dup.json"]); + }); + // ─── downloadOperatorsResult ────────────────────────────────────────────── it("downloads a single operator file directly when there's exactly one file", async () => { @@ -241,14 +283,10 @@ describe("DownloadService", () => { expect(notificationServiceSpy.success).toHaveBeenCalledWith("Operator result has been downloaded"); }); - // The multi-file zip path goes through `new JSZip()` against the - // `import * as JSZip from "jszip"` namespace, which the build flags as - // `Constructing "JSZip" will crash at run-time because it's an import - // namespace object`. Vitest reproduces the failure (`__vite_ssr_import_* - // is not a constructor`). Tracked as a separate cleanup in the codebase; - // the test is here as a placeholder so we re-enable it once the import - // is normalised to a default import. - it.skip("zips multiple operator files into a workflow-named archive", async () => { + // The multi-file zip path goes through `new JSZip()`; now that the source + // imports JSZip as a default import (rather than an import namespace object), + // it constructs correctly under Vitest so this exercises the real archive. + it("zips multiple operator files into a workflow-named archive", async () => { const a = new Blob(["a"], { type: "text/plain" }); const b = new Blob(["b"], { type: "text/plain" }); const result = await firstValueFrom( @@ -264,8 +302,32 @@ describe("DownloadService", () => { ); expect(result).toBeInstanceOf(Blob); + expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to download operator results as ZIP"); expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(expect.any(Blob), "results_7_TwoFile.zip"); expect(notificationServiceSpy.success).toHaveBeenCalledWith("Operator results have been downloaded as ZIP"); + + // The produced blob is a real zip carrying both operator files. + const loaded = await JSZip.loadAsync(result); + expect(Object.keys(loaded.files).sort()).toEqual(["a.csv", "b.csv"]); + }); + + it("flattens operator result batches before deciding single vs zip", async () => { + // Two source observables, each emitting one file: flat() merges them into + // two files, which routes through the multi-file zip branch. + const result = await firstValueFrom( + downloadService.downloadOperatorsResult( + [ + of([{ filename: "first.csv", blob: new Blob(["1"], { type: "text/plain" }) }]), + of([{ filename: "second.csv", blob: new Blob(["2"], { type: "text/plain" }) }]), + ], + { wid: 3, name: "Flat" } as any + ) + ); + + expect(result).toBeInstanceOf(Blob); + expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(expect.any(Blob), "results_3_Flat.zip"); + const loaded = await JSZip.loadAsync(result); + expect(Object.keys(loaded.files).sort()).toEqual(["first.csv", "second.csv"]); }); it("errors out cleanly when no operator result files are provided", async () => { @@ -409,6 +471,30 @@ describe("DownloadService", () => { expect(document.querySelector('iframe[name="download-iframe"]')).toBeNull(); }); + it("falls back to an empty token when none is stored", () => { + // Override the beforeEach stub so getItem returns null, exercising the + // `?? ""` fallback on the auth token. + vi.stubGlobal("localStorage", { + getItem: vi.fn().mockReturnValue(null), + setItem: vi.fn(), + removeItem: vi.fn(), + }); + + downloadService.exportWorkflowResultToLocal( + "csv", + 1, + "WF", + EXPORT_OPERATORS, + 0, + 0, + "out.csv", + computingUnit("local", 5) + ); + + const form = document.querySelector('form[target="download-iframe"]') as HTMLFormElement; + expect((form.querySelector('input[name="token"]') as HTMLInputElement).value).toBe(""); + }); + it("targets the cuid-scoped endpoint for a kubernetes computing unit", () => { downloadService.exportWorkflowResultToLocal( "csv", diff --git a/frontend/src/app/dashboard/service/user/download/download.service.ts b/frontend/src/app/dashboard/service/user/download/download.service.ts index 811ebe37f4..2b7e5544e2 100644 --- a/frontend/src/app/dashboard/service/user/download/download.service.ts +++ b/frontend/src/app/dashboard/service/user/download/download.service.ts @@ -24,7 +24,7 @@ import { FileSaverService } from "../file/file-saver.service"; import { NotificationService } from "../../../../common/service/notification/notification.service"; import { DatasetService } from "../dataset/dataset.service"; import { WorkflowPersistService } from "src/app/common/service/workflow-persist/workflow-persist.service"; -import * as JSZip from "jszip"; +import JSZip from "jszip"; import { Workflow } from "../../../../common/type/workflow"; import { HttpClient, HttpResponse } from "@angular/common/http"; import { WORKFLOW_EXECUTIONS_API_BASE_URL } from "../workflow-executions/workflow-executions.service";
