Skip to content

fix(designer): regenerate stale fluentui-icon snapshots missed by #9493 - #9581

Closed
Krrish Mittal (takyyon) wants to merge 1 commit into
Azure:mainfrom
takyyon:takyyon/fix/designer-icon-snapshots
Closed

fix(designer): regenerate stale fluentui-icon snapshots missed by #9493#9581
Krrish Mittal (takyyon) wants to merge 1 commit into
Azure:mainfrom
takyyon:takyyon/fix/designer-icon-snapshots

Conversation

@takyyon

Copy link
Copy Markdown
Collaborator

Summary

libs/designer (v1) had 11 stale Vitest snapshots that baked in real inline SVG markup from @fluentui/react-icons. They pass on stale local installs but fail in CI's fresh install, breaking the build and coverage jobs on every PR (and on main).

Root cause

#9493 made the @fluentui/react-icons Vitest mock reliably intercept — it added the package as a devDependency of shared-test-utils so vi.mock('@fluentui/react-icons') resolves to the same physical module consumers import. It then regenerated the affected snapshots in libs/designer-ui and libs/designer-v2, and concluded:

libs/designer (v1) also wires this setup file but has no snapshots containing real icon SVG output, so nothing to regenerate there.

That was incorrect. 11 v1 snapshots still contained real icon SVG output. With the mock now intercepting under CI's fresh pnpm layout, those icons render as <span data-icon-name="…" /> / <span data-testid="fluent-icon" /> stubs, so the committed snapshots mismatch.

Why it hid locally: on a stale node_modules, libs/shared-test-utils/node_modules/@fluentui/react-icons isn't linked, so the mock silently doesn't resolve and real icons render — matching the stale snapshots. The mismatch only reproduces after a clean pnpm install, which is exactly what CI does. That's why build/coverage are red on main and every open PR while local runs look green.

Changes

  • Regenerate the 11 stale libs/designer snapshots to the mock's documented stub output:
    • MonitoringTimeline: TimelineButtons, TimelineContent, TimelineGroup, TimelineHeader
    • menuItems: collapseMenuItem, pinMenuItem
    • panel: agentChatHeader, monitoring inputsPanel, outputsPanel
    • settings: advancedSettingsMessage, settingsection
  • Port the host-element filter fix(test-utils): make fluentui-react-icons vitest mock reliably intercept #9493 applied to designer-v2's TimelineHeader.test.tsx to the v1 copy it missed. Under the mock the icon stub spreads className onto its host <span>, so findAllByProps({ className: 'timeline-icon' }) matched both the wrapper component instance and its host element, double-counting the icon. Filtering to host elements restores the intended count of 1.

Out of scope

libs/designer's quickViewPanel.spec.tsx has a pre-existing, unrelated failing assertion (the test reassigns its Redux store mid-test without re-rendering), documented as pre-existing in #9493. It is not caused by the mock and is left untouched.

Testing

Clean pnpm install, then from libs/designer:

  • All 11 previously-failing spec files pass (68 tests).
  • npx vitest run -u produces no further snapshot changes.

Commit Type

fix

Risk Level

low — test-only change (regenerated snapshots + one test-assertion filter); no product code touched.

…re#9493

Azure#9493 made the @fluentui/react-icons vitest mock reliably intercept (by
adding it as a devDependency of shared-test-utils so it resolves to the
same physical module), then regenerated the affected snapshots in
libs/designer-ui and libs/designer-v2. It concluded libs/designer (v1)
"has no snapshots containing real icon SVG output, so nothing to
regenerate there" - but that was incorrect: 11 v1 snapshots still baked
in real inline SVG markup from the previously un-intercepted mock.

With the mock now intercepting in CI's fresh pnpm layout, icons render
as <span data-icon-name="..." /> / <span data-testid="fluent-icon" />
stubs, so those 11 snapshots mismatch and the build/coverage jobs fail.
Locally the mock silently does not resolve on stale installs, which is
why the failure only reproduces after a clean install - masking it from
snapshot authors.

- Regenerate the 11 stale libs/designer snapshots to the mock's stub
  output (MonitoringTimeline Timeline{Buttons,Content,Group,Header},
  menuItems collapse/pin, agentChatHeader, monitoring inputs/outputs
  panels, settings advancedSettingsMessage/settingsection).
- Port the same host-element filter Azure#9493 applied to designer-v2's
  TimelineHeader.test.tsx to the v1 copy it missed: findAllByProps
  matched both the wrapper component instance and its host <span>
  (the stub spreads className), double-counting the icon.

The v1 quickViewPanel.spec.tsx assertion Azure#9493 documented as a
pre-existing, unrelated store-mutation failure is left untouched.

Co-authored-by: Copilot <[email protected]>
Copilot AI balanced review requested due to automatic review settings August 27, 2026 23:41
@takyyon Krrish Mittal (takyyon) added the risk:low Low risk change with minimal impact label Aug 27, 2026
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(designer): regenerate stale fluentui-icon snapshots missed by #9493
  • Issue: None — uses a valid fix(scope): prefix and is specific about what changed.
  • Recommendation: No change needed.

Commit Type

  • fix is clearly declared and correct — this repairs stale snapshots that break CI's build/coverage jobs.
  • Single type selected; no overload.

Risk Level

  • Declared Low, risk:low label present, and this matches my advised estimate. The change is test-only (11 regenerated Vitest snapshots + one host-element filter in a test assertion). No product/runtime code is touched, so there is no shipped-behavior impact.

What & Why

  • Current: Detailed Summary + Root cause sections explain the stale snapshots, why they hid locally, and why CI fails.
  • Issue: None.
  • Recommendation: No change needed.

⚠️ Impact of Change

  • No explicit Impact of Change section with Users/Developers/System bullets, though the body's Root cause/Changes sections cover the effect contextually. Non-blocking.
  • Recommendation:
    • Users: None — no user-facing behavior change.
    • Developers: CI build/coverage jobs go green on fresh installs; contributors no longer see spurious snapshot diffs.
    • System: None — test artifacts only.

Test Plan

  • The change is the test update: 11 spec files pass (68 tests) and vitest run -u produces no further diffs after a clean install. No additional automated tests required for a snapshot-regeneration fix.

⚠️ Contributors

  • No Contributors section. Consider crediting anyone who helped diagnose the CI failure. Never blocking.

⚠️ Screenshots/Videos

  • The diff only touches __tests__/.snap files under libs/designer/src/ — no product UI change — so screenshots are not required. Nudge only.

Summary Table

Section Status Recommendation
Title None
Commit Type None
Risk Level None — low matches label + estimate
What & Why None
Impact of Change ⚠️ Add explicit Users/Developers/System bullets
Test Plan None
Contributors ⚠️ Credit collaborators if any
Screenshots/Videos ⚠️ Not required (test-only)

All required checks pass — this test-only snapshot fix is compliant and cleared to merge. Optional: add an Impact section for completeness.


Powered by: Copilot CLI (claude-opus-4.8) | Last updated: Thu, 27 Aug 2026 23:42:15 GMT

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

Regenerates stale designer-v1 snapshots to match the Fluent UI icon mock introduced by #9493.

Changes:

  • Replaces real icon SVG snapshots with mock stubs.
  • Prevents TimelineHeader icon double-counting.
  • Leaves production code unchanged.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
settings/.../settingsection.spec.tsx.snap Updates mocked icon component.
settings/.../advancedSettingsMessage.spec.tsx.snap Replaces SVG with icon stub.
monitoringTab/.../outputsPanel.spec.tsx.snap Regenerates output-panel icons.
monitoringTab/.../inputsPanel.spec.tsx.snap Regenerates input-panel icons.
agentChat/.../agentChatHeader.spec.tsx.snap Regenerates header icons.
MonitoringTimeline/.../TimelineHeader.test.tsx Filters matches to host elements.
MonitoringTimeline/.../TimelineHeader.test.tsx.snap Regenerates timeline header icons.
MonitoringTimeline/.../TimelineGroup.test.tsx.snap Regenerates group icons.
MonitoringTimeline/.../TimelineContent.test.tsx.snap Regenerates content icons.
MonitoringTimeline/.../TimelineButtons.test.tsx.snap Regenerates navigation icons.
menuItems/.../pinMenuItem.spec.tsx.snap Regenerates pin icons.
menuItems/.../collapseMenuItem.spec.tsx.snap Regenerates collapse icons.

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

@takyyon

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #9580, which lands the same fix (identical 11 snapshot regenerations + the same TimelineHeader host-element filter ported from #9493's designer-v2 change, quickViewPanel likewise left out). Elaina's PR got there first and carries the fuller context from the #9544/#9560 chain. Opened this independently before spotting #9580 — no need for two.

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

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants