Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Bug: Completion Test Fails on Unexpected Return Types
The completion test (test('completion suggests attributes')) contains two related bugs:
- The result of
vscode.commands.executeCommand('vscode.executeCompletionItemProvider')is incorrectly assumed to be aCompletionListwith anitemsproperty. If the command returnsCompletionItem[]ornull/undefined, accessing.itemswill cause a runtime error. - Within the mapping of completion item labels,
item.label.labelis accessed without ensuringitem.labelis an object with alabelproperty, which can lead to aTypeError.
packages/poml-vscode/tests/lsp.test.ts#L91-L93
poml/packages/poml-vscode/tests/lsp.test.ts
Lines 91 to 93 in 8bd89bf
Bug: Unprotected Array Access Causes TypeError
The code attempts to access hovers[0].contents[0] without verifying that hovers[0].contents exists or contains elements. This can lead to a TypeError if contents is undefined or an empty array, as the index access [0] will fail before the nullish coalescing operator can be applied.
packages/poml-vscode/tests/lsp.test.ts#L81-L82
poml/packages/poml-vscode/tests/lsp.test.ts
Lines 81 to 82 in 8bd89bf
Was this report helpful? Give feedback by reacting with 👍 or 👎
There was a problem hiding this comment.
Pull Request Overview
This PR adds a VS Code LSP server test suite to validate diagnostics, hover, and completion functionality.
- Introduces new tests for LSP diagnostics, hover responses, and completions
- Includes tests to verify preview features and behavior with bad syntax files
- Adds teardown steps to close editors after tests to avoid interference between tests
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/poml-vscode/tests/preview.test.ts | Added preview feature tests with improved teardown handling |
| packages/poml-vscode/tests/lsp.test.ts | New LSP server tests for diagnostics, hover, completion, preview |
| packages/poml-vscode/test-fixtures/badSyntaxLsp.poml | Fixture file with intentionally bad syntax for diagnostics |
| suite('Preview Feature', () => { | ||
| teardown(async () => { | ||
| await vscode.commands.executeCommand('workbench.action.closeAllEditors'); | ||
| await new Promise(resolve => setTimeout(resolve, 500)); |
There was a problem hiding this comment.
Consider replacing this fixed delay with an event-based or promise-driven approach to ensure editors are fully closed before tests continue.
| await new Promise(resolve => setTimeout(resolve, 500)); | |
| await new Promise(resolve => { | |
| const interval = setInterval(() => { | |
| if (vscode.workspace.textDocuments.length === 0) { | |
| clearInterval(interval); | |
| resolve(); | |
| } | |
| }, 100); | |
| }); |
| }; | ||
| client = new LanguageClient('poml-test', 'POML Language Server', serverOptions, clientOptions); | ||
| await client.start(); | ||
| await new Promise(resolve => setTimeout(resolve, 3000)); |
There was a problem hiding this comment.
Consider using event-based synchronization for the LSP server startup instead of relying on a fixed timeout, which may lead to flaky tests.
| await new Promise(resolve => setTimeout(resolve, 3000)); | |
| await client.onReady(); |
Summary
Testing
npm testnpm run test-vscode(fails: ENOENT or large output)https://chatgpt.com/codex/tasks/task_e_686242735268832e9463f9c2adbea89d