Skip to content

test(frontend): browser-mode spec for code-editor.component#5241

Open
Ma77Ball wants to merge 2 commits into
apache:mainfrom
Ma77Ball:test/codeEditorSpec
Open

test(frontend): browser-mode spec for code-editor.component#5241
Ma77Ball wants to merge 2 commits into
apache:mainfrom
Ma77Ball:test/codeEditorSpec

Conversation

@Ma77Ball
Copy link
Copy Markdown
Contributor

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

@github-actions github-actions Bot added the frontend Changes related to the frontend GUI label May 26, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.57%. Comparing base (d5bc8b7) to head (a8457e8).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5241      +/-   ##
============================================
- Coverage     48.72%   48.57%   -0.16%     
- Complexity     2375     2379       +4     
============================================
  Files          1044     1042       -2     
  Lines         40096    39971     -125     
  Branches       4252     4252              
============================================
- Hits          19538    19415     -123     
+ Misses        19411    19410       -1     
+ Partials       1147     1146       -1     
Flag Coverage Δ *Carryforward flag
access-control-service 39.53% <ø> (ø) Carriedforward from 8a7366f
agent-service 33.76% <ø> (ø) Carriedforward from 8a7366f
amber 51.58% <ø> (+0.02%) ⬆️ Carriedforward from 8a7366f
computing-unit-managing-service 0.00% <ø> (ø) Carriedforward from 8a7366f
config-service 0.00% <ø> (ø) Carriedforward from 8a7366f
file-service 37.99% <ø> (ø) Carriedforward from 8a7366f
frontend 40.02% <ø> (ø)
python 90.46% <ø> (-0.34%) ⬇️ Carriedforward from 8a7366f
workflow-compiling-service 56.81% <ø> (ø) Carriedforward from 8a7366f

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Ma77Ball
Copy link
Copy Markdown
Contributor Author

/request-review @aglinxinyuan

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds browser-mode frontend coverage for CodeEditorComponent paths that require Monaco/browser APIs, while routing .browser.spec.ts files to the existing Chromium-based Vitest target.

Changes:

  • Adds code-editor.component.browser.spec.ts covering Monaco initialization, diff config, AI actions, type annotation positioning, and resize layout.
  • Adds a browser Buffer/process setup shim and wires it into vitest.browser.config.ts.
  • Updates Angular test target globs so browser specs run only in gui:test-browser.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.browser.spec.ts Adds browser-mode component tests for Monaco-bound behavior.
frontend/src/browser-buffer-polyfill.ts Adds global browser shims needed by Monaco-related packages in browser tests.
frontend/vitest.browser.config.ts Registers the buffer setup file first and pre-bundles buffer.
frontend/angular.json Routes *.browser.spec.ts files to the browser test target and excludes them from jsdom tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// 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";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also have this concern. In addition, why do we need to have a polyfill file for buffer?

// 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 (see the
// nodePolyfills entry in vitest.browser.config.ts).
@aglinxinyuan aglinxinyuan enabled auto-merge May 27, 2026 04:53
// 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";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also have this concern. In addition, why do we need to have a polyfill file for buffer?

// (`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 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we testing a fake editor?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wire code-editor.component.spec to gui:test-browser for Monaco-bound coverage

5 participants