This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/texera.git
commit cd28761ba02f80b9028599f045a1326f7bc54a85 Author: Xinyuan Lin <[email protected]> AuthorDate: Tue Aug 4 17:46:59 2026 -0700 test(frontend): cover the codearea template and breakpoint popup placement (#7304) ### What changes were proposed in this PR? Two workspace editor sub-components whose specs stopped early. | Component | Before | Tests now | |---|---|---| | `codearea-custom-template` | 64.5%, one "should create" test | 6 | | `breakpoint-condition-input` | 63.9%, condition + keyboard paths | 13 | **`CodeareaCustomTemplateComponent`** had a single creation test. Its actual job is keeping one shared, per-operator "is the editor open" flag in sync across three inputs — this component opening or closing the editor, a co-editor opening the same one remotely, and the component being torn down while the editor is still up. The flag lives in `CodeEditorService` keyed by operator id, so a mix-up there silently reopens the wrong operator's editor. Also pinned: `ngOnDestroy` persists the **current** flag rather than a hardcoded false (so a component torn down with its editor up comes back open), and the created editor is handed **this field's** form control rather than a fresh one — otherwise the dialog opens detached from the property it is meant to edit. The co-editor test stubs `getCoeditorOpenedCodeEditorSubject` and builds its own fixture, because the component subscribes in its **constructor** and that getter returns `asObservable()`. Casting the result back to a `Subject` would only work by accident of the current implementation — the same trap that came up in review on the mini-map spec. **`BreakpointConditionInputComponent`**'s popup has no layout of its own; it is positioned by arithmetic over Monaco's reported geometry, and none of `left()`, `top()`, `isVisible` or the css offsets written by `ngOnChanges` was tested. The existing stub supplies distinguishable non-zero values, so every new expectation is a specific number — ``` left() = 30 (rect.left) + 10 (glyphMarginLeft) - 0 (scrollLeft) - 160 (popup width) = -120 top() = 20 (rect.top) + 40 (line bottom) - 5 (scrollTop) = 55 ``` — rather than a zero that jsdom would produce anyway. Also covers both the no-editor and no-line guards, and the horizontal scroll offset that only `left()` reads (so a copy-paste of `top()`'s body would be caught). `code-debugger.component.ts` (20 missed) is deliberately out of scope: its spec already has 16 tests covering the decoration and status-change handlers, and the residue looked like it would need padding rather than real assertions. Said plainly rather than quietly skipped. No production file is touched. ### Any related issues, documentation, discussions? Closes #7301 ### How was this PR tested? ``` npx ng test --watch=false --include="**/codearea-custom-template.component.spec.ts" --include="**/breakpoint-condition-input.component.spec.ts" ``` ``` ✓ .../breakpoint-condition-input.component.spec.ts (13 tests) ✓ .../codearea-custom-template.component.spec.ts (6 tests) Test Files 2 passed (2) ``` `yarn format:ci` passes (prettier-eslint + eslint), which Vitest does not cover on its own — it flagged both files on the first attempt. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --------- Signed-off-by: Xinyuan Lin <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> --- .../breakpoint-condition-input.component.spec.ts | 83 +++++++++++++++ .../codearea-custom-template.component.spec.ts | 116 +++++++++++++++++++++ 2 files changed, 199 insertions(+) diff --git a/frontend/src/app/workspace/component/code-editor-dialog/breakpoint-condition-input/breakpoint-condition-input.component.spec.ts b/frontend/src/app/workspace/component/code-editor-dialog/breakpoint-condition-input/breakpoint-condition-input.component.spec.ts index 2566f86e0e..43d891c2c5 100644 --- a/frontend/src/app/workspace/component/code-editor-dialog/breakpoint-condition-input/breakpoint-condition-input.component.spec.ts +++ b/frontend/src/app/workspace/component/code-editor-dialog/breakpoint-condition-input/breakpoint-condition-input.component.spec.ts @@ -71,6 +71,89 @@ describe("BreakpointConditionInputComponent", () => { component.closeEmitter.emit(); }); + /** + * The popup has no layout of its own: it is positioned by arithmetic over the Monaco editor's + * reported geometry. The stub above supplies distinguishable non-zero values + * (glyphMarginLeft 10, editor rect top 20 / left 30, line bottom 40, scrollTop 5), so every + * expectation below is a specific number rather than a zero that jsdom would produce anyway. + */ + describe("popup placement", () => { + it("offsets left by the glyph margin and the fixed popup width", () => { + // 30 (rect.left) + 10 (glyphMarginLeft) - 0 (scrollLeft) - 160 (popup width) + expect(component.left()).toBe(-120); + }); + + it("places the top at the bottom of the target line, less the scroll offset", () => { + // 20 (rect.top) + 40 (line bottom) - 5 (scrollTop) + expect(component.top()).toBe(55); + }); + + it("subtracts the horizontal scroll offset", () => { + // Only left() reads scrollLeft; a copy-paste of top()'s body would miss this. + component.monacoEditor = { + ...component.monacoEditor, + getScrollLeft: () => 25, + } as unknown as editor.IStandaloneCodeEditor; + + expect(component.left()).toBe(-145); + }); + + it("falls back to 0 rather than throwing when there is no editor yet", () => { + const stub = component.monacoEditor; + component.monacoEditor = undefined as unknown as editor.IStandaloneCodeEditor; + try { + // The guards exist because the popup can be rendered a tick before the editor is attached. + expect(component.left()).toBe(0); + expect(component.top()).toBe(0); + } finally { + // afterEach disposes the editor, so put the stub back. + component.monacoEditor = stub; + } + }); + + it("falls back to 0 for top() when no line is targeted", () => { + component.lineNum = undefined; + expect(component.top()).toBe(0); + // left() does not depend on the line, so it still resolves. + expect(component.left()).toBe(-120); + }); + + it("writes both css offsets when the target line changes", () => { + mockUdfDebugService.getCondition.mockReturnValue("x > 1"); + component.lineNum = 3; + const changes: SimpleChanges = { + lineNum: { currentValue: 3, previousValue: 1, firstChange: false, isFirstChange: () => false }, + }; + + component.ngOnChanges(changes); + + expect(component.topPosition).toBe("55px"); + expect(component.leftPosition).toBe("-120px"); + }); + }); + + describe("visibility", () => { + it("is visible only while a line is targeted", () => { + // The template keys its *ngIf on this, so an inverted getter leaves the popup stuck open. + expect(component.isVisible).toBe(true); + + component.lineNum = undefined; + expect(component.isVisible).toBe(false); + }); + }); + + it("ignores a keypress when no line is targeted", () => { + component.lineNum = undefined; + const emitted = vi.fn(); + component.closeEmitter.subscribe(emitted); + + component.handleEvent(new KeyboardEvent("keydown", { key: "Enter" })); + + // Neither saved nor closed: with no line there is nothing to attach a condition to. + expect(mockUdfDebugService.doUpdateBreakpointCondition).not.toHaveBeenCalled(); + expect(emitted).not.toHaveBeenCalled(); + }); + it("should create the component", () => { expect(component).toBeTruthy(); }); diff --git a/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.spec.ts b/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.spec.ts index d94217f7c2..6b1a81cc21 100644 --- a/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.spec.ts +++ b/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.spec.ts @@ -25,6 +25,9 @@ import { OperatorMetadataService } from "../../service/operator-metadata/operato import { StubOperatorMetadataService } from "../../service/operator-metadata/stub-operator-metadata.service"; import { FormControl } from "@angular/forms"; import { commonTestProviders } from "../../../common/testing/test-utils"; +import { CodeEditorService } from "../../service/code-editor/code-editor.service"; +import { CoeditorPresenceService } from "../../service/workflow-graph/model/coeditor-presence.service"; +import { Subject } from "rxjs"; describe("CodeareaCustomTemplateComponent", () => { let component: CodeareaCustomTemplateComponent; @@ -52,4 +55,117 @@ describe("CodeareaCustomTemplateComponent", () => { it("should create", () => { expect(component).toBeTruthy(); }); + + /** + * The component's whole job is to keep one shared, per-operator "is the editor open" flag in + * sync across three inputs: this component's own open/close, the same operator's editor opened + * by a co-editor, and the component being torn down while the editor is still up. The flag lives + * in CodeEditorService keyed by operator id, so a mix-up there silently reopens (or fails to + * reopen) the wrong operator's editor. + * + * `openEditor` is driven directly rather than through the template button: it calls + * `codeEditorService.vc.createComponent`, and `vc` is a ViewContainerRef the real workspace + * supplies. The tests below install a fake one so the component's own bookkeeping is observable + * without standing up a Monaco editor. + */ + describe("editor-open state", () => { + let codeEditorService: CodeEditorService; + let destroyCallbacks: (() => void)[]; + + // The highlighted operator is what getOperatorID reads, and it is the key every + // setEditorState/getEditorState call is scoped by. + function highlightedOperatorId(): string { + return TestBed.inject(WorkflowActionService).getJointGraphWrapper().getCurrentHighlightedOperatorIDs()[0]; + } + + beforeEach(() => { + const workflowActionService = TestBed.inject(WorkflowActionService); + vi.spyOn(workflowActionService.getJointGraphWrapper(), "getCurrentHighlightedOperatorIDs").mockReturnValue([ + "test-operator-id", + ]); + // ngOnInit reads the highlighted operator id; re-run it so operatorID is deterministic for these tests. + component.ngOnInit(); + + codeEditorService = TestBed.inject(CodeEditorService); + destroyCallbacks = []; + // Stand-in for the workspace's ViewContainerRef. Records the onDestroy hook the component + // registers so the close path can be triggered without a real component teardown. + codeEditorService.vc = { + createComponent: () => ({ + instance: {} as any, + onDestroy: (cb: () => void) => destroyCallbacks.push(cb), + destroy: () => destroyCallbacks.forEach(cb => cb()), + }), + } as any; + }); + + it("marks the operator's editor open and hands the field's control to it", () => { + component.openEditor(); + + expect(component.isEditorOpen).toBe(true); + // The created editor must edit THIS field, not a fresh control - otherwise the dialog opens + // detached from the operator property it is meant to edit. + expect(component.componentRef!.instance.formControl).toBe(component.field.formControl); + + let published: boolean | undefined; + codeEditorService.getEditorState(highlightedOperatorId()).subscribe(v => (published = v)); + expect(published).toBe(true); + }); + + it("clears the flag again when the editor component is destroyed", () => { + component.openEditor(); + component.componentRef!.destroy(); + + expect(component.isEditorOpen).toBe(false); + let published: boolean | undefined; + codeEditorService.getEditorState(highlightedOperatorId()).subscribe(v => (published = v)); + expect(published).toBe(false); + }); + + it("opens the editor when a co-editor opens one", () => { + // The constructor subscribes to the co-editor stream, so the stub has to be in place before + // the component exists - hence a fresh fixture here rather than the shared one. Stubbing the + // getter (rather than casting its result back to a Subject) keeps this honest: it returns + // `asObservable()`, so a cast would only work by accident of the current implementation. + const opened = new Subject<{ operatorId: string }>(); + vi.spyOn(TestBed.inject(CoeditorPresenceService), "getCoeditorOpenedCodeEditorSubject").mockReturnValue( + opened.asObservable() + ); + + const remoteFixture = TestBed.createComponent(CodeareaCustomTemplateComponent); + const remoteComponent = remoteFixture.componentInstance; + remoteComponent.field = { props: {}, formControl: new FormControl() } as any; + remoteFixture.detectChanges(); + + expect(remoteComponent.isEditorOpen).toBe(false); + + // A remote open has to bring this client's editor up too - that is what makes the session + // collaborative rather than merely presence-aware. + opened.next({ operatorId: highlightedOperatorId() }); + + expect(remoteComponent.isEditorOpen).toBe(true); + expect(remoteComponent.componentRef).toBeDefined(); + }); + + it("persists the open flag on destroy so a reopened panel restores it", () => { + component.openEditor(); + + component.ngOnDestroy(); + + // ngOnDestroy writes the CURRENT flag rather than a hardcoded false, so a component torn + // down with its editor still up comes back open. + let published: boolean | undefined; + codeEditorService.getEditorState(highlightedOperatorId()).subscribe(v => (published = v)); + expect(published).toBe(true); + }); + + it("tracks an external state change through ngOnInit's subscription", () => { + // Another component sharing the same operator id flips the flag; this one must follow. + codeEditorService.setEditorState(highlightedOperatorId(), true); + expect(component.isEditorOpen).toBe(true); + + codeEditorService.setEditorState(highlightedOperatorId(), false); + expect(component.isEditorOpen).toBe(false); + }); + }); });
