Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new unit test in the Copilot inline chat (inlineChat2) area to verify that request handling attaches the expected metadata object to the latest conversation turn.
Changes:
- Added a new
InlineChatIntentunit test assertingTurn.setMetadatais called with aCopilotInteractiveEditorResponse. - Validates key fields on the stored metadata (message id and prompt query contents).
Show a summary per file
| File | Description |
|---|---|
| extensions/copilot/src/extension/inlineChat2/test/node/inlineChatIntent.spec.ts | New unit test covering metadata attachment behavior in InlineChatIntent.handleRequest. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 2
| const documentContext = { document: TextDocumentSnapshot.create(document) } as IDocumentContext; | ||
| const chatTelemetry = {} as ChatTelemetryBuilder; | ||
|
|
||
| await intent.handleRequest(conversation, request, stream, token, documentContext, 'agent', ChatLocation.Editor, chatTelemetry); |
There was a problem hiding this comment.
InlineChatIntent.handleRequest schedules a 1s timeout(...) progress callback; this test currently leaves that as a real timer, which can add ~1s to the suite runtime and leave dangling timers until it fires. Consider using vi.useFakeTimers() and advancing time (e.g. advanceTimersByTimeAsync(1000) / runAllTimersAsync) with vi.useRealTimers() in teardown, so the test stays fast and doesn't depend on wall-clock time.
| await intent.handleRequest(conversation, request, stream, token, documentContext, 'agent', ChatLocation.Editor, chatTelemetry); | |
| vi.useFakeTimers(); | |
| try { | |
| const requestPromise = intent.handleRequest(conversation, request, stream, token, documentContext, 'agent', ChatLocation.Editor, chatTelemetry); | |
| await vi.runAllTimersAsync(); | |
| await requestPromise; | |
| } finally { | |
| vi.useRealTimers(); | |
| } |
| createInstance: vi.fn((ctor, ...args) => { | ||
| if (ctor.name === 'InlineChatProgressMessages') { | ||
| return { | ||
| getContextualMessage: vi.fn().mockResolvedValue('mock message') | ||
| }; | ||
| } | ||
| if (ctor.name === 'InlineChatToolCalling') { | ||
| return { | ||
| run: vi.fn().mockResolvedValue({ | ||
| lastResponse: { type: ChatFetchResponseType.Success, value: 'mocked success!' }, | ||
| telemetry: { telemetryMessageId: 'test-msg-id' }, | ||
| needsExitTool: false | ||
| }) | ||
| }; | ||
| } | ||
| return {}; | ||
| }) |
There was a problem hiding this comment.
The createInstance mock matches on ctor.name and returns {} for any unexpected ctor. That makes the test brittle (class renames) and can hide new dependencies by failing later with unclear errors. Consider (a) renaming the unused rest param to ..._args to satisfy unused-parameter checks, and (b) throwing in the default branch so the test fails immediately if InlineChatIntent starts instantiating other collaborators.
No description provided.