-
Notifications
You must be signed in to change notification settings - Fork 36
Phase 2: Add missing test coverage before shared package extraction #765
Copy link
Copy link
Open
Labels
triage-neededIssue is not triaged.Issue is not triaged.
Description
Phase 2: Test Coverage — Add missing tests before extraction
Part of the shared package extraction effort: microsoft/vscode-python-tools-extension-template#290
The following merged PRs introduced bug fixes or features that lack adequate test coverage. Tests should be added before extracting shared code into the monorepo package to ensure behavioral correctness is preserved.
| PR | Untested functionality description | Missing tests |
|---|---|---|
| #684 | Align PYTHON_MINOR constant with CI minimum of 3.10 | Changes PYTHON_MINOR from 9 to 10 in src/common/constants.ts. The python.unit.test.ts has version rejection tests that import from constants and test the version-checking logic in python.ts, which uses PYTHON_MINOR. However, these tests mock the Python version returned by the extension API rather than testing specific version boundaries, so the exact threshold change (3.9->3.10) may not be directly validated. |
| #687 | Add loading indicator | Changes lsp_server.py (adds pylint/lintingStarted and pylint/lintingFailed notifications, modifies score notification to always fire), server.ts (adds handlers for new notifications), and status.ts (loading/error UI states). Score notification tests cover the modified pylint/score notification path and verify structure. Linting tests exercise _linting_helper indirectly. However, the NEW pylint/lintingStarted and pylint/lintingFailed notifications have no dedicated tests. The TypeScript status bar loading/error display logic in status.ts and the new handlers in server.ts are entirely untested. |
| #705 | Sync with template: Fix duplicate server handlers on concurrent restarts | Changes src/common/server.ts (disposables cleanup moved outside if-block) and src/extension.ts (try/catch around lsClient.stop in deactivate). No TypeScript tests exist for restartServer() or deactivate() functions. The changes affect server lifecycle management which has no test coverage. |
| #706 | Sync with template: Fix duplicate server handlers on concurrent restarts | Changes src/common/settings.ts (adds serverEnabled to checkIfConfigurationChanged and new getServerEnabled function) and src/extension.ts (adds serverEnabled check in restart flow). Settings tests cover getWorkspaceSettings and isLintOnChangeEnabled but do NOT test checkIfConfigurationChanged or the new getServerEnabled function. The extension.ts restart logic with serverEnabled guard has no test coverage. |
| #732 | Use contextlib.suppress instead of try-except-pass |
The changed files (lsp_jsonrpc.py, lsp_utils.py) are exercised indirectly by all LSP-based tests since they provide the JSON-RPC transport and module-running utilities. However, no tests specifically target the error-suppression paths (stream close failures, SystemExit suppression). The refactoring is purely stylistic (try/except/pass to contextlib.suppress) so the existing integration tests validate that normal behavior is preserved. |
| #737 | Replace hand-rolled typings with @vscode/python-environments npm package | python.unit.test.ts directly tests getInterpreterDetails from src/common/python.ts which was the main file changed. The 'Finds interpreter using Python Environments extension' test exercises the environments API path. However, tests mock the extension APIs, so the new PythonEnvironments.api() call from @vscode/python-environments is not directly validated -- only the downstream logic is covered. |
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
triage-neededIssue is not triaged.Issue is not triaged.