You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@texera.apache.org by gi...@apache.org on 2026/05/24 22:54:56 UTC
(texera) branch gh-readonly-queue/main/pr-5191-cbe90c74c479b8b29b1a02a71a29c1bb17e7d7bd created (now a25c54348d)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-5191-cbe90c74c479b8b29b1a02a71a29c1bb17e7d7bd
in repository https://gitbox.apache.org/repos/asf/texera.git
at a25c54348d test(frontend): extend code-debugger and code-editor coverage (#5191)
This branch includes the following new commits:
new a25c54348d test(frontend): extend code-debugger and code-editor coverage (#5191)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(texera) 01/01: test(frontend): extend code-debugger and code-editor coverage (#5191)
Posted by gi...@apache.org.
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 <17...@users.noreply.github.com>
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");
+ });
+ });
});