Copilot commented on code in PR #6359:
URL: https://github.com/apache/texera/pull/6359#discussion_r3565408223
##########
frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts:
##########
@@ -357,4 +366,343 @@ describe("SavedWorkflowSectionComponent", () => {
expect(testWorkflowFileNameConflictEntries[0].checked).toBe(true);
expect(testWorkflowFileNameConflictEntries[2].checked).toBe(true);
});
+
+ describe("additional UserWorkflowComponent behaviors", () => {
+ const VIEW_MODE_KEY = "texera.userWorkflow.viewMode";
+
+ const makeDashboardWorkflow = (wid: number | undefined, name: string):
DashboardWorkflow => ({
+ workflow: {
+ wid,
+ name,
+ description: "desc",
+ content: testWorkflowContent([]),
+ creationTime: 0,
+ lastModifiedTime: 0,
+ isPublished: 0,
+ readonly: false,
+ },
+ isOwner: true,
+ ownerName: "Texera",
+ accessLevel: "Write",
+ projectIDs: [],
+ ownerId: 1,
+ });
+
+ const makeEntry = (wid: number | undefined, name: string, checked =
false): DashboardEntry => {
+ const entry = new DashboardEntry(makeDashboardWorkflow(wid, name));
+ entry.checked = checked;
+ return entry;
+ };
+
+ const setEntries = (entries: DashboardEntry[]): void => {
+ component.searchResultsComponent.entries = entries;
+ };
+
+ afterEach(() => {
+ vi.restoreAllMocks();
+ localStorage.removeItem(VIEW_MODE_KEY);
+ });
+
+ describe("deleteWorkflow", () => {
+ it("deletes an entry with a wid and removes it from the results", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.deleteWorkflow = vi.fn().mockReturnValue(of(null));
+ const target = makeEntry(5, "to delete");
+ setEntries([target, makeEntry(6, "keep")]);
+
+ component.deleteWorkflow(target);
+
+ expect(persist.deleteWorkflow).toHaveBeenCalledWith([5]);
+ expect(component.searchResultsComponent.entries.map(e =>
e.name)).toEqual(["keep"]);
+ });
+
+ it("does nothing when the entry has no wid", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.deleteWorkflow = vi.fn();
+
+ component.deleteWorkflow(makeEntry(undefined, "no wid"));
+
+ expect(persist.deleteWorkflow).not.toHaveBeenCalled();
+ });
+ });
+
+ describe("onClickDuplicateSelectedWorkflows", () => {
+ it("duplicates checked wids without a pid and prepends the new entries",
() => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.duplicateWorkflow = vi
+ .fn()
+ .mockReturnValue(of([makeDashboardWorkflow(101, "dup a"),
makeDashboardWorkflow(102, "dup b")]));
+ component.pid = undefined;
+ setEntries([makeEntry(1, "wf 1", true), makeEntry(2, "wf 2", true),
makeEntry(3, "wf 3", false)]);
+
+ component.onClickDuplicateSelectedWorkflows();
+
+ expect(persist.duplicateWorkflow).toHaveBeenCalledWith([1, 2]);
+ expect(component.searchResultsComponent.entries.map(e =>
e.name)).toEqual([
+ "dup a",
+ "dup b",
+ "wf 1",
+ "wf 2",
+ "wf 3",
+ ]);
+ });
+
+ it("passes the pid to duplicateWorkflow when the section belongs to a
project", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.duplicateWorkflow =
vi.fn().mockReturnValue(of([makeDashboardWorkflow(101, "dup a")]));
+ component.pid = 9;
+ setEntries([makeEntry(1, "wf 1", true), makeEntry(2, "wf 2", true)]);
+
+ component.onClickDuplicateSelectedWorkflows();
+
+ expect(persist.duplicateWorkflow).toHaveBeenCalledWith([1, 2], 9);
+ });
+
+ it("early-returns without calling the service when a checked entry has
no wid", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.duplicateWorkflow = vi.fn();
+ setEntries([makeEntry(1, "wf 1", true), makeEntry(undefined, "wf 2",
true)]);
+
+ component.onClickDuplicateSelectedWorkflows();
+
+ expect(persist.duplicateWorkflow).not.toHaveBeenCalled();
+ });
+
+ it("alerts on a duplication error", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.duplicateWorkflow = vi.fn().mockReturnValue(throwError(() =>
"boom"));
+ const alertSpy = vi.spyOn(window, "alert").mockImplementation(() =>
{});
+ component.pid = undefined;
+ setEntries([makeEntry(1, "wf 1", true)]);
+
+ component.onClickDuplicateSelectedWorkflows();
+
+ expect(alertSpy).toHaveBeenCalledWith("boom");
+ });
+ });
+
+ describe("handleConfirmDeleteSelectedWorkflows", () => {
+ it("deletes checked wids and keeps undefined-wid entries", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.deleteWorkflow = vi.fn().mockReturnValue(of(null));
+ setEntries([
+ makeEntry(1, "a", true),
+ makeEntry(2, "b", true),
+ makeEntry(undefined, "c", false),
+ makeEntry(3, "d", false),
+ ]);
+
+ component.handleConfirmDeleteSelectedWorkflows();
+
+ expect(persist.deleteWorkflow).toHaveBeenCalledWith([1, 2]);
+ expect(component.searchResultsComponent.entries.map(e =>
e.name)).toEqual(["c", "d"]);
+ });
+
+ it("early-returns when a checked entry has no wid", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.deleteWorkflow = vi.fn();
+ setEntries([makeEntry(undefined, "a", true)]);
+
+ component.handleConfirmDeleteSelectedWorkflows();
+
+ expect(persist.deleteWorkflow).not.toHaveBeenCalled();
+ });
+
+ it("alerts on a deletion error", () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.deleteWorkflow = vi.fn().mockReturnValue(throwError(() =>
"delfail"));
+ const alertSpy = vi.spyOn(window, "alert").mockImplementation(() =>
{});
+ setEntries([makeEntry(1, "a", true)]);
+
+ component.handleConfirmDeleteSelectedWorkflows();
+
+ expect(alertSpy).toHaveBeenCalledWith("delfail");
+ });
+ });
+
+ describe("onClickOpenDownloadZip", () => {
+ it("does not call the download service when nothing is checked", () => {
+ setEntries([makeEntry(1, "a", false)]);
+
+ component.onClickOpenDownloadZip();
+
+
expect(downloadServiceSpy.downloadWorkflowsAsZip).not.toHaveBeenCalled();
+ });
+
+ it("logs an error when the download fails", () => {
+ setEntries([makeEntry(1, "a", true)]);
+
downloadServiceSpy.downloadWorkflowsAsZip.mockReturnValue(throwError(() =>
"dlfail"));
+ const errorSpy = vi.spyOn(console, "error").mockImplementation(() =>
{});
+
+ component.onClickOpenDownloadZip();
+
+
expect(downloadServiceSpy.downloadWorkflowsAsZip).toHaveBeenCalledWith([{ id:
1, name: "a" }]);
+ expect(errorSpy).toHaveBeenCalledWith("Error downloading workflows:",
"dlfail");
+ });
+ });
+
+ describe("workflow uploads", () => {
+ it("imports a valid .json workflow, appending an entry and toasting
success", async () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.createWorkflow =
vi.fn().mockReturnValue(of(makeDashboardWorkflow(42, "wf")));
+ const searchSpy = vi.spyOn(component,
"search").mockResolvedValue(undefined);
+ const successSpy = vi
+ .spyOn(TestBed.inject(NotificationService), "success")
+ .mockImplementation(() => undefined as any);
+ const content = testWorkflowContent([]);
+ const file = new File([JSON.stringify(content)], "wf.json");
+ setEntries([]);
+
+ const result = await
firstValueFrom(component.onClickUploadExistingWorkflowFromLocal(file as any));
+
+ expect(result).toBe(false);
+ expect(persist.createWorkflow).toHaveBeenCalledWith(content, "wf");
+ expect(component.searchResultsComponent.entries.map(e =>
e.name)).toContain("wf");
+ expect(searchSpy).toHaveBeenCalledWith(true);
+ expect(successSpy).toHaveBeenCalledWith("Upload Successful");
+ });
+
+ it("toasts an error and errors the stream when the file is not JSON",
async () => {
+ const errorSpy = vi
+ .spyOn(TestBed.inject(NotificationService), "error")
+ .mockImplementation(() => undefined as any);
+ const file = new File(["this is not json"], "bad.json");
+
+ await
expect(firstValueFrom(component.onClickUploadExistingWorkflowFromLocal(file as
any))).rejects.toThrow();
+
+ expect(errorSpy).toHaveBeenCalledWith(
+ "An error occurred when importing the workflow. Please import a
workflow json file."
+ );
+ });
+
+ it("falls back to DEFAULT_WORKFLOW_NAME when the stripped name is
empty", async () => {
+ const persist = TestBed.inject(WorkflowPersistService) as any;
+ persist.createWorkflow =
vi.fn().mockReturnValue(of(makeDashboardWorkflow(1, "x")));
+ vi.spyOn(component, "search").mockResolvedValue(undefined);
+ const content = testWorkflowContent([]);
+ const file = new File([JSON.stringify(content)], ".json");
+
+ await
firstValueFrom(component.onClickUploadExistingWorkflowFromLocal(file as any));
+
+ expect(persist.createWorkflow).toHaveBeenCalledWith(content,
DEFAULT_WORKFLOW_NAME);
+ });
+
+ // handleZipUploads is intentionally not covered here: its first
statement is
+ // `new JSZip()` against the `import * as JSZip from "jszip"` namespace,
which
+ // crashes under Vitest ("... is not a constructor"). The same
limitation is
+ // documented and skipped in download.service.spec.ts. nameWorkflow's
dedup
+ // logic is exercised below with a lightweight fake that mimics
`zip.files`.
Review Comment:
This block states `handleZipUploads` (the .zip import path) is intentionally
not covered due to the `new JSZip()`/`import * as JSZip from "jszip"`
constructor crash in Vitest, but the PR description and issue #6357 both list
“zip import (real JSZip)” / `handleZipUploads` coverage as an explicit goal and
the PR is marked `Closes #6357`. As written, the zip-import behavior is still
untested, so the PR metadata/closure appears premature.
Consider either (1) enabling zip-import coverage by fixing the underlying
JSZip import/constructor usage (so `new JSZip()` works in Vitest and at
runtime) and then adding the `.zip` test, or (2) updating the PR description
and removing the issue closure until zip-import coverage is actually added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
