Copilot commented on code in PR #7335:
URL: https://github.com/apache/texera/pull/7335#discussion_r3720189324
##########
frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts:
##########
@@ -431,6 +432,173 @@ describe("ContextMenuComponent", () => {
expect(component.highlightedCommentBoxIds).toEqual([]);
});
});
+ // ── Template menu-item (click) wiring ──
+ // The class methods are covered above; these render the menu and click each
+ // item so the template's *ngIf-gated (click) bindings are exercised too.
+ describe("menu item click bindings", () => {
+ /** Click the rendered <li nz-menu-item> whose text matches `label`
exactly. */
+ function clickItem(label: string): void {
+ const norm = (s: string | null) => (s ?? "").replace(/\s+/g, "
").trim().toLowerCase();
+ const items = fixture.debugElement.queryAll(By.css("li[nz-menu-item]"));
+ const item = items.find(li => norm(li.nativeElement.textContent) ===
label.toLowerCase());
+ if (!item) {
+ throw new Error(
+ `menu item "${label}" not rendered; present: [${items.map(i =>
norm(i.nativeElement.textContent)).join(" | ")}]`
+ );
+ }
+ item.triggerEventHandler("click", null);
+ }
+
+ it("copy invokes onCopy", () => {
+ highlightedOperatorsSubject.next(["op1"]);
+ fixture.detectChanges();
+ const spy = vi.spyOn(component, "onCopy").mockImplementation(() => {});
+
+ clickItem("copy");
+
+ expect(spy).toHaveBeenCalledTimes(1);
+ });
+
+ it("cut invokes onCut", () => {
+ highlightedOperatorsSubject.next(["op1"]);
+ component.isWorkflowModifiable = true;
+ fixture.detectChanges();
+ const spy = vi.spyOn(component, "onCut").mockImplementation(() => {});
+
+ clickItem("cut");
+
+ expect(spy).toHaveBeenCalledTimes(1);
+ });
+
+ it("paste invokes onPaste", () => {
+ highlightedOperatorsSubject.next([]);
+ highlightedCommentBoxesSubject.next([]);
+ component.isWorkflowModifiable = true;
+ fixture.detectChanges();
+ const spy = vi.spyOn(component, "onPaste").mockImplementation(() => {});
+
+ clickItem("paste");
+
+ expect(spy).toHaveBeenCalledTimes(1);
+ });
+
+ it("disable invokes operatorMenuService.disableHighlightedOperators", ()
=> {
+ operatorMenuService.isDisableOperator = true;
+ operatorMenuService.isDisableOperatorClickable = true;
+ fixture.detectChanges();
+
+ clickItem("disable");
+
+
expect(operatorMenuService.disableHighlightedOperators).toHaveBeenCalledTimes(1);
+ });
+
+ it("enable invokes operatorMenuService.disableHighlightedOperators", () =>
{
+ operatorMenuService.isDisableOperator = false;
+ operatorMenuService.isDisableOperatorClickable = true;
+ fixture.detectChanges();
+
+ clickItem("enable");
+
+
expect(operatorMenuService.disableHighlightedOperators).toHaveBeenCalledTimes(1);
+ });
+
+ it("view result invokes
operatorMenuService.viewResultHighlightedOperators", () => {
+ operatorMenuService.isToViewResult = true;
+ operatorMenuService.isToViewResultClickable = true;
+ fixture.detectChanges();
Review Comment:
These tests mutate shared service state (`operatorMenuService.*` flags). In
Angular TestBed, injected services are typically singletons across test cases
in the same suite, so state can leak between tests and create order-dependent
failures. Consider resetting all `operatorMenuService` flags to a known
baseline in a `beforeEach` (or setting every relevant flag explicitly in each
test) to ensure isolation.
##########
frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts:
##########
@@ -431,6 +432,173 @@ describe("ContextMenuComponent", () => {
expect(component.highlightedCommentBoxIds).toEqual([]);
});
});
+ // ── Template menu-item (click) wiring ──
+ // The class methods are covered above; these render the menu and click each
+ // item so the template's *ngIf-gated (click) bindings are exercised too.
+ describe("menu item click bindings", () => {
+ /** Click the rendered <li nz-menu-item> whose text matches `label`
exactly. */
+ function clickItem(label: string): void {
+ const norm = (s: string | null) => (s ?? "").replace(/\s+/g, "
").trim().toLowerCase();
+ const items = fixture.debugElement.queryAll(By.css("li[nz-menu-item]"));
+ const item = items.find(li => norm(li.nativeElement.textContent) ===
label.toLowerCase());
Review Comment:
The comparison normalizes the DOM text (`norm(...)`) but only lowercases the
provided `label`. This can introduce brittle mismatches if a label contains
multiple spaces/newlines or leading/trailing whitespace. Normalize the `label`
using the same `norm` function (e.g., compare `norm(textContent)` to
`norm(label)`) to keep the helper robust.
##########
frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts:
##########
@@ -431,6 +432,173 @@ describe("ContextMenuComponent", () => {
expect(component.highlightedCommentBoxIds).toEqual([]);
});
});
+ // ── Template menu-item (click) wiring ──
+ // The class methods are covered above; these render the menu and click each
+ // item so the template's *ngIf-gated (click) bindings are exercised too.
+ describe("menu item click bindings", () => {
+ /** Click the rendered <li nz-menu-item> whose text matches `label`
exactly. */
+ function clickItem(label: string): void {
+ const norm = (s: string | null) => (s ?? "").replace(/\s+/g, "
").trim().toLowerCase();
+ const items = fixture.debugElement.queryAll(By.css("li[nz-menu-item]"));
+ const item = items.find(li => norm(li.nativeElement.textContent) ===
label.toLowerCase());
+ if (!item) {
+ throw new Error(
+ `menu item "${label}" not rendered; present: [${items.map(i =>
norm(i.nativeElement.textContent)).join(" | ")}]`
+ );
+ }
+ item.triggerEventHandler("click", null);
+ }
+
+ it("copy invokes onCopy", () => {
+ highlightedOperatorsSubject.next(["op1"]);
+ fixture.detectChanges();
+ const spy = vi.spyOn(component, "onCopy").mockImplementation(() => {});
+
+ clickItem("copy");
+
+ expect(spy).toHaveBeenCalledTimes(1);
+ });
+
+ it("cut invokes onCut", () => {
+ highlightedOperatorsSubject.next(["op1"]);
+ component.isWorkflowModifiable = true;
+ fixture.detectChanges();
+ const spy = vi.spyOn(component, "onCut").mockImplementation(() => {});
+
+ clickItem("cut");
+
+ expect(spy).toHaveBeenCalledTimes(1);
+ });
+
+ it("paste invokes onPaste", () => {
+ highlightedOperatorsSubject.next([]);
+ highlightedCommentBoxesSubject.next([]);
+ component.isWorkflowModifiable = true;
+ fixture.detectChanges();
+ const spy = vi.spyOn(component, "onPaste").mockImplementation(() => {});
+
+ clickItem("paste");
+
+ expect(spy).toHaveBeenCalledTimes(1);
+ });
+
+ it("disable invokes operatorMenuService.disableHighlightedOperators", ()
=> {
+ operatorMenuService.isDisableOperator = true;
+ operatorMenuService.isDisableOperatorClickable = true;
+ fixture.detectChanges();
Review Comment:
These tests mutate shared service state (`operatorMenuService.*` flags). In
Angular TestBed, injected services are typically singletons across test cases
in the same suite, so state can leak between tests and create order-dependent
failures. Consider resetting all `operatorMenuService` flags to a known
baseline in a `beforeEach` (or setting every relevant flag explicitly in each
test) to ensure isolation.
##########
frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts:
##########
@@ -431,6 +432,173 @@ describe("ContextMenuComponent", () => {
expect(component.highlightedCommentBoxIds).toEqual([]);
});
});
+ // ── Template menu-item (click) wiring ──
+ // The class methods are covered above; these render the menu and click each
+ // item so the template's *ngIf-gated (click) bindings are exercised too.
+ describe("menu item click bindings", () => {
+ /** Click the rendered <li nz-menu-item> whose text matches `label`
exactly. */
+ function clickItem(label: string): void {
+ const norm = (s: string | null) => (s ?? "").replace(/\s+/g, "
").trim().toLowerCase();
+ const items = fixture.debugElement.queryAll(By.css("li[nz-menu-item]"));
+ const item = items.find(li => norm(li.nativeElement.textContent) ===
label.toLowerCase());
+ if (!item) {
+ throw new Error(
+ `menu item "${label}" not rendered; present: [${items.map(i =>
norm(i.nativeElement.textContent)).join(" | ")}]`
+ );
+ }
+ item.triggerEventHandler("click", null);
+ }
+
+ it("copy invokes onCopy", () => {
+ highlightedOperatorsSubject.next(["op1"]);
+ fixture.detectChanges();
+ const spy = vi.spyOn(component, "onCopy").mockImplementation(() => {});
+
+ clickItem("copy");
+
+ expect(spy).toHaveBeenCalledTimes(1);
+ });
+
+ it("cut invokes onCut", () => {
+ highlightedOperatorsSubject.next(["op1"]);
+ component.isWorkflowModifiable = true;
+ fixture.detectChanges();
+ const spy = vi.spyOn(component, "onCut").mockImplementation(() => {});
+
+ clickItem("cut");
+
+ expect(spy).toHaveBeenCalledTimes(1);
+ });
+
+ it("paste invokes onPaste", () => {
+ highlightedOperatorsSubject.next([]);
+ highlightedCommentBoxesSubject.next([]);
+ component.isWorkflowModifiable = true;
+ fixture.detectChanges();
+ const spy = vi.spyOn(component, "onPaste").mockImplementation(() => {});
+
+ clickItem("paste");
+
+ expect(spy).toHaveBeenCalledTimes(1);
+ });
+
+ it("disable invokes operatorMenuService.disableHighlightedOperators", ()
=> {
+ operatorMenuService.isDisableOperator = true;
+ operatorMenuService.isDisableOperatorClickable = true;
+ fixture.detectChanges();
+
+ clickItem("disable");
+
+
expect(operatorMenuService.disableHighlightedOperators).toHaveBeenCalledTimes(1);
+ });
+
+ it("enable invokes operatorMenuService.disableHighlightedOperators", () =>
{
+ operatorMenuService.isDisableOperator = false;
+ operatorMenuService.isDisableOperatorClickable = true;
+ fixture.detectChanges();
+
+ clickItem("enable");
+
+
expect(operatorMenuService.disableHighlightedOperators).toHaveBeenCalledTimes(1);
+ });
+
+ it("view result invokes
operatorMenuService.viewResultHighlightedOperators", () => {
+ operatorMenuService.isToViewResult = true;
+ operatorMenuService.isToViewResultClickable = true;
+ fixture.detectChanges();
+
+ clickItem("view result");
+
+
expect(operatorMenuService.viewResultHighlightedOperators).toHaveBeenCalledTimes(1);
+ });
+
+ it("remove view result invokes
operatorMenuService.viewResultHighlightedOperators", () => {
+ operatorMenuService.isToViewResult = false;
+ operatorMenuService.isToViewResultClickable = true;
+ fixture.detectChanges();
+
+ clickItem("remove view result");
+
+
expect(operatorMenuService.viewResultHighlightedOperators).toHaveBeenCalledTimes(1);
+ });
+
+ it("renders the reuse result item (disabled) when marked for reuse", () =>
{
+ // This entry is hardcoded `nzDisabled`, so it can't be clicked; assert
it renders.
+ // Its (click) handler is the same as "remove reusing result", covered
by the next test.
+ operatorMenuService.isMarkForReuse = true;
+ operatorMenuService.isReuseResultClickable = true;
+ fixture.detectChanges();
+
+ const rendered = fixture.debugElement
+ .queryAll(By.css("li[nz-menu-item]"))
+ .some(li => (li.nativeElement.textContent ?? "").trim().toLowerCase()
=== "reuse result");
+ expect(rendered).toBe(true);
+ });
+
+ it("remove reusing result invokes
operatorMenuService.reuseResultHighlightedOperator", () => {
+ operatorMenuService.isMarkForReuse = false;
+ operatorMenuService.isReuseResultClickable = true;
+ fixture.detectChanges();
+
+ clickItem("remove reusing result");
+
+
expect(operatorMenuService.reuseResultHighlightedOperator).toHaveBeenCalledTimes(1);
+ });
+
+ it("delete invokes onDelete when operators are highlighted", () => {
+ highlightedOperatorsSubject.next(["op1"]);
+ component.isWorkflowModifiable = true;
+ fixture.detectChanges();
+ const spy = vi.spyOn(component, "onDelete").mockImplementation(() => {});
+
+ clickItem("delete");
+
+ expect(spy).toHaveBeenCalledTimes(1);
+ });
+
+ it("delete invokes onDelete for a links-only selection", () => {
+ highlightedOperatorsSubject.next([]);
+ highlightedCommentBoxesSubject.next([]);
+
jointGraphWrapperSpy.getCurrentHighlightedLinkIDs.mockReturnValue(["link1"]);
+ component.isWorkflowModifiable = true;
+ fixture.detectChanges();
+ const spy = vi.spyOn(component, "onDelete").mockImplementation(() => {});
+
+ clickItem("delete");
+
+ expect(spy).toHaveBeenCalledTimes(1);
+ });
+
+ it("execute to this operator invokes executeUpToOperator", () => {
+ highlightedOperatorsSubject.next(["op1"]);
+ component.isWorkflowModifiable = true;
+
jointGraphWrapperSpy.getCurrentHighlightedOperatorIDs.mockReturnValue(["op1"]);
+ validationWorkflowService.validateOperator.mockReturnValue({ isValid:
true });
+ (workflowActionService.getTexeraGraph() as unknown as
Mocked<WorkflowGraph>).isOperatorDisabled.mockReturnValue(
+ false
+ );
+ fixture.detectChanges();
+ expect(component.canExecuteOperator()).toBe(true); // item is enabled
+
+ clickItem("execute to this operator");
+
+ expect(operatorMenuService.executeUpToOperator).toHaveBeenCalledTimes(1);
+ });
+
+ it("Export result invokes onClickExportHighlightedExecutionResult", () => {
+ (
+ workflowResultExportService as unknown as {
hasResultToExportOnHighlightedOperators: boolean }
+ ).hasResultToExportOnHighlightedOperators = true;
+ (component as unknown as { config: { env: Record<string, unknown> }
}).config.env.exportExecutionResultEnabled =
+ true;
Review Comment:
The test relies on `unknown` casts to mutate likely-private/internal fields
(`component.config.env...`) and a loosely-typed service property, which makes
the test brittle to refactors and typing changes. Prefer configuring these via
the TestBed provider setup (e.g., providing a config token/mock) and/or using
spies on a public method/getter on `workflowResultExportService` if available,
so the test stays resilient and type-safe.
##########
frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts:
##########
@@ -431,6 +432,173 @@ describe("ContextMenuComponent", () => {
expect(component.highlightedCommentBoxIds).toEqual([]);
});
});
+ // ── Template menu-item (click) wiring ──
+ // The class methods are covered above; these render the menu and click each
+ // item so the template's *ngIf-gated (click) bindings are exercised too.
+ describe("menu item click bindings", () => {
+ /** Click the rendered <li nz-menu-item> whose text matches `label`
exactly. */
+ function clickItem(label: string): void {
+ const norm = (s: string | null) => (s ?? "").replace(/\s+/g, "
").trim().toLowerCase();
+ const items = fixture.debugElement.queryAll(By.css("li[nz-menu-item]"));
+ const item = items.find(li => norm(li.nativeElement.textContent) ===
label.toLowerCase());
+ if (!item) {
+ throw new Error(
+ `menu item "${label}" not rendered; present: [${items.map(i =>
norm(i.nativeElement.textContent)).join(" | ")}]`
+ );
+ }
+ item.triggerEventHandler("click", null);
+ }
Review Comment:
`DebugElement.triggerEventHandler("click", ...)` bypasses real DOM event
dispatch and can ignore framework/directive behavior (notably around disabled
menu items), which can make these tests less representative of actual user
clicks. Prefer triggering a real click via the native element (e.g.,
`item.nativeElement.click()` or dispatching a `MouseEvent`) so
`nz-menu-item`/`nzDisabled` behavior is exercised the same way it is in the
browser.
--
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]
