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-7682-00fe327c2ec9d89920db2a11d3959b87a0562fe3 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 2b66f1a2d4870ee35cadabafa5eee3350d007698 Author: Meng Wang <[email protected]> AuthorDate: Sat Aug 15 04:24:53 2026 +0000 test(frontend): cover DatasetDetail upload failures and the remaining template controls (#7682) ### What changes were proposed in this PR? Extends the `DatasetDetailComponent` spec with the upload failure handling and the template controls that were still unexecuted. No production code was changed. **Upload failures** (+5) — driven by erroring the spec's existing upload subject, so no real time passes: - a `409` produces the specific "Upload blocked (409)" notification, any other status the generic `"Upload failed. Please retry."`; - the task is marked `failed` and **keeps its progress** (a partially uploaded file stays at 42%, rather than being left at 100); - the concurrency slot is freed, so a queued fourth upload starts; - the `taskIndex === -1` arm still reports the failure. **Template** (+7) — contributor rows and their actions trigger, edit and delete, the toolbar's download and scale controls, and the sider resize. Three notes on how the DOM actually behaves here, recorded in comments: - The toolbar's download/scale buttons sit behind `*ngIf="selectedVersion"`, so the tests seed a version; the scale icon is `expand`/`compress`. - A contributor's Edit/Delete items live in an `nz-dropdown-menu`, which only mounts into a CDK overlay on a real user open — jsdom does not drive that. Those two tests assert the handlers the menu items bind to, while a separate test asserts the rendered rows and one dropdown trigger per contributor. - `onSideResize` defers to `requestAnimationFrame`; the test awaits a real frame and asserts `siderWidth`, and a second test spies `cancelAnimationFrame` to check the previous frame is dropped. No fake timers are introduced, and no pixel width is asserted (zero under jsdom). ### Any related issues, documentation, discussions? Closes #7679 ### How was this PR tested? Extended unit tests, run locally in `frontend/` (all green; the failure path was verified by breaking an assertion to confirm the suite goes red): ``` ng test --watch=false --include src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.spec.ts # Test Files 1 passed (1) | Tests 141 passed (141) prettier --write <spec> # clean eslint <spec> # clean ``` ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) --- .../dataset-detail.component.spec.ts | 164 ++++++++++++++++++++- 1 file changed, 163 insertions(+), 1 deletion(-) diff --git a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.spec.ts b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.spec.ts index 27f00cbfdf..78c35761a4 100644 --- a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.spec.ts @@ -45,7 +45,8 @@ import { DatasetStagedObject } from "../../../../../common/type/dataset-staged-o import { commonTestImports, commonTestProviders } from "../../../../../common/testing/test-utils"; import { Contributor, Dataset, DatasetVersion } from "../../../../../common/type/dataset"; import { DashboardDataset } from "../../../../type/dashboard-dataset.interface"; -import { HttpErrorResponse } from "@angular/common/http"; +import { HttpErrorResponse, HttpStatusCode } from "@angular/common/http"; +import { NzResizeEvent } from "ng-zorro-antd/resizable"; import { format } from "date-fns"; import { USER_DATASET } from "../../../../../app-routing.constant"; @@ -140,6 +141,64 @@ describe("DatasetDetailComponent upload queue", () => { fixture.detectChanges(); }); + /** + * A failed upload has to tell the user why, mark the task failed without leaving its bar at + * 100%, and free the concurrency slot — otherwise the queue stalls behind a dead upload. + */ + describe("a failed upload", () => { + const notification = () => TestBed.inject(NotificationService) as unknown as { error: ReturnType<typeof vi.fn> }; + + /** Fails the in-flight upload of `name` with the given HTTP status. */ + const failUpload = (index: number, status: number) => + uploadSubjects[index].error(new HttpErrorResponse({ status })); + + it("names the 409 conflict so the user knows to retry", () => { + dropFiles("a.csv"); + + failUpload(0, HttpStatusCode.Conflict); + + expect(notification().error).toHaveBeenCalledWith(expect.stringContaining("Upload blocked (409)")); + }); + + it("falls back to a generic message for any other failure", () => { + dropFiles("a.csv"); + + failUpload(0, HttpStatusCode.InternalServerError); + + expect(notification().error).toHaveBeenCalledWith("Upload failed. Please retry."); + }); + + it("marks the task failed and keeps its progress rather than showing it complete", () => { + dropFiles("a.csv"); + // a partially-uploaded file: the bar must not jump to 100 when it fails + uploadSubjects[0].next({ filePath: "a.csv", percentage: 42, status: "uploading" }); + + failUpload(0, HttpStatusCode.InternalServerError); + + const task = component.uploadTasks.find(t => t.filePath === "a.csv"); + expect(task?.status).toBe("failed"); + expect(task?.percentage).toBe(42); + }); + + it("frees the concurrency slot so a queued upload can start", () => { + // maxConcurrentFiles is 3, so a fourth file waits for a slot + dropFiles("a.csv", "b.csv", "c.csv", "d.csv"); + expect(uploadedPaths).toEqual(["a.csv", "b.csv", "c.csv"]); + + failUpload(0, HttpStatusCode.InternalServerError); + + expect(uploadedPaths).toContain("d.csv"); + }); + + it("still reports the failure when the task is no longer in the list", () => { + dropFiles("a.csv"); + component.uploadTasks = []; // the taskIndex === -1 arm + + expect(() => failUpload(0, HttpStatusCode.InternalServerError)).not.toThrow(); + expect(notification().error).toHaveBeenCalled(); + }); + }); + /** * Aborting an in-flight upload has to survive the backend still finalizing the previous attempt: * the abort call is retried on 409 up to ABORT_RETRY_MAX_ATTEMPTS, a 404 means it is already gone, @@ -2054,6 +2113,109 @@ describe("DatasetDetailComponent behavior", () => { switches[1].triggerEventHandler("ngModelChange", false); expect(datasetServiceStub.updateDatasetDownloadable).toHaveBeenCalledWith(5); }); + + // ─── contributor management ───────────────────────────────────────────── + const contributors = [ + { name: "Ada", email: "[email protected]", affiliation: "" } as Contributor, + { name: "Grace", email: "[email protected]", affiliation: "" } as Contributor, + ]; + + it("renders a row per contributor with the actions trigger", () => { + renderWith({ did: 5, datasetContributors: [...contributors], userDatasetAccessLevel: "WRITE" }); + + const rendered = fixture.nativeElement.textContent ?? ""; + expect(rendered).toContain("Ada"); + expect(rendered).toContain("Grace"); + // each row carries the dropdown trigger that hosts Edit/Delete + const triggers = fixture.debugElement + .queryAll(By.css("button[nz-dropdown]")) + .filter(btn => btn.nativeElement.querySelector("i.anticon-more")); + expect(triggers.length).toBe(contributors.length); + }); + + // Edit/Delete live inside an nz-dropdown-menu, which only mounts into a CDK overlay on a + // real user open — jsdom does not drive that. Assert the handlers those menu items bind to + // instead; the rendered trigger is covered above. + it("edits the chosen contributor through the menu's binding target", () => { + const updated = { ...contributors[0], affiliation: "Lab" }; + modalServiceStub.create.mockReturnValue({ afterClose: of(updated) }); + renderWith({ did: 5, datasetContributors: [...contributors], userDatasetAccessLevel: "WRITE" }); + + component.onEditContributor(contributors[0]); + + expect(component.datasetContributors[0]).toEqual(updated); + }); + + it("deletes the chosen contributor through the popconfirm's binding target", () => { + renderWith({ did: 5, datasetContributors: [...contributors], userDatasetAccessLevel: "WRITE" }); + + component.onDeleteContributor(contributors[0]); + + expect(component.datasetContributors.map(c => c.name)).toEqual(["Grace"]); + }); + + // ─── view controls ────────────────────────────────────────────────────── + + it("downloads the current file from the toolbar", () => { + // the toolbar controls are behind *ngIf="selectedVersion" + renderWith({ did: 5, selectedVersion: { dvid: 1, name: "v1" } as DatasetVersion }); + openTab("Versions & Files"); + const onDownload = vi.spyOn(component, "onClickDownloadCurrentFile").mockImplementation(() => {}); + + const downloadBtn = fixture.debugElement + .queryAll(By.css("button")) + .find(btn => btn.nativeElement.querySelector("i.anticon-download")); + expect(downloadBtn).toBeTruthy(); + downloadBtn!.triggerEventHandler("click", null); + + expect(onDownload).toHaveBeenCalled(); + }); + + it("toggles the scaled view from the toolbar", () => { + renderWith({ did: 5, isMaximized: false, selectedVersion: { dvid: 1, name: "v1" } as DatasetVersion }); + openTab("Versions & Files"); + + const scaleBtn = fixture.debugElement + .queryAll(By.css("button")) + .find(btn => btn.nativeElement.querySelector("i.anticon-expand, i.anticon-compress")); + expect(scaleBtn).toBeTruthy(); + scaleBtn!.triggerEventHandler("click", null); + fixture.detectChanges(); + + expect(component.isMaximized).toBe(true); + }); + + // ─── sider resize ─────────────────────────────────────────────────────── + + it("applies the dragged sider width on the next animation frame", async () => { + renderWith({ did: 5 }); + + component.onSideResize({ width: 321 } as NzResizeEvent); + // the handler defers to requestAnimationFrame; let that frame run + await new Promise(resolve => requestAnimationFrame(() => resolve(null))); + + expect(component.siderWidth).toBe(321); + }); + + it("cancels the frame the previous resize scheduled", () => { + renderWith({ did: 5 }); + // Hand out a known frame id so the assertion below pins down *which* frame is + // cancelled: the component starts with id = -1, so merely asserting that + // cancelAnimationFrame was called would pass even if the id were never tracked. + const request = vi.spyOn(globalThis, "requestAnimationFrame").mockReturnValue(100); + const cancel = vi.spyOn(globalThis, "cancelAnimationFrame"); + try { + component.onSideResize({ width: 100 } as NzResizeEvent); + cancel.mockClear(); // drop the initial cancel(-1) + + component.onSideResize({ width: 200 } as NzResizeEvent); + + expect(cancel).toHaveBeenCalledWith(100); + } finally { + cancel.mockRestore(); + request.mockRestore(); + } + }); }); });
