Skip to content

chore(vscode): add test for non installation of deps'#4735

Merged
benfdking merged 1 commit intomainfrom
no_lsp_dependencies
Jun 14, 2025
Merged

chore(vscode): add test for non installation of deps'#4735
benfdking merged 1 commit intomainfrom
no_lsp_dependencies

Conversation

@benfdking
Copy link
Copy Markdown
Contributor

@benfdking benfdking commented Jun 13, 2025

  • refactors some of the setup code
  • adds a test to make sure if you don't have LSP deps it asks you to add them
  • merge first fix(vscode): improve the tcloud stability

@benfdking benfdking force-pushed the no_lsp_dependencies branch from 906fd7a to 80151cc Compare June 13, 2025 23:56
@benfdking benfdking marked this pull request as ready for review June 13, 2025 23:57
@benfdking benfdking requested a review from Copilot June 13, 2025 23:59
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

Adds utilities for creating and populating Python virtual environments in the test suite, refactors existing tests to use these helpers, and introduces a new test to ensure users are prompted when LSP dependencies are missing.

  • Extracted venv creation and package installation into createVirtualEnvironment and pipInstall.
  • Updated tcloud.spec.ts to use the new helpers.
  • Added no_lsp.spec.ts to verify the “Install” prompt appears when LSP extras aren’t installed.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
vscode/extension/tests/utils.ts Added execAsync, createVirtualEnvironment, and pipInstall.
vscode/extension/tests/tcloud.spec.ts Replaced inline venv and pip calls with the new helpers.
vscode/extension/tests/no_lsp.spec.ts New test to check UI prompt for missing LSP dependencies.
Comments suppressed due to low confidence (3)

vscode/extension/tests/utils.ts:127

  • Update the JSDoc for pipInstall to reflect that it accepts multiple packages: change the description to 'Install packages in the given virtual environment.' and update @param packagePath to @param packagePaths (string[]).
* @param packagePath The path to the package to install.

vscode/extension/tests/tcloud.spec.ts:25

  • [nitpick] The variable name custom_materializations uses snake_case in a TypeScript codebase; consider renaming to customMaterializations for consistency with camelCase conventions.
const custom_materializations = path.join(

vscode/extension/tests/no_lsp.spec.ts:13

  • The test title refers to signing in but the logic checks for missing LSP dependencies; consider renaming to something like 'missing LSP dependencies shows install prompt' to match the test behavior.
test('not signed in, shows sign in window', async ({}, testInfo) => {

@benfdking benfdking force-pushed the no_lsp_dependencies branch from 80151cc to 19f214c Compare June 14, 2025 00:03
@benfdking benfdking merged commit abf9239 into main Jun 14, 2025
25 checks passed
@benfdking benfdking deleted the no_lsp_dependencies branch June 14, 2025 14:27
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