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-5241-2ebef323605b156bcaaeb9eb02454797059758a2 in repository https://gitbox.apache.org/repos/asf/texera.git
commit e25c28225f25990a4c149b85fa48f8090900cf85 Author: Matthew B. <[email protected]> AuthorDate: Sun May 31 16:48:20 2026 -0700 test(frontend): browser-mode spec for code-editor.component (#5241) ### What changes were proposed in this PR? - Adds a browser-mode companion spec, code-editor.component.browser.spec.ts, that covers the four paths the jsdom spec cannot reach: the initializeMonacoEditor subscribe body (readOnly toggle, MonacoBinding wiring, AI-action registration), initializeDiffEditor's diff config path, the OpenAI gate in setupAIAssistantActions, the type-annotation action's getScrolledVisiblePosition branch, and the onWindowResize -> editor.layout() handler. y-monaco's MonacoBinding is mocked via vi.hoisted so the constructor records its args without needing a real Y.Text + monaco TextModel. - Adds src/browser-buffer-polyfill.ts and wires it as the first setupFile in vitest.browser.config.ts so monaco-editor-wrapper and monaco-vscode-files-service-override can be imported under package by listing it in optimizeDeps.include so Vite's import-scan picks it up despite the setup-file indirection. - Broadens the gui:test-browser target's include glob to **/*.browser.spec.ts in angular.json, and adds the same pattern to gui:test's exclude list so the two runners do not double-execute. ### Any related issues, documentation, or discussions? Closes: #5192 ### How was this PR tested? - yarn nx run gui:test-browser: 29/29 passing (7 new code-editor + 22 existing workflow-editor). - yarn nx run gui:test: 623 passed, 2 skipped, 1 todo (no regression from the exclude change). ### Was this PR authored or co-authored using generative AI tooling? Co-authored with Claude Opus 4.7 in compliance with ASF --- frontend/angular.json | 8 +- frontend/package.json | 1 + .../code-editor.component.browser.spec.ts | 324 +++++++++++++++++++++ frontend/src/browser-buffer-polyfill.ts | 44 +++ frontend/vitest.browser.config.ts | 17 +- frontend/yarn.lock | 1 + 6 files changed, 392 insertions(+), 3 deletions(-) diff --git a/frontend/angular.json b/frontend/angular.json index b9e9961d02..01b2efdd6f 100644 --- a/frontend/angular.json +++ b/frontend/angular.json @@ -94,7 +94,8 @@ "include": ["**/*.spec.ts"], "setupFiles": ["src/jsdom-svg-polyfill.ts"], "exclude": [ - "**/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts" + "**/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts", + "**/*.browser.spec.ts" ] } }, @@ -105,7 +106,10 @@ "runner": "vitest", "runnerConfig": "vitest.browser.config.ts", "tsConfig": "src/tsconfig.spec.json", - "include": ["**/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts"] + "include": [ + "**/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts", + "**/*.browser.spec.ts" + ] } } } diff --git a/frontend/package.json b/frontend/package.json index 481e720d97..265b0bbe4f 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -122,6 +122,7 @@ "@vitest/browser": "4.1.5", "@vitest/browser-playwright": "4.1.5", "@vitest/coverage-v8": "4.1.5", + "buffer": "5.7.1", "concurrently": "7.4.0", "eslint": "8.57.0", "eslint-plugin-rxjs": "5.0.3", diff --git a/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.browser.spec.ts b/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.browser.spec.ts new file mode 100644 index 0000000000..d6a3266fd1 --- /dev/null +++ b/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.browser.spec.ts @@ -0,0 +1,324 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Browser-mode companion to code-editor.component.spec.ts. The sibling jsdom +// spec covers the constructor, language detection, getFileSuffixByLanguage, +// onFocus, getCoeditorCursorStyles, and the accept/reject annotation paths, +// but cannot reach anything gated on a real Monaco editor — the +// `initAndStart` subscribe body, `initializeDiffEditor`, AI-action run +// callbacks, `handleTypeAnnotation`'s position branch, and the resize +// handler. This spec drives those by swapping the component's editorWrapper +// for a fake-with-real-DOM and running in vitest's Playwright/Chromium +// browser mode, where monaco-editor's codingame fork can be imported without +// jsdom's missing-canvas / Node-Buffer-allocation tripwires (the Buffer/process +// shim is wired as the first setupFile in vitest.browser.config.ts — see +// src/browser-buffer-polyfill.ts). + +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { HttpClientTestingModule } from "@angular/common/http/testing"; +import { FormControl } from "@angular/forms"; +import { BehaviorSubject, of } from "rxjs"; +import * as Y from "yjs"; + +import { CodeEditorComponent } from "./code-editor.component"; +import { WorkflowActionService } from "../../service/workflow-graph/model/workflow-action.service"; +import { WorkflowVersionService } from "../../../dashboard/service/user/workflow-version/workflow-version.service"; +import { AIAssistantService } from "../../service/ai-assistant/ai-assistant.service"; +import { OperatorMetadataService } from "../../service/operator-metadata/operator-metadata.service"; +import { StubOperatorMetadataService } from "../../service/operator-metadata/stub-operator-metadata.service"; +import { mockOperatorMetaData } from "../../service/operator-metadata/mock-operator-metadata.data"; +import { mockJavaUDFPredicate, mockPoint } from "../../service/workflow-graph/model/mock-workflow-data"; +import { OperatorSchema } from "../../types/operator-schema.interface"; +import { commonTestProviders } from "../../../common/testing/test-utils"; +import * as monaco from "monaco-editor"; + +// y-monaco's MonacoBinding wires real listeners against the YText and a real +// monaco TextModel. Our fake editor returns a stub model, so the binding's +// constructor would throw on `model.onDidChangeContent(...)`. The component +// only depends on the binding's `destroy()` (called in ngOnDestroy) and the +// fact that the constructor was called with the right shape of args, so a +// recording stub is sufficient. +// `vi.mock` is hoisted to the top of the file, so any closure variables it +// references must be declared inside `vi.hoisted` — a plain top-level `const` +// is evaluated AFTER the mock factory runs, leaving `monacoBindingCalls` +// undefined at the moment MonacoBinding's constructor would try to push. +const { monacoBindingCalls } = vi.hoisted(() => ({ + monacoBindingCalls: [] as unknown[][], +})); +vi.mock("y-monaco", () => ({ + MonacoBinding: class { + constructor(...args: unknown[]) { + monacoBindingCalls.push(args); + } + destroy = vi.fn(); + }, +})); + +// Re-use the augmented stub from the jsdom spec so the component constructor +// can resolve its highlighted operator regardless of operatorType. +const baseSchema = mockOperatorMetaData.operators.find(op => op.operatorType === "PythonUDF"); +if (!baseSchema) { + throw new Error( + "CodeEditorComponent browser spec setup expected a PythonUDF schema in mockOperatorMetaData — fixture has drifted." + ); +} +const synthesizeSchema = (operatorType: string): OperatorSchema => ({ ...baseSchema, operatorType }); +const augmentedSchemas: OperatorSchema[] = [...mockOperatorMetaData.operators, synthesizeSchema("PythonUDFV2")]; +class AugmentedStubMetadataService extends StubOperatorMetadataService { + private readonly augmentedMetadata = of({ ...mockOperatorMetaData, operators: augmentedSchemas }); + override getOperatorMetadata(): typeof this.augmentedMetadata { + return this.augmentedMetadata; + } + override getOperatorSchema(operatorType: string): OperatorSchema { + const schema = augmentedSchemas.find(op => op.operatorType === operatorType); + if (!schema) throw new Error(`unknown operatorType ${operatorType}`); + return schema; + } + override operatorTypeExists(operatorType: string): boolean { + return augmentedSchemas.some(op => op.operatorType === operatorType); + } +} + +// A minimal, recording Monaco editor stand-in. Returns realistic values +// where the component reads from it (`getScrolledVisiblePosition`, +// `getSelection`, `getModel`) and records what the component asks it to do +// (`addAction`, `updateOptions`, `layout`). The component does not introspect +// any of these beyond truthiness, so the stub does not need to be a real +// IStandaloneCodeEditor — TypeScript's structural check is the only gate. +interface FakeEditor { + addAction: ReturnType<typeof vi.fn>; + updateOptions: ReturnType<typeof vi.fn>; + layout: ReturnType<typeof vi.fn>; + getSelection: ReturnType<typeof vi.fn>; + getModel: ReturnType<typeof vi.fn>; + getScrolledVisiblePosition: ReturnType<typeof vi.fn>; + createDecorationsCollection: ReturnType<typeof vi.fn>; + actions: monaco.editor.IActionDescriptor[]; +} + +function makeFakeEditor(): FakeEditor { + const actions: monaco.editor.IActionDescriptor[] = []; + return { + actions, + addAction: vi.fn((action: monaco.editor.IActionDescriptor) => { + actions.push(action); + return { dispose: vi.fn(), id: action.id, label: action.label }; + }), + updateOptions: vi.fn(), + layout: vi.fn(), + getSelection: vi.fn(() => new monaco.Selection(1, 1, 1, 5)), + getModel: vi.fn(() => ({ + getValue: () => "x = 1\ny = 2\n", + getValueInRange: () => "x", + onDidChangeContent: () => ({ dispose: () => {} }), + getOffsetAt: () => 0, + })), + getScrolledVisiblePosition: vi.fn(() => ({ top: 50, left: 100, height: 18 })), + createDecorationsCollection: vi.fn(() => ({ clear: vi.fn() })), + }; +} + +function makeFakeWrapper(editor: FakeEditor) { + return { + initAndStart: vi.fn().mockResolvedValue(undefined), + getEditor: vi.fn(() => editor), + dispose: vi.fn(), + }; +} + +describe("CodeEditorComponent (browser)", () => { + let displayVersionStream$: BehaviorSubject<boolean>; + let aiEnabled$: BehaviorSubject<string>; + let getTypeAnnotationsSpy: ReturnType<typeof vi.fn>; + let locateUnannotatedSpy: ReturnType<typeof vi.fn>; + let workflowActionService: WorkflowActionService; + + beforeEach(async () => { + monacoBindingCalls.length = 0; + displayVersionStream$ = new BehaviorSubject<boolean>(false); + aiEnabled$ = new BehaviorSubject<string>("OpenAI"); + getTypeAnnotationsSpy = vi.fn().mockReturnValue(of({ choices: [{ message: { content: ": int" } }] })); + locateUnannotatedSpy = vi.fn().mockReturnValue(of([])); + + await TestBed.configureTestingModule({ + providers: [ + WorkflowActionService, + { provide: OperatorMetadataService, useClass: AugmentedStubMetadataService }, + { + provide: AIAssistantService, + useValue: { + checkAIAssistantEnabled: () => aiEnabled$, + getTypeAnnotations: getTypeAnnotationsSpy, + locateUnannotated: locateUnannotatedSpy, + }, + }, + { + provide: WorkflowVersionService, + useValue: { + getDisplayParticularVersionStream: () => displayVersionStream$, + }, + }, + ...commonTestProviders, + ], + imports: [CodeEditorComponent, HttpClientTestingModule], + }).compileComponents(); + + workflowActionService = TestBed.inject(WorkflowActionService); + }); + + // Builds a fixture for the highlighted operator, but defers the + // detectChanges/ngAfterViewInit step so the caller can swap in the fake + // editor wrapper and stage `code` / `formControl` before the subscribe body + // fires. Returns the fixture, the fake wrapper, and the fake editor. + function makeFixtureWithFakes() { + const predicate = { ...mockJavaUDFPredicate }; + workflowActionService.addOperator(predicate, mockPoint); + workflowActionService.getJointGraphWrapper().highlightOperators(predicate.operatorID); + + const fixture = TestBed.createComponent(CodeEditorComponent); + const editor = makeFakeEditor(); + const wrapper = makeFakeWrapper(editor); + const c = fixture.componentInstance as any; + c.editorWrapper = wrapper; + c.formControl = new FormControl({ value: "", disabled: false }); + // A YText must live inside a Y.Doc to be useful; the binding stub doesn't + // care, but we stage it as if it came from the shared model so the + // subscribe body crosses the `if (!this.code) return;` gate. + c.code = new Y.Doc().getText("code"); + return { fixture, wrapper, editor, c: c as CodeEditorComponent }; + } + + // The component's subscribe path runs `from(initAndStart(...)).pipe(...)`, + // which is microtask-async. One macrotask flush after detectChanges is + // enough for the RxJS chain to deliver the editor into the subscribe body. + async function flush(): Promise<void> { + await Promise.resolve(); + await new Promise(r => setTimeout(r, 0)); + } + + it("initializeMonacoEditor: wires the editor + MonacoBinding + AI actions when code exists", async () => { + const { fixture, wrapper, editor } = makeFixtureWithFakes(); + + fixture.detectChanges(); + await flush(); + + // The non-diff config branch should fire with the editor host element. + expect(wrapper.initAndStart).toHaveBeenCalledOnce(); + const [userConfig, host] = wrapper.initAndStart.mock.calls[0]; + expect((userConfig as any).wrapperConfig.editorAppConfig.useDiffEditor).toBeUndefined(); + expect((userConfig as any).wrapperConfig.editorAppConfig.codeResources.main.uri).toMatch(/^in-memory-.*\.\.java$/); + expect(host).toBeInstanceOf(HTMLElement); + + // The subscribe body should: push readOnly via updateOptions, construct + // MonacoBinding, and register the two AI actions. + expect(editor.updateOptions).toHaveBeenCalledWith({ readOnly: false }); + expect(monacoBindingCalls.length).toBe(1); + expect(editor.addAction).toHaveBeenCalledTimes(2); + }); + + it("initializeMonacoEditor: respects formControl.disabled when toggling readOnly", async () => { + const { fixture, editor, c } = makeFixtureWithFakes(); + (c as any).formControl = new FormControl({ value: "", disabled: true }); + + fixture.detectChanges(); + await flush(); + + expect(editor.updateOptions).toHaveBeenCalledWith({ readOnly: true }); + }); + + it("initializeDiffEditor: when displayParticularVersion is true, runs the diff config path", async () => { + const { fixture, wrapper } = makeFixtureWithFakes(); + // Seed the stream BEFORE detectChanges so the subscribe in ngAfterViewInit + // picks `true` on first emission and takes the diff branch. + displayVersionStream$.next(true); + + fixture.detectChanges(); + await flush(); + + expect(wrapper.initAndStart).toHaveBeenCalledOnce(); + const [userConfig] = wrapper.initAndStart.mock.calls[0]; + expect((userConfig as any).wrapperConfig.editorAppConfig.useDiffEditor).toBe(true); + // `original` is the previous-version source, only set on the diff path. + expect((userConfig as any).wrapperConfig.editorAppConfig.codeResources.original).toBeDefined(); + }); + + it("setupAIAssistantActions: registers only the 'all' action when the gate is not OpenAI", async () => { + aiEnabled$.next("none"); + const { fixture, editor } = makeFixtureWithFakes(); + + fixture.detectChanges(); + await flush(); + + expect(editor.addAction).toHaveBeenCalledTimes(1); + expect(editor.addAction.mock.calls[0][0].id).toBe("all-type-annotation-action"); + }); + + it("type-annotation-action: invoking the run callback populates suggestion state from position", async () => { + const { fixture, editor } = makeFixtureWithFakes(); + fixture.detectChanges(); + await flush(); + + // The first registered action is "type-annotation-action" (the OpenAI + // gate fires). Pull its `run` callback and invoke it directly — the + // alternative is dispatching via the Monaco command palette, which our + // fake editor doesn't implement. + const typeAction = editor.actions.find(a => a.id === "type-annotation-action"); + expect(typeAction).toBeDefined(); + typeAction!.run(editor as unknown as monaco.editor.ICodeEditor); + + await flush(); + + expect(getTypeAnnotationsSpy).toHaveBeenCalledOnce(); + // getScrolledVisiblePosition returns {top: 50, left: 100}; the component + // adds +100 on each axis when staging the suggestion. + expect(fixture.componentInstance.suggestionTop).toBe(150); + expect(fixture.componentInstance.suggestionLeft).toBe(200); + expect(fixture.componentInstance.showAnnotationSuggestion).toBe(true); + expect(fixture.componentInstance.currentSuggestion).toBe(": int"); + }); + + it("all-type-annotation-action: locateUnannotated returns empty list, action no-ops cleanly", async () => { + const { fixture, editor } = makeFixtureWithFakes(); + fixture.detectChanges(); + await flush(); + + const allAction = editor.actions.find(a => a.id === "all-type-annotation-action"); + expect(allAction).toBeDefined(); + allAction!.run(editor as unknown as monaco.editor.ICodeEditor); + + await flush(); + + expect(locateUnannotatedSpy).toHaveBeenCalledOnce(); + // Empty unannotated list -> processNextVariable never invoked; the + // multi-variable state stays in its initial shape. + expect((fixture.componentInstance as any).isMultipleVariables).toBe(false); + }); + + it("onWindowResize: calls editor.layout() through adjustEditorSize", async () => { + const { fixture, editor } = makeFixtureWithFakes(); + fixture.detectChanges(); + await flush(); + + // Reset layout's call count so we don't pick up Monaco's own init layout. + editor.layout.mockClear(); + fixture.componentInstance.onWindowResize(); + + expect(editor.layout).toHaveBeenCalledOnce(); + }); +}); diff --git a/frontend/src/browser-buffer-polyfill.ts b/frontend/src/browser-buffer-polyfill.ts new file mode 100644 index 0000000000..981615df24 --- /dev/null +++ b/frontend/src/browser-buffer-polyfill.ts @@ -0,0 +1,44 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// monaco-editor-wrapper + monaco-vscode-files-service-override dereference +// Node's Buffer (including Buffer.allocUnsafe) at module-evaluation time. +// Under jsdom-mode Vitest, the test runs on Node and Buffer is built-in. In +// browser-mode (Playwright/Chromium), Buffer doesn't exist, so importing +// monaco-editor-wrapper crashes at load. The `buffer` npm package is the +// browser polyfill of Node's API; expose its `Buffer` (and a minimal +// `process` shim) on the global so monaco's internals find them at the same +// names they'd find Node's. Has to run before any monaco import is +// evaluated, which is why this is wired into vitest.browser.config.ts as the +// FIRST setupFile — setupFiles run in order, before any test module loads. +// +// We use a namespace import (rather than a named or default import) because +// Vite's dep-optimizer rewrites `buffer@5`'s CJS exports in a way that +// exposes neither a `Buffer` named export nor a `default` export at +// module-eval time. The namespace object always has the optimizer-injected +// shape, so we read `Buffer` off it dynamically. +import * as bufferModule from "buffer"; + +const { Buffer } = bufferModule as unknown as { Buffer: typeof globalThis.Buffer }; + +(globalThis as unknown as { Buffer: typeof Buffer }).Buffer = Buffer; +(globalThis as unknown as { global: typeof globalThis }).global = globalThis; +if (!(globalThis as unknown as { process?: { env: Record<string, string> } }).process) { + (globalThis as unknown as { process: { env: Record<string, string> } }).process = { env: {} }; +} diff --git a/frontend/vitest.browser.config.ts b/frontend/vitest.browser.config.ts index eee0624ff1..eb609d1cda 100644 --- a/frontend/vitest.browser.config.ts +++ b/frontend/vitest.browser.config.ts @@ -41,6 +41,15 @@ export default defineConfig({ { find: /^lib0\/webcrypto$/, replacement: lib0Webcrypto }, ], }, + // `buffer` is imported only by the test setup file (src/browser-buffer- + // polyfill.ts), which Vite's import-scan via the @angular/build:unit-test + // builder does NOT crawl — without this hint the dep is served raw and + // `require()` calls inside the CJS package crash on first import. + // Explicit-include forces esbuild to pre-bundle it (alongside its + // `base64-js` + `ieee754` transitive deps) into a browser-runnable ESM. + optimizeDeps: { + include: ["buffer"], + }, test: { // Emit a JUnit-XML report alongside the default console reporter so // Codecov Test Analytics can ingest browser-mode failures and detect @@ -48,7 +57,13 @@ export default defineConfig({ // can disambiguate it from the unit-test report. reporters: ["default", ["junit", { outputFile: "junit-browser.xml" }]], globals: true, - setupFiles: ["src/test-zone-setup.ts"], + // browser-buffer-polyfill must run FIRST: it puts Buffer/process on + // globalThis before any test module loads, which is required for + // monaco-editor-wrapper's eager Buffer.allocUnsafe references (under + // jsdom-mode Vitest these are satisfied by Node's built-in Buffer; in + // browser-mode the runtime has neither, so we install the `buffer` npm + // package as a shim). + setupFiles: ["src/browser-buffer-polyfill.ts", "src/test-zone-setup.ts"], browser: { enabled: true, provider: playwright(), diff --git a/frontend/yarn.lock b/frontend/yarn.lock index b929da0c62..70c89eb2ef 100644 --- a/frontend/yarn.lock +++ b/frontend/yarn.lock @@ -11094,6 +11094,7 @@ __metadata: "@vitest/coverage-v8": "npm:4.1.5" ai: "npm:5.0.93" ajv: "npm:8.10.0" + buffer: "npm:5.7.1" concaveman: "npm:2.0.0" concurrently: "npm:7.4.0" d3-shape: "npm:2.1.0"
