You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@texera.apache.org by gi...@apache.org on 2026/07/26 12:34:27 UTC
(texera) 01/01: test(frontend): extend DownloadService coverage; fix jszip default import (#6817)
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 <xi...@uci.edu>
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 <17...@users.noreply.github.com>
Co-authored-by: Meng Wang <me...@uci.edu>
---
.../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";