Skip to content

test: comprehensive unit tests for single-pane layout strategies#324707

Open
sandy081 wants to merge 2 commits into
mainfrom
agents/single-pane-strategy-tests
Open

test: comprehensive unit tests for single-pane layout strategies#324707
sandy081 wants to merge 2 commits into
mainfrom
agents/single-pane-strategy-tests

Conversation

@sandy081

@sandy081 sandy081 commented Jul 7, 2026

Copy link
Copy Markdown
Member

Summary

Adds singlePaneStrategies.test.ts — focused unit tests for each composed single-pane layout strategy in isolation, complementing the existing controller-level integration tests in desktopSessionLayoutController.test.ts.

The single-pane layout (shipped in #324657) is composed from eight strategy objects under contrib/layout/browser/singlePane/. These tests instantiate each real strategy class directly via createInstance(StrategyClass, ctx, ...) — driving the actual constructor autoruns, event listeners, and private sync logic — with the shared createTestHarness service doubles and a hand-built ISinglePaneLayoutContext (createStrategyTestContext) that exposes the coordination flags (isRestoringSessionLayout, togglingSidePane, hidingAuxiliaryBarForRestore) and the viewStateBySession map the controller normally owns.

Coverage (46 tests)

Strategy Tests Notable edges
NewSessionRulesStrategy (R1) 6 become-visible hide, inherited-visible hide on entry, explicit-reveal respected, created/quick-chat/multi-visible excluded
QuickChatEditorHideStrategy 4 hide only when empty group; not for real editor / non-quick-chat / already-hidden
DetailVisibilityStrategy (D1-D4) 13 switch-away capture, live D2 capture + guards, D3b default reveal/hide, D3c leave-as-is with no saved state, intrinsic submit reveal Changes vs hidden, collapse marker
DetailPanelStrategy 9 quick-chat/empty to Hidden, empty-group to Preserve during restore, Changes/Files mapping, forced targets, no-reveal when hidden, maximized to Changes
ManagedTabsStrategy 6 ensure Changes+Files, uncreated files-only, quick-chat none, dismissal memory + + Files context, reopen clears dismissal, workspace-file collapse
EditorAreaCollapseStrategy 4 collapse/restore non-managed, skip during restore, skip managed placeholder
ResponsiveSidebarStrategy (D7) 4 toggleDetails open+auto-hide / close+restore, restore on full side-pane hide, manual toggle hands back

Validation

  • typecheck-client, transpile: clean
  • 46 (new) + 114 (controller) + 45 (workbench) tests pass, no disposable leaks
  • valid-layers-check, eslint: clean

Test-only change; no production code modified.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Add singlePaneStrategies.test.ts covering each composed single-pane
strategy in isolation (NewSessionRules, QuickChatEditorHide,
DetailVisibility, DetailPanel, ManagedTabs, EditorAreaCollapse,
ResponsiveSidebar) via a hand-built ISinglePaneLayoutContext backed by
the shared test harness. Focuses on the bug-prone edges surfaced during
the refactor: intrinsic submit detection, D3c leave-as-is with no saved
state, empty-group Preserve during restore, R1 editor re-hide, managed
tab dismissal memory, and responsive sidebar auto-hide/restore.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 7, 2026 09:04
@sandy081 sandy081 self-assigned this Jul 7, 2026
@vs-code-engineering

Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@benibenj

Matched files:

  • src/vs/sessions/contrib/layout/test/browser/singlePaneStrategies.test.ts

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

This is a test-only PR that adds singlePaneStrategies.test.ts, providing focused unit coverage for each of the eight composed single-pane layout strategy objects introduced in #324657 (contrib/layout/browser/singlePane/). Rather than exercising behavior only through the controller integration tests, each real strategy class is instantiated in isolation via createInstance(StrategyClass, ctx, …) against the shared createTestHarness service doubles and a hand-built ISinglePaneLayoutContext (createStrategyTestContext) that exposes the coordination flags (isRestoringSessionLayout, togglingSidePane, hidingAuxiliaryBarForRestore) and viewStateBySession map the controller normally owns. A companion note is added to the sessions SKILL doc describing this per-strategy testing approach and its two gotchas.

Changes:

  • Adds 46 unit tests across the seven single-pane strategies (new-session rules, quick-chat editor hide, detail visibility, detail panel, managed tabs, editor-area collapse, responsive sidebar), including bug-prone edges (D3c leave-as-is, intrinsic submit detection, empty-group→Preserve during restore).
  • Introduces createStrategyTestContext, a lightweight ISinglePaneLayoutContext double with mutable coordination flags plus a settle() sequencer-flush helper.
  • Documents the isolated per-strategy testing pattern in .github/skills/sessions/SKILL.md.
Show a summary per file
File Description
src/vs/sessions/contrib/layout/test/browser/singlePaneStrategies.test.ts New 837-line test suite instantiating each single-pane strategy directly and asserting its isolated behavior; verified to align with each strategy's implementation and to reuse the existing harness/disposable conventions.
.github/skills/sessions/SKILL.md Adds an accurate learnings bullet describing how to unit-test single-pane strategies in isolation, including the direct-construction of SinglePaneDockedTabsCoordinator and the sequencer settle() caveat.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread src/vs/sessions/contrib/layout/test/browser/singlePaneStrategies.test.ts Outdated
Address CCR feedback: the restoreCalls field was never read or asserted.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sandy081 sandy081 enabled auto-merge (squash) July 7, 2026 09:21
@sandy081 sandy081 added this to the 1.129.0 milestone Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants