Skip to content

Sanity tests: stop constructing a second ConversationFeature#316732

Draft
benvillalobos wants to merge 3 commits into
microsoft:mainfrom
benvillalobos:bv/sanity-no-local-conversation-feature
Draft

Sanity tests: stop constructing a second ConversationFeature#316732
benvillalobos wants to merge 3 commits into
microsoft:mainfrom
benvillalobos:bv/sanity-no-local-conversation-feature

Conversation

@benvillalobos
Copy link
Copy Markdown
Member

Sanity tests currently construct their own ConversationFeature and force activated = true synchronously, in addition to the global one that ContributionCollection creates during extension activation. Both instances react to onDidAuthenticationChange and try to register global ids (e.g. the copilot-chat.fixWithCopilot terminal quick-fix provider, chat participants), which races and produces flaky already registered failures — sometimes synchronously during new ConversationFeature() in the test, sometimes asynchronously from the global's auth listener firing inside a Timeout._onTimeout.

This change removes the test-local ConversationFeature entirely. suiteSetup now:

  • awaits IAuthenticationService.getCopilotToken() so the Copilot token is minted, then
  • waits briefly for onDidAuthenticationChange listeners to run, which flips the global ConversationFeature to activated = true (registering chat participants and other global state) before any test runs.

Tests then use ChatParticipantRequestHandler directly against the production-activated global feature, which is what the real chat flow does anyway.

Net effect: only one ConversationFeature exists at runtime, eliminating the registration race. Companion to #316709 (Mocha retries) and #316719 (proper dispose in finally).

Copilot AI review requested due to automatic review settings May 15, 2026 23:48
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

Removes the test-local ConversationFeature instances from sanity tests to eliminate a race with the globally-activated ConversationFeature registered by ContributionCollection during extension activation. Instead, suiteSetup mints a Copilot token to drive the global feature's auth listeners to flip activated = true before tests run.

Changes:

  • Import IAuthenticationService; remove ConversationFeature import.
  • In suiteSetup, call getCopilotToken() and wait 500ms so the global ConversationFeature gets activated.
  • Drop per-test ConversationFeature creation and try/finally activated toggling across all four tests.
Show a summary per file
File Description
extensions/copilot/src/extension/test/vscode-node/sanity.sanity-test.ts Replace per-test ConversationFeature setup with a one-time token mint + brief wait in suiteSetup so the global ConversationFeature activates once.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

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.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new

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.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new

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.

2 participants