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-6739-03038c798261186501bdbba4acb5cce28c31b541 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 71963f73890cf4fd5a7b9a79e7bd258f2e4341d6 Author: Xinyuan Lin <[email protected]> AuthorDate: Wed Jul 22 12:53:45 2026 -0700 test(frontend): extend WorkflowPersistService unit test coverage (#6739) ### What changes were proposed in this PR? Extends `workflow-persist.service.spec.ts` with 24 tests (7 -> 31) covering the persist/create/duplicate/delete/retrieve/update endpoints, their request bodies and urls, the broken-workflow and null-response filters, and the updateName/Description catchError branches (via HttpTestingController). Coverage 54% -> 100%. No existing tests modified. ### Any related issues, documentation, discussions? Closes #6732. ### How was this PR tested? `ng test --include='**/workflow-persist.service.spec.ts'` -> 31/31 passing. `yarn format:ci` passes. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) --- .../workflow-persist.service.spec.ts | 349 +++++++++++++++++++++ 1 file changed, 349 insertions(+) diff --git a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts index 1a9dd46871..007f42f99f 100644 --- a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts +++ b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts @@ -25,12 +25,25 @@ import { WORKFLOW_ID_URL, WORKFLOW_OWNER_URL, WORKFLOW_SEARCH_URL, + WORKFLOW_PERSIST_URL, + WORKFLOW_CREATE_URL, + WORKFLOW_DUPLICATE_URL, + WORKFLOW_DELETE_URL, + WORKFLOW_LIST_URL, + WORKFLOW_UPDATENAME_URL, + WORKFLOW_UPDATEDESCRIPTION_URL, + WORKFLOW_OWNER_NAME, + WORKFLOW_NAME, + WORKFLOW_PUBLIC_WORKFLOW, + WORKFLOW_DESCRIPTION, + WORKFLOW_SIZE, } from "./workflow-persist.service"; import { jsonCast } from "../../util/storage"; import { Workflow, WorkflowContent } from "../../type/workflow"; import { AppSettings } from "../../app-setting"; import { DashboardWorkflow } from "../../../dashboard/type/dashboard-workflow.interface"; import { SearchFilterParameters, toQueryStrings } from "../../../dashboard/type/search-filter-parameters"; +import { NotificationService } from "../notification/notification.service"; import { last } from "rxjs/operators"; describe("WorkflowPersistService", () => { @@ -151,4 +164,340 @@ describe("WorkflowPersistService", () => { expect(result?.[0]).toHaveProperty("dashboardWorkflowEntry"); expect(result?.[0]?.workflow?.content).toEqual({ operators: [] }); }); + + describe("uncovered persist/retrieve/duplicate/delete endpoints", () => { + let notificationService: NotificationService; + + // content with no links -> checkIfWorkflowBroken returns false + const validContent = { operators: [], operatorPositions: {}, links: [] } as unknown as WorkflowContent; + // a link that references operators that do not exist -> checkIfWorkflowBroken returns true + const brokenContent = { + operators: [], + operatorPositions: {}, + links: [{ source: { operatorID: "does-not-exist" }, target: { operatorID: "also-missing" } }], + } as unknown as WorkflowContent; + + beforeEach(() => { + notificationService = TestBed.inject(NotificationService); + }); + + afterEach(() => { + httpTestingController.verify(); + vi.restoreAllMocks(); + }); + + it("persistWorkflow POSTs the serialized body and parses the response content", () => { + const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); + const workflow = { + wid: 9, + name: "my wf", + description: "a description", + content: validContent, + isPublished: true, + } as unknown as Workflow; + + let result: Workflow | undefined; + service.persistWorkflow(workflow).subscribe(w => (result = w)); + + const req = httpTestingController.expectOne(`${API}/${WORKFLOW_PERSIST_URL}`); + expect(req.request.method).toBe("POST"); + expect(req.request.body).toEqual({ + wid: 9, + name: "my wf", + description: "a description", + content: JSON.stringify(validContent), + isPublic: true, + }); + + req.flush({ wid: 9, name: "my wf", content: '{"operators":[]}' }); + + // valid workflow -> no error notification, and string content is parsed + expect(errorSpy).not.toHaveBeenCalled(); + expect(result?.content).toEqual({ operators: [] }); + }); + + it("persistWorkflow notifies the user when the workflow is broken but still POSTs", () => { + const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); + const workflow = { + wid: 1, + name: "broken", + description: "", + content: brokenContent, + isPublished: false, + } as unknown as Workflow; + + service.persistWorkflow(workflow).subscribe(); + + expect(errorSpy).toHaveBeenCalledTimes(1); + expect(errorSpy).toHaveBeenCalledWith( + "Sorry! The workflow is broken and cannot be persisted. Please contact the system admin." + ); + + const req = httpTestingController.expectOne(`${API}/${WORKFLOW_PERSIST_URL}`); + expect(req.request.body.isPublic).toBe(false); + req.flush({ wid: 1, name: "broken", content: '{"operators":[]}' }); + }); + + it("persistWorkflow filters out a null response so no value is emitted", () => { + const workflow = { + wid: 2, + name: "n", + description: "", + content: validContent, + isPublished: false, + } as unknown as Workflow; + + let emitted = false; + service.persistWorkflow(workflow).subscribe(() => (emitted = true)); + + httpTestingController.expectOne(`${API}/${WORKFLOW_PERSIST_URL}`).flush(null); + expect(emitted).toBe(false); + }); + + it("createWorkflow POSTs the name and serialized content and emits the created workflow", () => { + const content = jsonCast<WorkflowContent>(testContent); + + let result: DashboardWorkflow | undefined; + service.createWorkflow(content, "brand new").subscribe(r => (result = r)); + + const req = httpTestingController.expectOne(`${API}/${WORKFLOW_CREATE_URL}`); + expect(req.request.method).toBe("POST"); + expect(req.request.body).toEqual({ name: "brand new", content: JSON.stringify(content) }); + + const created = { workflow: { wid: 99, name: "brand new" } } as unknown as DashboardWorkflow; + req.flush(created); + expect(result).toEqual(created); + }); + + it("createWorkflow filters out a null response so no value is emitted", () => { + let emitted = false; + service.createWorkflow(jsonCast<WorkflowContent>(testContent)).subscribe(() => (emitted = true)); + + httpTestingController.expectOne(`${API}/${WORKFLOW_CREATE_URL}`).flush(null); + expect(emitted).toBe(false); + }); + + it("duplicateWorkflow POSTs only wids when no pid is provided", () => { + let result: DashboardWorkflow[] | undefined; + service.duplicateWorkflow([3, 4]).subscribe(r => (result = r)); + + const req = httpTestingController.expectOne(`${API}/${WORKFLOW_DUPLICATE_URL}`); + expect(req.request.method).toBe("POST"); + expect(req.request.body).toEqual({ wids: [3, 4] }); + expect(req.request.body).not.toHaveProperty("pid"); + + const dup = [{ workflow: { wid: 10 } }] as unknown as DashboardWorkflow[]; + req.flush(dup); + expect(result).toEqual(dup); + }); + + it("duplicateWorkflow includes pid in the body when provided", () => { + service.duplicateWorkflow([5], 42).subscribe(); + + const req = httpTestingController.expectOne(`${API}/${WORKFLOW_DUPLICATE_URL}`); + expect(req.request.body).toEqual({ wids: [5], pid: 42 }); + req.flush([{ workflow: { wid: 11 } }]); + }); + + it("duplicateWorkflow filters out an empty-array response", () => { + let emitted = false; + service.duplicateWorkflow([6]).subscribe(() => (emitted = true)); + + httpTestingController.expectOne(`${API}/${WORKFLOW_DUPLICATE_URL}`).flush([]); + expect(emitted).toBe(false); + }); + + it("retrieveWorkflowsBySessionUser GETs the list url and maps each entry", () => { + const entry = { workflow: { wid: 1, name: "w", content: '{"operators":[]}' } } as unknown as DashboardWorkflow; + + let result: DashboardWorkflow[] | undefined; + service.retrieveWorkflowsBySessionUser().subscribe(r => (result = r)); + + const req = httpTestingController.expectOne(`${API}/${WORKFLOW_LIST_URL}`); + expect(req.request.method).toBe("GET"); + req.flush([entry]); + + expect(result?.length).toBe(1); + expect(result?.[0]).toHaveProperty("dashboardWorkflowEntry"); + expect(result?.[0]?.workflow?.content).toEqual({ operators: [] }); + }); + + it("deleteWorkflow POSTs the wids to the delete url", () => { + let responded = false; + service.deleteWorkflow([7, 8]).subscribe(() => (responded = true)); + + const req = httpTestingController.expectOne(`${API}/${WORKFLOW_DELETE_URL}`); + expect(req.request.method).toBe("POST"); + expect(req.request.body).toEqual({ wids: [7, 8] }); + req.flush({}); + expect(responded).toBe(true); + }); + + it("updateWorkflowName POSTs wid and name on success", () => { + let responded = false; + service.updateWorkflowName(12, "renamed").subscribe(() => (responded = true)); + + const req = httpTestingController.expectOne(`${API}/${WORKFLOW_UPDATENAME_URL}`); + expect(req.request.method).toBe("POST"); + expect(req.request.body).toEqual({ wid: 12, name: "renamed" }); + req.flush({}); + expect(responded).toBe(true); + }); + + it("updateWorkflowName notifies with the server message and rethrows on error", () => { + const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); + + let caught: unknown; + service.updateWorkflowName(13, "bad").subscribe({ + next: () => {}, + error: (e: unknown) => (caught = e), + }); + + httpTestingController + .expectOne(`${API}/${WORKFLOW_UPDATENAME_URL}`) + .flush({ message: "name already taken" }, { status: 400, statusText: "Bad Request" }); + + expect(errorSpy).toHaveBeenCalledWith("name already taken"); + expect(caught).toBeTruthy(); + }); + + it("updateWorkflowDescription POSTs wid and description on success", () => { + let responded = false; + service.updateWorkflowDescription(14, "new desc").subscribe(() => (responded = true)); + + const req = httpTestingController.expectOne(`${API}/${WORKFLOW_UPDATEDESCRIPTION_URL}`); + expect(req.request.method).toBe("POST"); + expect(req.request.body).toEqual({ wid: 14, description: "new desc" }); + req.flush({}); + expect(responded).toBe(true); + }); + + it("updateWorkflowDescription notifies with the server message and rethrows on error", () => { + const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); + + let caught: unknown; + service.updateWorkflowDescription(15, "bad").subscribe({ + next: () => {}, + error: (e: unknown) => (caught = e), + }); + + httpTestingController + .expectOne(`${API}/${WORKFLOW_UPDATEDESCRIPTION_URL}`) + .flush({ message: "description too long" }, { status: 500, statusText: "Server Error" }); + + expect(errorSpy).toHaveBeenCalledWith("description too long"); + expect(caught).toBeTruthy(); + }); + + it("getWorkflowIsPublished GETs the type url as text", () => { + let result: string | undefined; + service.getWorkflowIsPublished(16).subscribe(r => (result = r)); + + const req = httpTestingController.expectOne(`${API}/${WORKFLOW_BASE_URL}/type/16`); + expect(req.request.method).toBe("GET"); + expect(req.request.responseType).toBe("text"); + req.flush("true"); + expect(result).toBe("true"); + }); + + it("updateWorkflowIsPublished PUTs to the public url when publishing", () => { + service.updateWorkflowIsPublished(17, true).subscribe(); + + const req = httpTestingController.expectOne(`${API}/${WORKFLOW_BASE_URL}/public/17`); + expect(req.request.method).toBe("PUT"); + expect(req.request.body).toBeNull(); + req.flush(null); + }); + + it("updateWorkflowIsPublished PUTs to the private url when unpublishing", () => { + service.updateWorkflowIsPublished(18, false).subscribe(); + + const req = httpTestingController.expectOne(`${API}/${WORKFLOW_BASE_URL}/private/18`); + expect(req.request.method).toBe("PUT"); + expect(req.request.body).toBeNull(); + req.flush(null); + }); + + it("setWorkflowPersistFlag toggles the value read by isWorkflowPersistEnabled", () => { + // defaults to enabled + expect(service.isWorkflowPersistEnabled()).toBe(true); + + service.setWorkflowPersistFlag(false); + expect(service.isWorkflowPersistEnabled()).toBe(false); + + service.setWorkflowPersistFlag(true); + expect(service.isWorkflowPersistEnabled()).toBe(true); + }); + + it("getOwnerName GETs the owner-name url with a wid param as text", () => { + let result: string | undefined; + service.getOwnerName(19).subscribe(r => (result = r)); + + const req = httpTestingController.expectOne( + r => r.url === `${API}/${WORKFLOW_OWNER_NAME}` && r.params.get("wid") === "19" + ); + expect(req.request.method).toBe("GET"); + expect(req.request.responseType).toBe("text"); + req.flush("alice"); + expect(result).toBe("alice"); + }); + + it("getWorkflowName GETs the workflow-name url with a wid param as text", () => { + let result: string | undefined; + service.getWorkflowName(20).subscribe(r => (result = r)); + + const req = httpTestingController.expectOne( + r => r.url === `${API}/${WORKFLOW_NAME}` && r.params.get("wid") === "20" + ); + expect(req.request.method).toBe("GET"); + expect(req.request.responseType).toBe("text"); + req.flush("some workflow"); + expect(result).toBe("some workflow"); + }); + + it("retrievePublicWorkflow GETs the publicised url and parses the content", () => { + let result: Workflow | undefined; + service.retrievePublicWorkflow(21).subscribe(w => (result = w)); + + const req = httpTestingController.expectOne(`${API}/${WORKFLOW_PUBLIC_WORKFLOW}/21`); + expect(req.request.method).toBe("GET"); + req.flush({ wid: 21, name: "pub", content: '{"operators":[]}' }); + + expect(result?.content).toEqual({ operators: [] }); + }); + + it("retrievePublicWorkflow filters out a null response", () => { + let emitted = false; + service.retrievePublicWorkflow(22).subscribe(() => (emitted = true)); + + httpTestingController.expectOne(`${API}/${WORKFLOW_PUBLIC_WORKFLOW}/22`).flush(null); + expect(emitted).toBe(false); + }); + + it("getWorkflowDescription GETs the description url with a wid param as text", () => { + let result: string | undefined; + service.getWorkflowDescription(23).subscribe(r => (result = r)); + + const req = httpTestingController.expectOne( + r => r.url === `${API}/${WORKFLOW_DESCRIPTION}` && r.params.get("wid") === "23" + ); + expect(req.request.method).toBe("GET"); + expect(req.request.responseType).toBe("text"); + req.flush("the description"); + expect(result).toBe("the description"); + }); + + it("getSizes GETs the size url appending every wid as a separate param", () => { + let result: Record<number, number> | undefined; + service.getSizes([24, 25, 26]).subscribe(r => (result = r)); + + const req = httpTestingController.expectOne( + r => r.url === `${API}/${WORKFLOW_SIZE}` && r.params.getAll("wid")?.join(",") === "24,25,26" + ); + expect(req.request.method).toBe("GET"); + const sizes = { 24: 100, 25: 200, 26: 300 }; + req.flush(sizes); + expect(result).toEqual(sizes); + }); + }); });
