This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-5191-cbe90c74c479b8b29b1a02a71a29c1bb17e7d7bd in repository https://gitbox.apache.org/repos/asf/texera.git
commit a25c54348d95dbc149ca3f48db0c061623c88b8c Author: Yicong Huang <[email protected]> AuthorDate: Sun May 24 15:54:35 2026 -0700 test(frontend): extend code-debugger and code-editor coverage (#5191) ### What changes were proposed in this PR? Extends the existing `code-debugger.component.spec.ts` and `code-editor.component.spec.ts` with tests for the class-level surface that the original specs left behind. No Monaco-editor interaction is added — the parts that require a real editor are left for a future browser-mode pass. A latent test-setup bug surfaced while writing the new debugger tests: the spec built `debugState` from `new Y.Map<…>()` directly, which silently never fires observers because the map is not attached to a Y.Doc. Production never hits this — `UdfDebugService` hands out maps from a real doc — but every Y.Map observer test would have stayed at 0 coverage until the map was wired into a doc. Switching to `new Y.Doc().getMap(…)` fixes it. ### Any related issues, documentation, discussions? Closes #5190. ### How was this PR tested? `yarn ng test --watch=false --include=…` runs 2 spec files / 39 tests, all green. `yarn lint` and `yarn format:ci` both clean. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Opus 4.7 --- .../code-debugger.component.spec.ts | 155 ++++++++++++++++++++- .../code-editor.component.spec.ts | 141 ++++++++++++++++++- 2 files changed, 294 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/workspace/component/code-editor-dialog/code-debugger.component.spec.ts b/frontend/src/app/workspace/component/code-editor-dialog/code-debugger.component.spec.ts index abd76975e0..cac2331de8 100644 --- a/frontend/src/app/workspace/component/code-editor-dialog/code-debugger.component.spec.ts +++ b/frontend/src/app/workspace/component/code-editor-dialog/code-debugger.component.spec.ts @@ -45,7 +45,12 @@ describe("CodeDebuggerComponent", () => { beforeEach(async () => { // Initialize streams and spy objects statusUpdateStream = new Subject<Record<string, OperatorStatistics>>(); - debugState = new Y.Map<BreakpointInfo>(); + // Y.Map observers only fire when the map is attached to a Y.Doc (the doc + // owns the transaction lifecycle that drives observation). A standalone + // `new Y.Map()` accepts `.set()` but never notifies observers — production + // never hits this because `UdfDebugService` hands out maps from a real + // doc, but the spec used to construct a detached map. + debugState = new Y.Doc().getMap<BreakpointInfo>("debug"); mockWorkflowStatusService = { getStatusUpdateStream: vi.fn() } as unknown as Mocked<WorkflowStatusService>; mockWorkflowStatusService.getStatusUpdateStream.mockReturnValue(statusUpdateStream.asObservable()); @@ -238,4 +243,152 @@ describe("CodeDebuggerComponent", () => { expect(component.breakpointConditionLine).toBeUndefined(); }); + + describe("registerBreakpointRenderingHandler", () => { + // Stand-in for the MonacoBreakpoint instance set up in + // setupMonacoBreakpointMethods. The observer in + // registerBreakpointRenderingHandler reaches into these as bracket + // properties, so we wire up the four it touches: createSpecifyDecoration, + // removeSpecifyDecoration, setLineHighlight, removeHighlight, plus the + // lineNumberAndDecorationIdMap reads by removeBreakpointDecoration. + let monacoBreakpointStub: { + createSpecifyDecoration: ReturnType<typeof vi.fn>; + removeSpecifyDecoration: ReturnType<typeof vi.fn>; + setLineHighlight: ReturnType<typeof vi.fn>; + removeHighlight: ReturnType<typeof vi.fn>; + lineNumberAndDecorationIdMap: Map<number, string>; + mouseDownDisposable: { dispose: ReturnType<typeof vi.fn> }; + dispose: ReturnType<typeof vi.fn>; + }; + + beforeEach(() => { + monacoBreakpointStub = { + createSpecifyDecoration: vi.fn(), + removeSpecifyDecoration: vi.fn(), + setLineHighlight: vi.fn(), + removeHighlight: vi.fn(), + lineNumberAndDecorationIdMap: new Map<number, string>(), + mouseDownDisposable: { dispose: vi.fn() }, + dispose: vi.fn(), + }; + component.monacoBreakpoint = monacoBreakpointStub as unknown as MonacoBreakpoint; + }); + + it("draws a decoration when a debug-state entry with breakpointId is added", () => { + debugState.set("3", { breakpointId: 3, condition: "", hit: false } as BreakpointInfo); + + expect(monacoBreakpointStub.createSpecifyDecoration).toHaveBeenCalledWith({ + startLineNumber: 3, + startColumn: 1, + endLineNumber: 3, + endColumn: 1, + }); + }); + + it("does not draw a decoration when the added entry has no breakpointId", () => { + debugState.set("4", { breakpointId: undefined, condition: "", hit: false } as BreakpointInfo); + + expect(monacoBreakpointStub.createSpecifyDecoration).not.toHaveBeenCalled(); + }); + + it("removes the decoration when a debug-state entry with breakpointId is deleted", () => { + debugState.set("5", { breakpointId: 5, condition: "", hit: false } as BreakpointInfo); + // Mirror the lookup that removeBreakpointDecoration performs. + monacoBreakpointStub.lineNumberAndDecorationIdMap.set(5, "dec-5"); + monacoBreakpointStub.createSpecifyDecoration.mockClear(); + + debugState.delete("5"); + + expect(monacoBreakpointStub.removeSpecifyDecoration).toHaveBeenCalledWith("dec-5", 5); + }); + + it("does not call removeSpecifyDecoration when the deleted entry had no breakpointId", () => { + // Y.Map.observe surfaces a delete with oldValue.breakpointId === undefined + // when the entry never had a breakpoint to begin with. + debugState.set("6", { breakpointId: undefined, condition: "", hit: false } as BreakpointInfo); + debugState.delete("6"); + + expect(monacoBreakpointStub.removeSpecifyDecoration).not.toHaveBeenCalled(); + }); + + it("calls setLineHighlight when an entry's hit flag flips on", () => { + debugState.set("7", { breakpointId: 7, condition: "", hit: false } as BreakpointInfo); + + debugState.set("7", { breakpointId: 7, condition: "", hit: true } as BreakpointInfo); + + expect(monacoBreakpointStub.setLineHighlight).toHaveBeenCalledWith(7); + }); + + it("calls removeHighlight when an entry's hit flag flips off", () => { + debugState.set("8", { breakpointId: 8, condition: "", hit: true } as BreakpointInfo); + + debugState.set("8", { breakpointId: 8, condition: "", hit: false } as BreakpointInfo); + + expect(monacoBreakpointStub.removeHighlight).toHaveBeenCalled(); + }); + + it("re-creates the decoration when the condition string changes on an existing entry", () => { + debugState.set("9", { breakpointId: 9, condition: "", hit: false } as BreakpointInfo); + monacoBreakpointStub.lineNumberAndDecorationIdMap.set(9, "dec-9"); + monacoBreakpointStub.createSpecifyDecoration.mockClear(); + monacoBreakpointStub.removeSpecifyDecoration.mockClear(); + + debugState.set("9", { breakpointId: 9, condition: "x > 0", hit: false } as BreakpointInfo); + + expect(monacoBreakpointStub.removeSpecifyDecoration).toHaveBeenCalledWith("dec-9", 9); + expect(monacoBreakpointStub.createSpecifyDecoration).toHaveBeenCalledWith({ + startLineNumber: 9, + startColumn: 1, + endLineNumber: 9, + endColumn: 1, + }); + }); + }); + + describe("rerenderExistingBreakpoints", () => { + let monacoBreakpointStub: { + createSpecifyDecoration: ReturnType<typeof vi.fn>; + }; + + beforeEach(() => { + monacoBreakpointStub = { createSpecifyDecoration: vi.fn() }; + component.monacoBreakpoint = monacoBreakpointStub as unknown as MonacoBreakpoint; + }); + + it("re-creates a decoration for every existing entry that has a breakpointId", () => { + debugState.set("10", { breakpointId: 10, condition: "", hit: false } as BreakpointInfo); + debugState.set("11", { breakpointId: undefined, condition: "", hit: false } as BreakpointInfo); + debugState.set("12", { breakpointId: 12, condition: "y > 0", hit: false } as BreakpointInfo); + monacoBreakpointStub.createSpecifyDecoration.mockClear(); + + component.rerenderExistingBreakpoints(); + + // Only entries 10 and 12 carry a breakpointId; 11 is filtered out by the + // early-return inside rerenderExistingBreakpoints. + const lines = monacoBreakpointStub.createSpecifyDecoration.mock.calls.map(c => c[0].startLineNumber); + expect(lines.sort()).toEqual([10, 12]); + }); + }); + + describe("removeMonacoBreakpointMethods", () => { + it("disposes the mouseDownDisposable and the breakpoint instance when defined", () => { + const disposeMouseDown = vi.fn(); + const disposeBreakpoint = vi.fn(); + component.monacoBreakpoint = { + mouseDownDisposable: { dispose: disposeMouseDown }, + dispose: disposeBreakpoint, + } as unknown as MonacoBreakpoint; + + component.removeMonacoBreakpointMethods(); + + expect(disposeMouseDown).toHaveBeenCalledOnce(); + expect(disposeBreakpoint).toHaveBeenCalledOnce(); + }); + + it("returns early without touching anything when monacoBreakpoint is undefined", () => { + component.monacoBreakpoint = undefined; + // Should simply no-op. + expect(() => component.removeMonacoBreakpointMethods()).not.toThrow(); + }); + }); }); diff --git a/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.spec.ts b/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.spec.ts index b477ff5979..0f37f851aa 100644 --- a/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.spec.ts +++ b/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.spec.ts @@ -28,7 +28,9 @@ import { mockOperatorMetaData } from "../../service/operator-metadata/mock-opera import { commonTestProviders } from "../../../common/testing/test-utils"; import { OperatorPredicate } from "../../types/workflow-common.interface"; import { OperatorSchema } from "../../types/operator-schema.interface"; -import { of } from "rxjs"; +import { of, Subject } from "rxjs"; +import { AIAssistantService } from "../../service/ai-assistant/ai-assistant.service"; +import * as monaco from "monaco-editor"; // Operator types that the constructor's language-detection branch must map // to a specific language. `RUDFSource` / `RUDF` -> `r`; the three V2 Python @@ -184,4 +186,141 @@ describe("CodeEditorComponent", () => { } as any); expect(result).toBeTruthy(); }); + + describe("getFileSuffixByLanguage", () => { + // The method is private but determines the in-memory file URI that Monaco + // picks language syntax + LSP wiring from, so pinning every branch protects + // the language → file-suffix contract. + function suffixFor(lang: string): string { + const fixture = makeFixture(mockJavaUDFPredicate); + return (fixture.componentInstance as any).getFileSuffixByLanguage(lang); + } + + it("maps python → .py", () => expect(suffixFor("python")).toBe(".py")); + it("maps r → .r", () => expect(suffixFor("r")).toBe(".r")); + it("maps javascript → .js", () => expect(suffixFor("javascript")).toBe(".js")); + it("maps java → .java", () => expect(suffixFor("java")).toBe(".java")); + it("is case-insensitive on the language name", () => { + // `suffixFor` builds a fixture per call which adds the predicate's + // operator to the workflow; with the same predicate twice in one test + // the second `addOperator` collides. Call once and reach the method + // directly to assert another case-folded input. + const fixture = makeFixture(mockJavaUDFPredicate); + const fn = (fixture.componentInstance as any).getFileSuffixByLanguage.bind(fixture.componentInstance); + expect(fn("Python")).toBe(".py"); + expect(fn("JAVA")).toBe(".java"); + }); + it("falls back to .py for unknown languages so the default Monaco grammar is python", () => { + expect(suffixFor("brainfuck")).toBe(".py"); + }); + }); + + describe("onFocus", () => { + it("highlights the operator the editor is bound to", () => { + const fixture = makeFixture(mockJavaUDFPredicate); + const highlightSpy = vi.spyOn(workflowActionService.getJointGraphWrapper(), "highlightOperators"); + fixture.componentInstance.onFocus(); + expect(highlightSpy).toHaveBeenCalledWith(mockJavaUDFPredicate.operatorID); + }); + }); + + describe("rejectCurrentAnnotation", () => { + it("hides the suggestion UI and clears the staged code + suggestion", () => { + const fixture = makeFixture(mockJavaUDFPredicate); + const c = fixture.componentInstance; + + c.showAnnotationSuggestion = true; + c.currentCode = "x = 1"; + c.currentSuggestion = "x: int = 1"; + + c.rejectCurrentAnnotation(); + + expect(c.showAnnotationSuggestion).toBe(false); + expect(c.currentCode).toBe(""); + expect(c.currentSuggestion).toBe(""); + }); + + it("emits on the multi-variable response subject when one is staged", () => { + const fixture = makeFixture(mockJavaUDFPredicate); + const c = fixture.componentInstance; + const userResponseSubject = new Subject<void>(); + const nextSpy = vi.spyOn(userResponseSubject, "next"); + + // The two flags together gate the multi-variable continuation; both are + // private, so we reach through `(c as any)` to wire them up. + (c as any).isMultipleVariables = true; + (c as any).userResponseSubject = userResponseSubject; + + c.rejectCurrentAnnotation(); + + expect(nextSpy).toHaveBeenCalledOnce(); + }); + }); + + describe("acceptCurrentAnnotation", () => { + it("is a no-op when the suggestion UI is not currently shown", () => { + const fixture = makeFixture(mockJavaUDFPredicate); + const c = fixture.componentInstance; + // No state set → early return path; nothing should change. + c.showAnnotationSuggestion = false; + expect(() => c.acceptCurrentAnnotation()).not.toThrow(); + expect(c.showAnnotationSuggestion).toBe(false); + }); + + it("hides the suggestion UI after accepting", () => { + const fixture = makeFixture(mockJavaUDFPredicate); + const c = fixture.componentInstance; + + // The accept path reaches into the underlying MonacoEditorLanguageClientWrapper + // for `.getEditor()` and into the YText `.code` for `.insert()`. Both are + // private so we stub them through bracket access to a minimum that lets + // insertTypeAnnotations no-op cleanly. `dispose` is needed because + // ngOnDestroy fires at teardown and calls it. + (c as any).editorWrapper = { + getEditor: () => ({ + getModel: () => ({ getOffsetAt: () => 0 }), + }), + dispose: vi.fn(), + }; + (c as any).code = { insert: vi.fn() }; + + c.showAnnotationSuggestion = true; + c.currentRange = new monaco.Range(1, 1, 1, 5); + c.currentSuggestion = ": int"; + + c.acceptCurrentAnnotation(); + + expect(c.showAnnotationSuggestion).toBe(false); + }); + }); + + describe("AI assistant action wiring", () => { + // setupAIAssistantActions checks an AI-provider flag (OpenAI vs others) + // before deciding whether to register the per-selection 'Add Type + // Annotation' action. We can't drive the action body without a real + // Monaco editor, but the gate itself is plain RxJS — flip the flag and + // assert observable behaviour. + it("emits 'OpenAI' from the AI assistant gate when configured that way", async () => { + // Re-configure TestBed with a mock that drives the gate; existing tests + // use the default DI-resolved service. + await TestBed.resetTestingModule() + .configureTestingModule({ + providers: [ + WorkflowActionService, + { provide: OperatorMetadataService, useClass: AugmentedStubMetadataService }, + { provide: AIAssistantService, useValue: { checkAIAssistantEnabled: () => of("OpenAI") } }, + ...commonTestProviders, + ], + imports: [CodeEditorComponent, HttpClientTestingModule], + }) + .compileComponents(); + const wfActions = TestBed.inject(WorkflowActionService); + wfActions.addOperator(mockJavaUDFPredicate, mockPoint); + wfActions.getJointGraphWrapper().highlightOperators(mockJavaUDFPredicate.operatorID); + const fixture = TestBed.createComponent(CodeEditorComponent); + fixture.detectChanges(); + const checked = await TestBed.inject(AIAssistantService).checkAIAssistantEnabled().toPromise(); + expect(checked).toBe("OpenAI"); + }); + }); });
