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/22 07:47:54 UTC

(texera) branch gh-readonly-queue/main/pr-6739-8e2caa1cea5139fcfadb3e096e066b61e9e25b83 created (now 53bd675c35)

This is an automated email from the ASF dual-hosted git repository.

github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-6739-8e2caa1cea5139fcfadb3e096e066b61e9e25b83
in repository https://gitbox.apache.org/repos/asf/texera.git


      at 53bd675c35 test(frontend): extend WorkflowPersistService unit test coverage (#6739)

This branch includes the following new commits:

     new 53bd675c35 test(frontend): extend WorkflowPersistService unit test coverage (#6739)

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(texera) 01/01: test(frontend): extend WorkflowPersistService unit test coverage (#6739)

Posted by gi...@apache.org.
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-8e2caa1cea5139fcfadb3e096e066b61e9e25b83
in repository https://gitbox.apache.org/repos/asf/texera.git

commit 53bd675c35e76df4b698f093624cbff278b58d1e
Author: Xinyuan Lin <xi...@uci.edu>
AuthorDate: Wed Jul 22 00:47:47 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);
+    });
+  });
 });