Skip to content

test(frontend): cover the debugger's breakpoint gutter - #7425

Open
aglinxinyuan wants to merge 1 commit into
apache:mainfrom
aglinxinyuan:test-code-debugger-breakpoints
Open

test(frontend): cover the debugger's breakpoint gutter#7425
aglinxinyuan wants to merge 1 commit into
apache:mainfrom
aglinxinyuan:test-code-debugger-breakpoints

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

setupMonacoBreakpointMethods was the component's one uncovered block. The existing suite stubs it out — with a comment saying so — because the minimal editor mock cannot back a real MonacoBreakpoint, so neither of the two overrides it installs was exercised.

Adds 13 tests over both.

The glyph override decides what the gutter shows:

exists && condition present   ->  monaco-conditional-breakpoint
exists && no condition        ->  monaco-breakpoint
hovering only                 ->  monaco-hover-breakpoint

Covered including the Boolean(condition?.trim()) guard — a condition left as whitespace must render as an ordinary breakpoint rather than claiming a condition the debugger will not apply — and the lookup happening at range.startLineNumber, since reading endLineNumber would attribute another line's condition to this glyph.

The mouse-down override replaces the library's own handler. The dispose() before re-registering is load-bearing: two live handlers would add and immediately remove a breakpoint on a single click. A left click toggles; a right click opens the condition input instead of toggling, and only for a line that already has a breakpoint; clicks below the last line and outside the gutter do nothing.

Verified by mutation, all reverted (production diff empty):

Mutation Result
treat a blank condition as a condition red
read the condition from the range's end line red
key the condition lookup to a fixed operator red
swap the conditional and plain glyphs red
swap the exists and hover arms red
skip disposing the previous mouse-down handler red
drop the gutter target-type check red
drop the after-lines guard red
invert the left/right button branch red

The stand-in editor is a Proxy that answers any unstubbed on* listener with an inert disposable, so the spec does not have to track which events monaco-breakpoints subscribes to — the first attempt failed on onDidChangeCursorPosition, and guessing at the rest would have been fragile.

No production file is touched.

Any related issues, documentation, discussions?

Closes #7422

How was this PR tested?

npx ng test --watch=false --include="**/code-debugger.component.spec.ts"
 Test Files  1 passed (1)
      Tests  29 passed (29)

13 new on top of the existing 16. yarn format:ci passes.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 5)

setupMonacoBreakpointMethods was the component's one uncovered block. The
existing suite stubs it out because the minimal editor mock cannot back a real
MonacoBreakpoint, so neither of the two overrides it installs was exercised.

Adds 13 tests over both. The glyph override: a plain breakpoint, one carrying a
condition, a whitespace-only condition that must not claim to be conditional, a
hovered line, and the condition being looked up on the line the glyph starts at
rather than where its range ends.

The mouse-down override: the library's own handler is disposed before the
replacement registers, since two live handlers would add and immediately remove
a breakpoint on one click; a left click toggles; a right click opens the
condition input instead of toggling, and only for a line that has a breakpoint;
clicks below the last line and outside the gutter do nothing.

The stand-in editor answers any unstubbed on* listener with an inert disposable,
so it does not have to track which events monaco-breakpoints subscribes to.

No production file is touched.
@github-actions github-actions Bot added the frontend Changes related to the frontend GUI label Aug 8, 2026
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @Yicong-Huang
    You can notify them by mentioning @Yicong-Huang in a comment.

@aglinxinyuan
aglinxinyuan requested a review from mengw15 August 8, 2026 07:53
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.15%. Comparing base (fe89db4) to head (dfbd666).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7425      +/-   ##
============================================
+ Coverage     85.00%   85.15%   +0.15%     
  Complexity     4148     4148              
============================================
  Files          1169     1169              
  Lines         46740    46740              
  Branches       5202     5202              
============================================
+ Hits          39731    39802      +71     
+ Misses         5289     5219      -70     
+ Partials       1720     1719       -1     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from fe89db4
agent-service 85.50% <ø> (ø) Carriedforward from fe89db4
amber 80.85% <ø> (ø) Carriedforward from fe89db4
computing-unit-managing-service 50.72% <ø> (ø) Carriedforward from fe89db4
config-service 65.97% <ø> (ø) Carriedforward from fe89db4
file-service 69.05% <ø> (ø) Carriedforward from fe89db4
frontend 87.48% <ø> (+0.34%) ⬆️
notebook-migration-service 78.89% <ø> (ø) Carriedforward from fe89db4
pyamber 97.55% <ø> (ø) Carriedforward from fe89db4
workflow-compiling-service 26.31% <ø> (ø) Carriedforward from fe89db4

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

☔ View full report in Codecov by Harness.
📢 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.

@mengw15 mengw15 left a comment

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.

LGTM

Copilot AI left a comment

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.

Pull request overview

Adds dedicated unit coverage for CodeDebuggerComponent.setupMonacoBreakpointMethods, specifically the breakpoint gutter glyph rendering and gutter mouse interactions, without modifying production code.

Changes:

  • Introduces a new test suite that drives the MonacoBreakpoint decoration override to validate glyph class selection (plain/conditional/hover).
  • Adds tests for the overridden gutter mouse-down handler to validate left-click toggle vs right-click condition input behavior and guard conditions.
  • Implements a proxy-based stand-in Monaco editor to avoid brittle stubbing of all monaco-breakpoints event subscriptions.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +513 to +517
const fake = makeEditor();
editor = fake.editor;
component.monacoEditor = editor;
component.setupMonacoBreakpointMethods(editor);
});
Comment on lines +603 to +605

expect(component.breakpointConditionLine).not.toBe(12);
});
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.

Cover the debugger's breakpoint gutter

4 participants