mengw15 commented on code in PR #7335:
URL: https://github.com/apache/texera/pull/7335#discussion_r3720295471
##########
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:
Done — hoisted `norm` and normalize both sides (`norm(textContent) ===
norm(label)`); applied it to the reuse-result render check too.
##########
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:
This is the repo's documented pattern — `frontend/TESTING.md` Recipe C
prefers `triggerEventHandler("click", null)` over `nativeElement.click()` (it
goes through the Angular renderer and survives renderer differences). The one
genuinely-disabled item (`nzDisabled` "reuse result") is deliberately NOT
clicked — it only asserts render, and its handler is exercised via "remove
reusing result" — so no disabled-item behavior is bypassed.
##########
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:
`operatorMenuService` is a fresh object literal built in the outer
`beforeEach` and re-provided to `TestBed.configureTestingModule` for every
test, so it isn't a shared singleton across cases — no state leaks. Each test
also sets the flags it depends on explicitly before `detectChanges`, so there's
no reliance on leftover state.
##########
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:
Same as the sibling thread: `operatorMenuService` is rebuilt in the outer
`beforeEach` and re-provided per test (not a shared singleton), and every test
sets the flags it needs explicitly — so there's no cross-test leakage.
##########
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:
This flips an internal feature flag
(`config.env.exportExecutionResultEnabled`) so the export item renders; it's
deterministic and scoped to this single test. There's no public setter for that
env flag, so a provider-based mock wouldn't be meaningfully less coupled to the
internal shape — kept the direct set.
--
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]
