Copilot commented on code in PR #7915:
URL: https://github.com/apache/texera/pull/7915#discussion_r3842285468
##########
frontend/src/app/workspace/component/menu/menu.component.spec.ts:
##########
@@ -377,6 +493,18 @@ describe("MenuComponent", () => {
expect(setNameSpy).toHaveBeenCalledWith("renamed");
});
+ it("onWorkflowNameChange persists the rename only while logged in", () => {
+ vi.spyOn(workflowActionService, "setWorkflowName").mockImplementation(()
=> {});
+ const persistSpy = vi.spyOn(component,
"persistWorkflow").mockImplementation(() => {});
+
+ component.onWorkflowNameChange();
+ expect(persistSpy).toHaveBeenCalledTimes(1);
+
+ vi.spyOn(component.userService, "isLogin").mockReturnValue(false);
Review Comment:
The “logged in” half of this test implicitly depends on whatever the default
`userService.isLogin()` behavior is for the shared fixture. To make the test
self-contained and robust, explicitly stub `isLogin` to return `true` for the
first call (or set it once at the top and then override to `false` for the
second half).
##########
frontend/src/app/workspace/component/menu/menu.component.spec.ts:
##########
@@ -1216,4 +1358,293 @@ describe("MenuComponent", () => {
});
});
});
+
+ describe("ngOnInit subscriptions", () => {
+ afterEach(() => {
+ vi.restoreAllMocks();
+ });
+
+ it("re-applies the run button behavior on every execution state event", ()
=> {
+ const stateEvents$ = new Subject<{ current: { state: ExecutionState }
}>();
+ vi.spyOn(executeWorkflowService,
"getExecutionStateStream").mockReturnValue(
+ stateEvents$.asObservable() as ReturnType<typeof
executeWorkflowService.getExecutionStateStream>
+ );
+ const stateFixture = TestBed.createComponent(MenuComponent);
+ const stateComponent = stateFixture.componentInstance;
+ stateFixture.detectChanges();
+ stateComponent.isWorkflowValid = true;
+ stateComponent.isWorkflowEmpty = false;
+ stateComponent.computingUnitStatus = ComputingUnitState.Running;
+ Object.defineProperty(stateComponent.workflowWebsocketService,
"isConnected", {
+ get: () => true,
+ configurable: true,
+ });
+
+ try {
+ stateEvents$.next({ current: { state: ExecutionState.Running } });
+ expect(stateComponent.executionState).toBe(ExecutionState.Running);
+ expect(stateComponent.runButtonText).toBe("Pause");
+
+ stateEvents$.next({ current: { state: ExecutionState.Paused } });
+ expect(stateComponent.runButtonText).toBe("Resume");
+ } finally {
+ stateFixture.destroy();
+ }
+ });
+
+ it("deactivates the export button unless the feature is on and results
exist", () => {
+ const guiConfig = TestBed.inject(GuiConfigService);
+ const results$ =
component.workflowResultExportService.hasResultToExportOnAllOperators;
+
+ // Feature off: deactivated whatever the results say.
+ guiConfig.env.exportExecutionResultEnabled = false;
+ results$.next(true);
+ expect(component.isExportDeactivate).toBe(true);
+
+ // Feature on, but nothing to export.
+ guiConfig.env.exportExecutionResultEnabled = true;
+ results$.next(false);
+ expect(component.isExportDeactivate).toBe(true);
+
+ results$.next(true);
+ expect(component.isExportDeactivate).toBe(false);
Review Comment:
This test mutates a shared config singleton
(`guiConfig.env.exportExecutionResultEnabled`) without restoring it, which can
leak state into later tests. Capture the original value and restore it in the
test (or an `afterEach` in this `describe`), and consider creating a fresh
component instance per flag setting if the production code reads the flag only
during `ngOnInit`.
##########
frontend/src/app/workspace/component/menu/menu.component.spec.ts:
##########
@@ -1216,4 +1358,293 @@ describe("MenuComponent", () => {
});
});
});
+
+ describe("ngOnInit subscriptions", () => {
+ afterEach(() => {
+ vi.restoreAllMocks();
+ });
+
+ it("re-applies the run button behavior on every execution state event", ()
=> {
+ const stateEvents$ = new Subject<{ current: { state: ExecutionState }
}>();
+ vi.spyOn(executeWorkflowService,
"getExecutionStateStream").mockReturnValue(
+ stateEvents$.asObservable() as ReturnType<typeof
executeWorkflowService.getExecutionStateStream>
+ );
+ const stateFixture = TestBed.createComponent(MenuComponent);
+ const stateComponent = stateFixture.componentInstance;
+ stateFixture.detectChanges();
+ stateComponent.isWorkflowValid = true;
+ stateComponent.isWorkflowEmpty = false;
+ stateComponent.computingUnitStatus = ComputingUnitState.Running;
+ Object.defineProperty(stateComponent.workflowWebsocketService,
"isConnected", {
+ get: () => true,
+ configurable: true,
+ });
+
+ try {
+ stateEvents$.next({ current: { state: ExecutionState.Running } });
+ expect(stateComponent.executionState).toBe(ExecutionState.Running);
+ expect(stateComponent.runButtonText).toBe("Pause");
+
+ stateEvents$.next({ current: { state: ExecutionState.Paused } });
+ expect(stateComponent.runButtonText).toBe("Resume");
+ } finally {
+ stateFixture.destroy();
+ }
+ });
+
+ it("deactivates the export button unless the feature is on and results
exist", () => {
+ const guiConfig = TestBed.inject(GuiConfigService);
+ const results$ =
component.workflowResultExportService.hasResultToExportOnAllOperators;
+
+ // Feature off: deactivated whatever the results say.
+ guiConfig.env.exportExecutionResultEnabled = false;
+ results$.next(true);
+ expect(component.isExportDeactivate).toBe(true);
+
+ // Feature on, but nothing to export.
+ guiConfig.env.exportExecutionResultEnabled = true;
+ results$.next(false);
+ expect(component.isExportDeactivate).toBe(true);
+
+ results$.next(true);
+ expect(component.isExportDeactivate).toBe(false);
+ });
+ });
+
+ describe("onClickGenerateReport", () => {
+ let reportService: ReportGenerationService;
+
+ beforeEach(() => {
+ reportService = TestBed.inject(ReportGenerationService);
+ vi.spyOn(notificationService, "blank");
+ });
+
+ afterEach(() => {
+ vi.restoreAllMocks();
+ });
+
+ it("orders the operator results by operator id and blanks the ones the
backend omitted", () => {
+ vi.spyOn(workflowActionService, "getWorkflowContent").mockReturnValue({
+ operators: [{ operatorID: "op-1" }, { operatorID: "op-2" }],
+ links: [],
+ commentBoxes: [],
+ settings: {},
+ } as unknown as WorkflowContent);
+ vi.spyOn(reportService,
"generateWorkflowSnapshot").mockReturnValue(of("snap-url"));
+ vi.spyOn(reportService, "getAllOperatorResults").mockReturnValue(of([{
operatorId: "op-1", html: "<b>x</b>" }]));
+ const htmlSpy = vi.spyOn(reportService,
"generateReportAsHtml").mockImplementation(() => {});
+ const successSpy = vi.spyOn(notificationService,
"success").mockImplementation(() => {});
+ const removeSpy = vi.spyOn(notificationService,
"remove").mockImplementation(() => {});
+ const errorSpy = vi.spyOn(notificationService,
"error").mockImplementation(() => {});
+ component.currentWorkflowName = "wf";
+
+ component.onClickGenerateReport();
+
+ expect(htmlSpy).toHaveBeenCalledWith("snap-url", ["<b>x</b>", ""], "wf");
+ expect(removeSpy).toHaveBeenCalledTimes(1);
+ expect(successSpy).toHaveBeenCalledWith("Report successfully
generated.");
+ expect(errorSpy).not.toHaveBeenCalled();
+ });
+
+ it("reports a failure to retrieve the operator results and closes the
notification", () => {
+ vi.spyOn(reportService,
"generateWorkflowSnapshot").mockReturnValue(of("snap-url"));
+ vi.spyOn(reportService,
"getAllOperatorResults").mockReturnValue(throwError(() => new Error("no
results")));
+ const htmlSpy = vi.spyOn(reportService,
"generateReportAsHtml").mockImplementation(() => {});
+ const removeSpy = vi.spyOn(notificationService,
"remove").mockImplementation(() => {});
+ const errorSpy = vi.spyOn(notificationService,
"error").mockImplementation(() => {});
+
+ component.onClickGenerateReport();
+
+ expect(errorSpy).toHaveBeenCalledWith("Error in retrieving operator
results: no results");
+ expect(removeSpy).toHaveBeenCalledTimes(1);
+ expect(htmlSpy).not.toHaveBeenCalled();
+ });
+
+ it("reports a failure to take the workflow snapshot without asking for
results", () => {
+ vi.spyOn(reportService, "generateWorkflowSnapshot").mockReturnValue(
+ throwError(() => new Error("snapshot failed"))
+ );
+ const resultsSpy = vi.spyOn(reportService, "getAllOperatorResults");
+ const removeSpy = vi.spyOn(notificationService,
"remove").mockImplementation(() => {});
+ const errorSpy = vi.spyOn(notificationService,
"error").mockImplementation(() => {});
+
+ component.onClickGenerateReport();
+
+ expect(errorSpy).toHaveBeenCalledWith("snapshot failed");
+ expect(removeSpy).toHaveBeenCalledTimes(1);
+ expect(resultsSpy).not.toHaveBeenCalled();
+ });
+ });
+
+ describe("onClickEditDescription", () => {
+ /** Opens the modal over a workflow with `description`, with the editor
emitting `edited`. */
+ function open(description: string | undefined, edited: string) {
+ vi.spyOn(workflowActionService, "getWorkflow").mockReturnValue({
+ content: { operators: [], links: [], commentBoxes: [], settings: {} }
as unknown as WorkflowContent,
+ name: "wf",
+ description,
+ wid: 1,
+ creationTime: undefined,
+ lastModifiedTime: undefined,
+ readonly: false,
+ isPublished: 0,
+ });
+ const close = vi.fn();
+ const createSpy = vi.spyOn(modalService, "create").mockReturnValue({
+ afterClose: of(undefined),
+ getContentComponent: () => ({ descriptionChange: of(edited) }),
+ close,
+ } as unknown as NzModalRef);
+ return { close, createSpy };
+ }
+
+ afterEach(() => {
+ vi.restoreAllMocks();
+ });
+
+ it("seeds the editor with an empty string when the workflow has no
description", () => {
+ const { createSpy } = open(undefined, "ignored");
+
+ component.onClickEditDescription();
+
+ expect((createSpy.mock.calls[0][0] as ModalOptions).nzData).toEqual({
description: "" });
+ });
+
+ it("stores the edited description, persists it while logged in, and closes
the modal", () => {
+ const { close } = open("old", "new description");
+ const metadataSpy = vi.spyOn(workflowActionService,
"setWorkflowMetadata").mockImplementation(() => {});
+ const persistSpy = vi.spyOn(component,
"persistWorkflow").mockImplementation(() => {});
+
+ component.onClickEditDescription();
+
+ expect(metadataSpy).toHaveBeenCalledWith(expect.objectContaining({ wid:
1, description: "new description" }));
+ expect(persistSpy).toHaveBeenCalledTimes(1);
+ expect(close).toHaveBeenCalledTimes(1);
+ });
+
+ it("closes the modal without persisting when logged out", () => {
+ const { close } = open("old", "new description");
+ vi.spyOn(workflowActionService,
"setWorkflowMetadata").mockImplementation(() => {});
+ const persistSpy = vi.spyOn(component,
"persistWorkflow").mockImplementation(() => {});
+ vi.spyOn(component.userService, "isLogin").mockReturnValue(false);
+
+ component.onClickEditDescription();
+
+ expect(persistSpy).not.toHaveBeenCalled();
+ expect(close).toHaveBeenCalledTimes(1);
+ });
+ });
+
+ describe("persistWorkflow", () => {
+ const saved = (wid?: number) => ({
+ content: { operators: [], links: [], commentBoxes: [], settings: {} } as
unknown as WorkflowContent,
+ name: "wf",
+ description: undefined,
+ wid,
+ creationTime: undefined,
+ lastModifiedTime: undefined,
+ readonly: false,
+ isPublished: 0,
+ });
+
+ afterEach(() => {
+ vi.restoreAllMocks();
+ });
+
+ it("files the saved workflow under the open project when both ids are
known", () => {
+ const userProjectService = TestBed.inject(UserProjectService);
+ vi.spyOn(workflowPersistService,
"persistWorkflow").mockReturnValue(of(saved(9)));
+ const metadataSpy = vi.spyOn(workflowActionService,
"setWorkflowMetadata").mockImplementation(() => {});
+ const addSpy = vi.spyOn(userProjectService,
"addWorkflowToProject").mockReturnValue(of(new Response()));
+ component.pid = 3;
+
+ component.persistWorkflow();
+
+ expect(metadataSpy).toHaveBeenCalledWith(expect.objectContaining({ wid:
9 }));
+ expect(addSpy).toHaveBeenCalledWith(3, 9);
+ expect(component.isSaving).toBe(false);
Review Comment:
The test doesn’t use the value emitted by `addWorkflowToProject`, so
returning `of(new Response())` adds an unnecessary dependency on the global
`Response` implementation. Prefer returning a minimal value aligned to the
method’s actual return type (e.g., `of(undefined)` if it’s `Observable<void>`,
or `of({} as Response)`/`of({} as any)` if it requires a response object).
##########
frontend/src/app/workspace/component/menu/menu.component.spec.ts:
##########
@@ -1216,4 +1358,293 @@ describe("MenuComponent", () => {
});
});
});
+
+ describe("ngOnInit subscriptions", () => {
+ afterEach(() => {
+ vi.restoreAllMocks();
+ });
+
+ it("re-applies the run button behavior on every execution state event", ()
=> {
+ const stateEvents$ = new Subject<{ current: { state: ExecutionState }
}>();
+ vi.spyOn(executeWorkflowService,
"getExecutionStateStream").mockReturnValue(
+ stateEvents$.asObservable() as ReturnType<typeof
executeWorkflowService.getExecutionStateStream>
+ );
+ const stateFixture = TestBed.createComponent(MenuComponent);
+ const stateComponent = stateFixture.componentInstance;
+ stateFixture.detectChanges();
+ stateComponent.isWorkflowValid = true;
+ stateComponent.isWorkflowEmpty = false;
+ stateComponent.computingUnitStatus = ComputingUnitState.Running;
+ Object.defineProperty(stateComponent.workflowWebsocketService,
"isConnected", {
+ get: () => true,
+ configurable: true,
+ });
+
+ try {
+ stateEvents$.next({ current: { state: ExecutionState.Running } });
+ expect(stateComponent.executionState).toBe(ExecutionState.Running);
+ expect(stateComponent.runButtonText).toBe("Pause");
+
+ stateEvents$.next({ current: { state: ExecutionState.Paused } });
+ expect(stateComponent.runButtonText).toBe("Resume");
+ } finally {
+ stateFixture.destroy();
+ }
+ });
+
+ it("deactivates the export button unless the feature is on and results
exist", () => {
+ const guiConfig = TestBed.inject(GuiConfigService);
+ const results$ =
component.workflowResultExportService.hasResultToExportOnAllOperators;
+
+ // Feature off: deactivated whatever the results say.
+ guiConfig.env.exportExecutionResultEnabled = false;
+ results$.next(true);
+ expect(component.isExportDeactivate).toBe(true);
+
+ // Feature on, but nothing to export.
+ guiConfig.env.exportExecutionResultEnabled = true;
+ results$.next(false);
+ expect(component.isExportDeactivate).toBe(true);
+
+ results$.next(true);
+ expect(component.isExportDeactivate).toBe(false);
+ });
+ });
+
+ describe("onClickGenerateReport", () => {
+ let reportService: ReportGenerationService;
+
+ beforeEach(() => {
+ reportService = TestBed.inject(ReportGenerationService);
+ vi.spyOn(notificationService, "blank");
+ });
+
+ afterEach(() => {
+ vi.restoreAllMocks();
+ });
+
+ it("orders the operator results by operator id and blanks the ones the
backend omitted", () => {
+ vi.spyOn(workflowActionService, "getWorkflowContent").mockReturnValue({
+ operators: [{ operatorID: "op-1" }, { operatorID: "op-2" }],
+ links: [],
+ commentBoxes: [],
+ settings: {},
+ } as unknown as WorkflowContent);
+ vi.spyOn(reportService,
"generateWorkflowSnapshot").mockReturnValue(of("snap-url"));
+ vi.spyOn(reportService, "getAllOperatorResults").mockReturnValue(of([{
operatorId: "op-1", html: "<b>x</b>" }]));
+ const htmlSpy = vi.spyOn(reportService,
"generateReportAsHtml").mockImplementation(() => {});
+ const successSpy = vi.spyOn(notificationService,
"success").mockImplementation(() => {});
+ const removeSpy = vi.spyOn(notificationService,
"remove").mockImplementation(() => {});
+ const errorSpy = vi.spyOn(notificationService,
"error").mockImplementation(() => {});
+ component.currentWorkflowName = "wf";
+
+ component.onClickGenerateReport();
+
+ expect(htmlSpy).toHaveBeenCalledWith("snap-url", ["<b>x</b>", ""], "wf");
+ expect(removeSpy).toHaveBeenCalledTimes(1);
+ expect(successSpy).toHaveBeenCalledWith("Report successfully
generated.");
+ expect(errorSpy).not.toHaveBeenCalled();
+ });
+
+ it("reports a failure to retrieve the operator results and closes the
notification", () => {
+ vi.spyOn(reportService,
"generateWorkflowSnapshot").mockReturnValue(of("snap-url"));
+ vi.spyOn(reportService,
"getAllOperatorResults").mockReturnValue(throwError(() => new Error("no
results")));
+ const htmlSpy = vi.spyOn(reportService,
"generateReportAsHtml").mockImplementation(() => {});
+ const removeSpy = vi.spyOn(notificationService,
"remove").mockImplementation(() => {});
+ const errorSpy = vi.spyOn(notificationService,
"error").mockImplementation(() => {});
+
+ component.onClickGenerateReport();
+
+ expect(errorSpy).toHaveBeenCalledWith("Error in retrieving operator
results: no results");
+ expect(removeSpy).toHaveBeenCalledTimes(1);
+ expect(htmlSpy).not.toHaveBeenCalled();
+ });
+
+ it("reports a failure to take the workflow snapshot without asking for
results", () => {
+ vi.spyOn(reportService, "generateWorkflowSnapshot").mockReturnValue(
+ throwError(() => new Error("snapshot failed"))
+ );
+ const resultsSpy = vi.spyOn(reportService, "getAllOperatorResults");
+ const removeSpy = vi.spyOn(notificationService,
"remove").mockImplementation(() => {});
+ const errorSpy = vi.spyOn(notificationService,
"error").mockImplementation(() => {});
+
+ component.onClickGenerateReport();
+
+ expect(errorSpy).toHaveBeenCalledWith("snapshot failed");
+ expect(removeSpy).toHaveBeenCalledTimes(1);
+ expect(resultsSpy).not.toHaveBeenCalled();
+ });
+ });
+
+ describe("onClickEditDescription", () => {
+ /** Opens the modal over a workflow with `description`, with the editor
emitting `edited`. */
+ function open(description: string | undefined, edited: string) {
+ vi.spyOn(workflowActionService, "getWorkflow").mockReturnValue({
+ content: { operators: [], links: [], commentBoxes: [], settings: {} }
as unknown as WorkflowContent,
+ name: "wf",
+ description,
+ wid: 1,
+ creationTime: undefined,
+ lastModifiedTime: undefined,
+ readonly: false,
+ isPublished: 0,
+ });
+ const close = vi.fn();
+ const createSpy = vi.spyOn(modalService, "create").mockReturnValue({
+ afterClose: of(undefined),
+ getContentComponent: () => ({ descriptionChange: of(edited) }),
+ close,
+ } as unknown as NzModalRef);
+ return { close, createSpy };
+ }
+
+ afterEach(() => {
+ vi.restoreAllMocks();
+ });
+
+ it("seeds the editor with an empty string when the workflow has no
description", () => {
+ const { createSpy } = open(undefined, "ignored");
+
+ component.onClickEditDescription();
+
+ expect((createSpy.mock.calls[0][0] as ModalOptions).nzData).toEqual({
description: "" });
+ });
+
+ it("stores the edited description, persists it while logged in, and closes
the modal", () => {
+ const { close } = open("old", "new description");
+ const metadataSpy = vi.spyOn(workflowActionService,
"setWorkflowMetadata").mockImplementation(() => {});
+ const persistSpy = vi.spyOn(component,
"persistWorkflow").mockImplementation(() => {});
+
+ component.onClickEditDescription();
+
+ expect(metadataSpy).toHaveBeenCalledWith(expect.objectContaining({ wid:
1, description: "new description" }));
+ expect(persistSpy).toHaveBeenCalledTimes(1);
+ expect(close).toHaveBeenCalledTimes(1);
+ });
Review Comment:
This test asserts the “logged in” behavior but doesn’t explicitly force the
logged-in condition. For determinism (and to avoid relying on default fixture
state), stub `component.userService.isLogin()` to return `true` in this test,
mirroring the explicit `false` stub in the logged-out test.
--
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]
