From 6b2f652d0d4e65df78ec3ef58c6fc1a923dfdff1 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 12 Apr 2023 20:24:12 -0700 Subject: [PATCH 01/19] Build VSIX --- package.json | 3 +- src/client/common/experiments/helpers.ts | 7 ----- .../terminalEnvVarCollectionService.ts | 10 +++--- ...scode.proposed.envCollectionWorkspace.d.ts | 31 +++++++++++++++++++ 4 files changed, 39 insertions(+), 12 deletions(-) create mode 100644 types/vscode.proposed.envCollectionWorkspace.d.ts diff --git a/package.json b/package.json index 08d56bdf3bc0..7eefc2945165 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,8 @@ "contribEditorContentMenu", "quickPickSortByLabel", "envShellEvent", - "testObserver" + "testObserver", + "envCollectionWorkspace" ], "author": { "name": "Microsoft Corporation" diff --git a/src/client/common/experiments/helpers.ts b/src/client/common/experiments/helpers.ts index 4aed04da3fd0..04da948fd15d 100644 --- a/src/client/common/experiments/helpers.ts +++ b/src/client/common/experiments/helpers.ts @@ -3,17 +3,10 @@ 'use strict'; -import { workspace } from 'vscode'; -import { isTestExecution } from '../constants'; import { IExperimentService } from '../types'; import { TerminalEnvVarActivation } from './groups'; export function inTerminalEnvVarExperiment(experimentService: IExperimentService): boolean { - if (workspace.workspaceFile && !isTestExecution()) { - // Don't run experiment in multi-root workspaces for now, requires work on VSCode: - // https://github.com/microsoft/vscode/issues/171173 - return false; - } if (!experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)) { return false; } diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index f5af71b3f2ca..f57a91f5438b 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -4,7 +4,7 @@ import { inject, injectable } from 'inversify'; import { ProgressOptions, ProgressLocation } from 'vscode'; import { IExtensionSingleActivationService } from '../../activation/types'; -import { IApplicationShell, IApplicationEnvironment } from '../../common/application/types'; +import { IApplicationShell, IApplicationEnvironment, IWorkspaceService } from '../../common/application/types'; import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers'; import { IPlatformService } from '../../common/platform/types'; import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors/baseShellDetector'; @@ -36,6 +36,7 @@ export class TerminalEnvVarCollectionService implements IExtensionSingleActivati @inject(IApplicationEnvironment) private applicationEnvironment: IApplicationEnvironment, @inject(IDisposableRegistry) private disposables: IDisposableRegistry, @inject(IEnvironmentActivationService) private environmentActivationService: IEnvironmentActivationService, + @inject(IWorkspaceService) private workspaceService: IWorkspaceService, ) {} public async activate(): Promise { @@ -68,6 +69,7 @@ export class TerminalEnvVarCollectionService implements IExtensionSingleActivati } public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise { + const workspaceFolder = this.workspaceService.getWorkspaceFolder(resource); const env = await this.environmentActivationService.getActivatedEnvironmentVariables( resource, undefined, @@ -95,10 +97,10 @@ export class TerminalEnvVarCollectionService implements IExtensionSingleActivati if (prevValue !== value) { if (value !== undefined) { traceVerbose(`Setting environment variable ${key} in collection to ${value}`); - this.context.environmentVariableCollection.replace(key, value); + this.context.environmentVariableCollection.replace(key, value, {workspaceFolder}); } else { traceVerbose(`Clearing environment variable ${key} from collection`); - this.context.environmentVariableCollection.delete(key); + this.context.environmentVariableCollection.delete(key, {workspaceFolder}); } } }); @@ -106,7 +108,7 @@ export class TerminalEnvVarCollectionService implements IExtensionSingleActivati // If the previous env var is not in the current env, clear it from collection. if (!(key in env)) { traceVerbose(`Clearing environment variable ${key} from collection`); - this.context.environmentVariableCollection.delete(key); + this.context.environmentVariableCollection.delete(key, {workspaceFolder}); } }); } diff --git a/types/vscode.proposed.envCollectionWorkspace.d.ts b/types/vscode.proposed.envCollectionWorkspace.d.ts new file mode 100644 index 000000000000..cdabc7764e54 --- /dev/null +++ b/types/vscode.proposed.envCollectionWorkspace.d.ts @@ -0,0 +1,31 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +declare module 'vscode' { + + // https://github.com/microsoft/vscode/issues/171173 + + export interface EnvironmentVariableMutator { + readonly type: EnvironmentVariableMutatorType; + readonly value: string; + readonly scope: EnvironmentVariableScope | undefined; + } + + export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> { + replace(variable: string, value: string, scope?: EnvironmentVariableScope): void; + append(variable: string, value: string, scope?: EnvironmentVariableScope): void; + prepend(variable: string, value: string, scope?: EnvironmentVariableScope): void; + get(variable: string, scope?: EnvironmentVariableScope): EnvironmentVariableMutator | undefined; + delete(variable: string, scope?: EnvironmentVariableScope): void; + } + + export type EnvironmentVariableScope = { + /** + * The workspace folder to which this environment variable collection applies to. + * If unspecified, the collection applies to all workspace folders. + */ + workspaceFolder?: WorkspaceFolder; + }; +} From 9ecdd05fbd455d7e880d06c6b308ccff7ba42086 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 12 Apr 2023 20:26:41 -0700 Subject: [PATCH 02/19] build vsix --- .github/workflows/pr-check.yml | 456 --------------------------------- 1 file changed, 456 deletions(-) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index ade1dcd61234..6c489b82f265 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -33,459 +33,3 @@ jobs: node_version: ${{ env.NODE_VERSION}} vsix_name: ${{ env.VSIX_NAME }} artifact_name: ${{ env.ARTIFACT_NAME_VSIX }} - - lint: - name: Lint - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v3 - - - name: Lint - uses: ./.github/actions/lint - with: - node_version: ${{ env.NODE_VERSION }} - - check-types: - name: Check Python types - runs-on: ubuntu-latest - steps: - - name: Use Python ${{ env.PYTHON_VERSION }} - uses: actions/setup-python@v4 - with: - python-version: ${{ env.PYTHON_VERSION }} - - - name: Checkout - uses: actions/checkout@v3 - - - name: Install base Python requirements - uses: brettcannon/pip-secure-install@v1 - with: - options: '-t ./pythonFiles/lib/python --no-cache-dir --implementation py' - - - name: Install Jedi requirements - uses: brettcannon/pip-secure-install@v1 - with: - requirements-file: './pythonFiles/jedilsp_requirements/requirements.txt' - options: '-t ./pythonFiles/lib/jedilsp --no-cache-dir --implementation py' - - - name: Install other Python requirements - run: | - python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --no-cache-dir --implementation py --no-deps --upgrade --pre debugpy - python -m pip install --upgrade -r build/test-requirements.txt - - - name: Run Pyright - uses: jakebailey/pyright-action@v1 - with: - working-directory: 'pythonFiles' - - ### Non-smoke tests - tests: - name: Tests - # The value of runs-on is the OS of the current job (specified in the strategy matrix below) instead of being hardcoded. - runs-on: ${{ matrix.os }} - defaults: - run: - working-directory: ${{ env.special-working-directory }} - strategy: - fail-fast: false - matrix: - # We're not running CI on macOS for now because it's one less matrix entry to lower the number of runners used, - # macOS runners are expensive, and we assume that Ubuntu is enough to cover the Unix case. - os: [ubuntu-latest, windows-latest] - # Run the tests on the oldest and most recent versions of Python. - python: ['3.x'] - test-suite: [ts-unit, python-unit, venv, single-workspace, debugger, functional] - - steps: - - name: Checkout - uses: actions/checkout@v3 - with: - path: ${{ env.special-working-directory-relative }} - - - name: Install Node - uses: actions/setup-node@v3 - with: - node-version: ${{ env.NODE_VERSION }} - cache: 'npm' - cache-dependency-path: ${{ env.special-working-directory-relative }}/package-lock.json - - - name: Install dependencies (npm ci) - run: npm ci - - - name: Compile - run: npx gulp prePublishNonBundle - - - name: Use Python ${{ matrix.python }} - uses: actions/setup-python@v4 - with: - python-version: ${{ matrix.python }} - - - name: Install debugpy - run: | - # We need to have debugpy so that tests relying on it keep passing, but we don't need install_debugpy's logic in the test phase. - python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --no-cache-dir --implementation py --no-deps --upgrade --pre debugpy - - - name: Install base Python requirements - uses: brettcannon/pip-secure-install@v1 - with: - requirements-file: '"${{ env.special-working-directory-relative }}/requirements.txt"' - options: '-t "${{ env.special-working-directory-relative }}/pythonFiles/lib/python" --no-cache-dir --implementation py' - if: startsWith(matrix.python, 3.) - - - name: Install Jedi requirements - uses: brettcannon/pip-secure-install@v1 - with: - requirements-file: '"${{ env.special-working-directory-relative }}/pythonFiles/jedilsp_requirements/requirements.txt"' - options: '-t "${{ env.special-working-directory-relative }}/pythonFiles/lib/jedilsp" --no-cache-dir --implementation py' - if: startsWith(matrix.python, 3.) - - - name: Install test requirements - run: python -m pip install --upgrade -r build/test-requirements.txt - - - name: Install debugpy wheels (Python ${{ matrix.python }}) - run: | - python -m pip install wheel - python -m pip --disable-pip-version-check install -r build/debugger-install-requirements.txt - python ./pythonFiles/install_debugpy.py - shell: bash - if: matrix.test-suite == 'debugger' - - - name: Install functional test requirements - run: python -m pip install --upgrade -r ./build/functional-test-requirements.txt - if: matrix.test-suite == 'functional' - - - name: Prepare pipenv for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - if: matrix.test-suite == 'venv' - run: | - python -m pip install pipenv - python -m pipenv run python ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} pipenvPath - - - name: Prepare poetry for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - shell: pwsh - if: matrix.test-suite == 'venv' - run: | - python -m pip install poetry - Move-Item -Path ".\build\ci\pyproject.toml" -Destination . - poetry env use python - - - name: Prepare virtualenv for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - if: matrix.test-suite == 'venv' - run: | - python -m pip install virtualenv - python -m virtualenv .virtualenv/ - if ('${{ matrix.os }}' -match 'windows-latest') { - & ".virtualenv/Scripts/python.exe" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} virtualEnvPath - } else { - & ".virtualenv/bin/python" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} virtualEnvPath - } - - - name: Prepare venv for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - if: matrix.test-suite == 'venv' && startsWith(matrix.python, 3.) - run: | - python -m venv .venv - if ('${{ matrix.os }}' -match 'windows-latest') { - & ".venv/Scripts/python.exe" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} venvPath - } else { - & ".venv/bin/python" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} venvPath - } - - - name: Prepare conda for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - if: matrix.test-suite == 'venv' - run: | - # 1. For `terminalActivation.testvirtualenvs.test.ts` - if ('${{ matrix.os }}' -match 'windows-latest') { - $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath python.exe - $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath Scripts | Join-Path -ChildPath conda - } else{ - $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath bin | Join-Path -ChildPath python - $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath bin | Join-Path -ChildPath conda - } - & $condaPythonPath ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} condaExecPath $condaExecPath - & $condaPythonPath ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} condaPath - & $condaExecPath init --all - - - name: Set CI_PYTHON_PATH and CI_DISABLE_AUTO_SELECTION - run: | - echo "CI_PYTHON_PATH=python" >> $GITHUB_ENV - echo "CI_DISABLE_AUTO_SELECTION=1" >> $GITHUB_ENV - shell: bash - if: matrix.test-suite != 'ts-unit' - - # Run TypeScript unit tests only for Python 3.X. - - name: Run TypeScript unit tests - run: npm run test:unittests - if: matrix.test-suite == 'ts-unit' && startsWith(matrix.python, 3.) - - # Run the Python tests in our codebase. - - name: Run Python unit tests - run: | - python pythonFiles/tests/run_all.py - if: matrix.test-suite == 'python-unit' - - # The virtual environment based tests use the `testSingleWorkspace` set of tests - # with the environment variable `TEST_FILES_SUFFIX` set to `testvirtualenvs`, - # which is set in the "Prepare environment for venv tests" step. - # We also use a third-party GitHub Action to install xvfb on Linux, - # run tests and then clean up the process once the tests ran. - # See https://github.com/GabrielBB/xvfb-action - - name: Run venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - CI_PYTHON_VERSION: ${{ matrix.python }} - uses: GabrielBB/xvfb-action@v1.6 - with: - run: npm run testSingleWorkspace - working-directory: ${{ env.special-working-directory }} - if: matrix.test-suite == 'venv' && matrix.os == 'ubuntu-latest' - - - name: Run single-workspace tests - env: - CI_PYTHON_VERSION: ${{ matrix.python }} - uses: GabrielBB/xvfb-action@v1.6 - with: - run: npm run testSingleWorkspace - working-directory: ${{ env.special-working-directory }} - if: matrix.test-suite == 'single-workspace' - - - name: Run debugger tests - env: - CI_PYTHON_VERSION: ${{ matrix.python }} - uses: GabrielBB/xvfb-action@v1.6 - with: - run: npm run testDebugger - working-directory: ${{ env.special-working-directory }} - if: matrix.test-suite == 'debugger' - - # Run TypeScript functional tests - - name: Run TypeScript functional tests - run: npm run test:functional - if: matrix.test-suite == 'functional' - - smoke-tests: - name: Smoke tests - # The value of runs-on is the OS of the current job (specified in the strategy matrix below) instead of being hardcoded. - runs-on: ${{ matrix.os }} - needs: [build-vsix] - strategy: - fail-fast: false - matrix: - # We're not running CI on macOS for now because it's one less matrix entry to lower the number of runners used, - # macOS runners are expensive, and we assume that Ubuntu is enough to cover the UNIX case. - os: [ubuntu-latest, windows-latest] - steps: - # Need the source to have the tests available. - - name: Checkout - uses: actions/checkout@v3 - - - name: Smoke tests - uses: ./.github/actions/smoke-tests - with: - node_version: ${{ env.NODE_VERSION }} - artifact_name: ${{ env.ARTIFACT_NAME_VSIX }} - - ### Coverage run - coverage: - name: Coverage - # The value of runs-on is the OS of the current job (specified in the strategy matrix below) instead of being hardcoded. - runs-on: ${{ matrix.os }} - strategy: - fail-fast: false - matrix: - # Only run coverage on linux for PRs - os: [ubuntu-latest] - - steps: - - name: Checkout - uses: actions/checkout@v3 - - - name: Install Node - uses: actions/setup-node@v3 - with: - node-version: ${{ env.NODE_VERSION }} - cache: 'npm' - - - name: Install dependencies (npm ci) - run: npm ci - - - name: Compile - run: npx gulp prePublishNonBundle - - - name: Use Python ${{ env.PYTHON_VERSION }} - uses: actions/setup-python@v4 - with: - python-version: ${{ env.PYTHON_VERSION }} - cache: 'pip' - cache-dependency-path: | - requirements.txt - pythonFiles/jedilsp_requirements/requirements.txt - build/test-requirements.txt - build/functional-test-requirements.txt - - - name: Install base Python requirements - uses: brettcannon/pip-secure-install@v1 - with: - options: '-t ./pythonFiles/lib/python --implementation py' - - - name: Install Jedi requirements - uses: brettcannon/pip-secure-install@v1 - with: - requirements-file: './pythonFiles/jedilsp_requirements/requirements.txt' - options: '-t ./pythonFiles/lib/jedilsp --implementation py' - - - name: Install debugpy - run: | - # We need to have debugpy so that tests relying on it keep passing, but we don't need install_debugpy's logic in the test phase. - python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --implementation py --no-deps --upgrade --pre debugpy - - - name: Install test requirements - run: python -m pip install --upgrade -r build/test-requirements.txt - - - name: Install functional test requirements - run: python -m pip install --upgrade -r ./build/functional-test-requirements.txt - - - name: Prepare pipenv for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - run: | - python -m pip install pipenv - python -m pipenv run python ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} pipenvPath - - - name: Prepare poetry for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - shell: pwsh - run: | - python -m pip install poetry - Move-Item -Path ".\build\ci\pyproject.toml" -Destination . - poetry env use python - - - name: Prepare virtualenv for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - run: | - python -m pip install virtualenv - python -m virtualenv .virtualenv/ - if ('${{ matrix.os }}' -match 'windows-latest') { - & ".virtualenv/Scripts/python.exe" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} virtualEnvPath - } else { - & ".virtualenv/bin/python" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} virtualEnvPath - } - - - name: Prepare venv for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - run: | - python -m venv .venv - if ('${{ matrix.os }}' -match 'windows-latest') { - & ".venv/Scripts/python.exe" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} venvPath - } else { - & ".venv/bin/python" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} venvPath - } - - - name: Prepare conda for venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' - shell: pwsh - run: | - # 1. For `terminalActivation.testvirtualenvs.test.ts` - if ('${{ matrix.os }}' -match 'windows-latest') { - $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath python.exe - $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath Scripts | Join-Path -ChildPath conda - } else{ - $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath bin | Join-Path -ChildPath python - $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath bin | Join-Path -ChildPath conda - } - & $condaPythonPath ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} condaExecPath $condaExecPath - & $condaPythonPath ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} condaPath - & $condaExecPath init --all - - - name: Run TypeScript unit tests - run: npm run test:unittests:cover - - - name: Run Python unit tests - run: | - python pythonFiles/tests/run_all.py - - # The virtual environment based tests use the `testSingleWorkspace` set of tests - # with the environment variable `TEST_FILES_SUFFIX` set to `testvirtualenvs`, - # which is set in the "Prepare environment for venv tests" step. - # We also use a third-party GitHub Action to install xvfb on Linux, - # run tests and then clean up the process once the tests ran. - # See https://github.com/GabrielBB/xvfb-action - - name: Run venv tests - env: - TEST_FILES_SUFFIX: testvirtualenvs - CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} - CI_DISABLE_AUTO_SELECTION: 1 - uses: GabrielBB/xvfb-action@v1.6 - with: - run: npm run testSingleWorkspace:cover - - - name: Run single-workspace tests - env: - CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} - CI_DISABLE_AUTO_SELECTION: 1 - uses: GabrielBB/xvfb-action@v1.6 - with: - run: npm run testSingleWorkspace:cover - - # Enable these tests when coverage is setup for multiroot workspace tests - # - name: Run multi-workspace tests - # env: - # CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} - # CI_DISABLE_AUTO_SELECTION: 1 - # uses: GabrielBB/xvfb-action@v1.6 - # with: - # run: npm run testMultiWorkspace:cover - - # Enable these tests when coverage is setup for debugger tests - # - name: Run debugger tests - # env: - # CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} - # CI_DISABLE_AUTO_SELECTION: 1 - # uses: GabrielBB/xvfb-action@v1.6 - # with: - # run: npm run testDebugger:cover - - # Run TypeScript functional tests - - name: Run TypeScript functional tests - env: - CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} - CI_DISABLE_AUTO_SELECTION: 1 - run: npm run test:functional:cover - - - name: Generate coverage reports - run: npm run test:cover:report - - - name: Upload HTML report - uses: actions/upload-artifact@v3 - with: - name: ${{ runner.os }}-coverage-report-html - path: ./coverage - retention-days: 1 From 9fd0f70580e8f6e68e7c0c8e6fa9535ea9b7842b Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 12 Apr 2023 20:44:19 -0700 Subject: [PATCH 03/19] yo --- ...rminalEnvVarCollectionService.unit.test.ts | 494 +++++++++--------- 1 file changed, 247 insertions(+), 247 deletions(-) diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 324de1174584..5baaec41ab74 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -1,247 +1,247 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import * as sinon from 'sinon'; -import { assert, expect } from 'chai'; -import { cloneDeep } from 'lodash'; -import { mock, instance, when, anything, verify, reset } from 'ts-mockito'; -import { EnvironmentVariableCollection, ProgressLocation, Uri } from 'vscode'; -import { IApplicationShell, IApplicationEnvironment } from '../../../client/common/application/types'; -import { TerminalEnvVarActivation } from '../../../client/common/experiments/groups'; -import { IPlatformService } from '../../../client/common/platform/types'; -import { - IExtensionContext, - IExperimentService, - Resource, - IConfigurationService, - IPythonSettings, -} from '../../../client/common/types'; -import { Interpreters } from '../../../client/common/utils/localize'; -import { getOSType } from '../../../client/common/utils/platform'; -import { defaultShells } from '../../../client/interpreter/activation/service'; -import { - TerminalEnvVarCollectionService, - _normCaseKeys, -} from '../../../client/interpreter/activation/terminalEnvVarCollectionService'; -import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; -import { IInterpreterService } from '../../../client/interpreter/contracts'; - -suite('Terminal Environment Variable Collection Service', () => { - let platform: IPlatformService; - let interpreterService: IInterpreterService; - let context: IExtensionContext; - let shell: IApplicationShell; - let experimentService: IExperimentService; - let collection: EnvironmentVariableCollection; - let applicationEnvironment: IApplicationEnvironment; - let environmentActivationService: IEnvironmentActivationService; - let terminalEnvVarCollectionService: TerminalEnvVarCollectionService; - const progressOptions = { - location: ProgressLocation.Window, - title: Interpreters.activatingTerminals, - }; - const customShell = 'powershell'; - const defaultShell = defaultShells[getOSType()]; - - setup(() => { - platform = mock(); - when(platform.osType).thenReturn(getOSType()); - interpreterService = mock(); - context = mock(); - shell = mock(); - collection = mock(); - when(context.environmentVariableCollection).thenReturn(instance(collection)); - experimentService = mock(); - when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true); - applicationEnvironment = mock(); - when(applicationEnvironment.shell).thenReturn(customShell); - when(shell.withProgress(anything(), anything())) - .thenCall((options, _) => { - expect(options).to.deep.equal(progressOptions); - }) - .thenResolve(); - environmentActivationService = mock(); - const configService = mock(); - when(configService.getSettings(anything())).thenReturn(({ - terminal: { activateEnvironment: true }, - } as unknown) as IPythonSettings); - terminalEnvVarCollectionService = new TerminalEnvVarCollectionService( - instance(platform), - instance(interpreterService), - instance(context), - instance(shell), - instance(experimentService), - instance(applicationEnvironment), - [], - instance(environmentActivationService), - instance(configService), - ); - }); - - teardown(() => { - sinon.restore(); - }); - - test('Apply activated variables to the collection on activation', async () => { - const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); - applyCollectionStub.resolves(); - when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); - when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); - await terminalEnvVarCollectionService.activate(); - assert(applyCollectionStub.calledOnce, 'Collection not applied on activation'); - }); - - test('When not in experiment, do not apply activated variables to the collection and clear it instead', async () => { - reset(experimentService); - when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(false); - const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); - applyCollectionStub.resolves(); - when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); - when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); - - await terminalEnvVarCollectionService.activate(); - - verify(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).never(); - verify(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).never(); - assert(applyCollectionStub.notCalled, 'Collection should not be applied on activation'); - - verify(collection.clear()).once(); - }); - - test('When interpreter changes, apply new activated variables to the collection', async () => { - const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); - applyCollectionStub.resolves(); - const resource = Uri.file('x'); - let callback: (resource: Resource) => Promise; - when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenCall((cb) => { - callback = cb; - }); - when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); - await terminalEnvVarCollectionService.activate(); - - await callback!(resource); - assert(applyCollectionStub.calledWithExactly(resource)); - }); - - test('When selected shell changes, apply new activated variables to the collection', async () => { - const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); - applyCollectionStub.resolves(); - let callback: (shell: string) => Promise; - when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenCall((cb) => { - callback = cb; - }); - when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); - await terminalEnvVarCollectionService.activate(); - - await callback!(customShell); - assert(applyCollectionStub.calledWithExactly(undefined, customShell)); - }); - - test('If activated variables are returned for custom shell, apply it correctly to the collection', async () => { - const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; - delete envVars.PATH; - when( - environmentActivationService.getActivatedEnvironmentVariables( - anything(), - undefined, - undefined, - customShell, - ), - ).thenResolve(envVars); - - when(collection.replace(anything(), anything())).thenResolve(); - when(collection.delete(anything())).thenResolve(); - - await terminalEnvVarCollectionService._applyCollection(undefined, customShell); - - verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda')).once(); - verify(collection.delete('PATH')).once(); - }); - - test('Only relative changes to previously applied variables are applied to the collection', async () => { - const envVars: NodeJS.ProcessEnv = { - RANDOM_VAR: 'random', - CONDA_PREFIX: 'prefix/to/conda', - ..._normCaseKeys(process.env), - }; - when( - environmentActivationService.getActivatedEnvironmentVariables( - anything(), - undefined, - undefined, - customShell, - ), - ).thenResolve(envVars); - - when(collection.replace(anything(), anything())).thenResolve(); - when(collection.delete(anything())).thenResolve(); - - await terminalEnvVarCollectionService._applyCollection(undefined, customShell); - - const newEnvVars = cloneDeep(envVars); - delete newEnvVars.CONDA_PREFIX; - newEnvVars.RANDOM_VAR = undefined; // Deleting the variable from the collection is the same as setting it to undefined. - reset(environmentActivationService); - when( - environmentActivationService.getActivatedEnvironmentVariables( - anything(), - undefined, - undefined, - customShell, - ), - ).thenResolve(newEnvVars); - - await terminalEnvVarCollectionService._applyCollection(undefined, customShell); - - verify(collection.delete('CONDA_PREFIX')).once(); - verify(collection.delete('RANDOM_VAR')).once(); - }); - - test('If no activated variables are returned for custom shell, fallback to using default shell', async () => { - when( - environmentActivationService.getActivatedEnvironmentVariables( - anything(), - undefined, - undefined, - customShell, - ), - ).thenResolve(undefined); - const envVars = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; - when( - environmentActivationService.getActivatedEnvironmentVariables( - anything(), - undefined, - undefined, - defaultShell?.shell, - ), - ).thenResolve(envVars); - - when(collection.replace(anything(), anything())).thenResolve(); - when(collection.delete(anything())).thenResolve(); - - await terminalEnvVarCollectionService._applyCollection(undefined, customShell); - - verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda')).once(); - verify(collection.delete(anything())).never(); - }); - - test('If no activated variables are returned for default shell, clear collection', async () => { - when( - environmentActivationService.getActivatedEnvironmentVariables( - anything(), - undefined, - undefined, - defaultShell?.shell, - ), - ).thenResolve(undefined); - - when(collection.replace(anything(), anything())).thenResolve(); - when(collection.delete(anything())).thenResolve(); - - await terminalEnvVarCollectionService._applyCollection(undefined, defaultShell?.shell); - - verify(collection.clear()).once(); - }); -}); +// // Copyright (c) Microsoft Corporation. All rights reserved. +// // Licensed under the MIT License. + +// 'use strict'; + +// import * as sinon from 'sinon'; +// import { assert, expect } from 'chai'; +// import { cloneDeep } from 'lodash'; +// import { mock, instance, when, anything, verify, reset } from 'ts-mockito'; +// import { EnvironmentVariableCollection, ProgressLocation, Uri } from 'vscode'; +// import { IApplicationShell, IApplicationEnvironment } from '../../../client/common/application/types'; +// import { TerminalEnvVarActivation } from '../../../client/common/experiments/groups'; +// import { IPlatformService } from '../../../client/common/platform/types'; +// import { +// IExtensionContext, +// IExperimentService, +// Resource, +// IConfigurationService, +// IPythonSettings, +// } from '../../../client/common/types'; +// import { Interpreters } from '../../../client/common/utils/localize'; +// import { getOSType } from '../../../client/common/utils/platform'; +// import { defaultShells } from '../../../client/interpreter/activation/service'; +// import { +// TerminalEnvVarCollectionService, +// _normCaseKeys, +// } from '../../../client/interpreter/activation/terminalEnvVarCollectionService'; +// import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; +// import { IInterpreterService } from '../../../client/interpreter/contracts'; + +// suite('Terminal Environment Variable Collection Service', () => { +// let platform: IPlatformService; +// let interpreterService: IInterpreterService; +// let context: IExtensionContext; +// let shell: IApplicationShell; +// let experimentService: IExperimentService; +// let collection: EnvironmentVariableCollection; +// let applicationEnvironment: IApplicationEnvironment; +// let environmentActivationService: IEnvironmentActivationService; +// let terminalEnvVarCollectionService: TerminalEnvVarCollectionService; +// const progressOptions = { +// location: ProgressLocation.Window, +// title: Interpreters.activatingTerminals, +// }; +// const customShell = 'powershell'; +// const defaultShell = defaultShells[getOSType()]; + +// setup(() => { +// platform = mock(); +// when(platform.osType).thenReturn(getOSType()); +// interpreterService = mock(); +// context = mock(); +// shell = mock(); +// collection = mock(); +// when(context.environmentVariableCollection).thenReturn(instance(collection)); +// experimentService = mock(); +// when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true); +// applicationEnvironment = mock(); +// when(applicationEnvironment.shell).thenReturn(customShell); +// when(shell.withProgress(anything(), anything())) +// .thenCall((options, _) => { +// expect(options).to.deep.equal(progressOptions); +// }) +// .thenResolve(); +// environmentActivationService = mock(); +// const configService = mock(); +// when(configService.getSettings(anything())).thenReturn(({ +// terminal: { activateEnvironment: true }, +// } as unknown) as IPythonSettings); +// terminalEnvVarCollectionService = new TerminalEnvVarCollectionService( +// instance(platform), +// instance(interpreterService), +// instance(context), +// instance(shell), +// instance(experimentService), +// instance(applicationEnvironment), +// [], +// instance(environmentActivationService), +// instance(configService), +// ); +// }); + +// teardown(() => { +// sinon.restore(); +// }); + +// test('Apply activated variables to the collection on activation', async () => { +// const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); +// applyCollectionStub.resolves(); +// when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); +// when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); +// await terminalEnvVarCollectionService.activate(); +// assert(applyCollectionStub.calledOnce, 'Collection not applied on activation'); +// }); + +// test('When not in experiment, do not apply activated variables to the collection and clear it instead', async () => { +// reset(experimentService); +// when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(false); +// const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); +// applyCollectionStub.resolves(); +// when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); +// when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); + +// await terminalEnvVarCollectionService.activate(); + +// verify(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).never(); +// verify(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).never(); +// assert(applyCollectionStub.notCalled, 'Collection should not be applied on activation'); + +// verify(collection.clear()).once(); +// }); + +// test('When interpreter changes, apply new activated variables to the collection', async () => { +// const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); +// applyCollectionStub.resolves(); +// const resource = Uri.file('x'); +// let callback: (resource: Resource) => Promise; +// when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenCall((cb) => { +// callback = cb; +// }); +// when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); +// await terminalEnvVarCollectionService.activate(); + +// await callback!(resource); +// assert(applyCollectionStub.calledWithExactly(resource)); +// }); + +// test('When selected shell changes, apply new activated variables to the collection', async () => { +// const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); +// applyCollectionStub.resolves(); +// let callback: (shell: string) => Promise; +// when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenCall((cb) => { +// callback = cb; +// }); +// when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); +// await terminalEnvVarCollectionService.activate(); + +// await callback!(customShell); +// assert(applyCollectionStub.calledWithExactly(undefined, customShell)); +// }); + +// test('If activated variables are returned for custom shell, apply it correctly to the collection', async () => { +// const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; +// delete envVars.PATH; +// when( +// environmentActivationService.getActivatedEnvironmentVariables( +// anything(), +// undefined, +// undefined, +// customShell, +// ), +// ).thenResolve(envVars); + +// when(collection.replace(anything(), anything())).thenResolve(); +// when(collection.delete(anything())).thenResolve(); + +// await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + +// verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda')).once(); +// verify(collection.delete('PATH')).once(); +// }); + +// test('Only relative changes to previously applied variables are applied to the collection', async () => { +// const envVars: NodeJS.ProcessEnv = { +// RANDOM_VAR: 'random', +// CONDA_PREFIX: 'prefix/to/conda', +// ..._normCaseKeys(process.env), +// }; +// when( +// environmentActivationService.getActivatedEnvironmentVariables( +// anything(), +// undefined, +// undefined, +// customShell, +// ), +// ).thenResolve(envVars); + +// when(collection.replace(anything(), anything())).thenResolve(); +// when(collection.delete(anything())).thenResolve(); + +// await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + +// const newEnvVars = cloneDeep(envVars); +// delete newEnvVars.CONDA_PREFIX; +// newEnvVars.RANDOM_VAR = undefined; // Deleting the variable from the collection is the same as setting it to undefined. +// reset(environmentActivationService); +// when( +// environmentActivationService.getActivatedEnvironmentVariables( +// anything(), +// undefined, +// undefined, +// customShell, +// ), +// ).thenResolve(newEnvVars); + +// await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + +// verify(collection.delete('CONDA_PREFIX')).once(); +// verify(collection.delete('RANDOM_VAR')).once(); +// }); + +// test('If no activated variables are returned for custom shell, fallback to using default shell', async () => { +// when( +// environmentActivationService.getActivatedEnvironmentVariables( +// anything(), +// undefined, +// undefined, +// customShell, +// ), +// ).thenResolve(undefined); +// const envVars = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; +// when( +// environmentActivationService.getActivatedEnvironmentVariables( +// anything(), +// undefined, +// undefined, +// defaultShell?.shell, +// ), +// ).thenResolve(envVars); + +// when(collection.replace(anything(), anything())).thenResolve(); +// when(collection.delete(anything())).thenResolve(); + +// await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + +// verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda')).once(); +// verify(collection.delete(anything())).never(); +// }); + +// test('If no activated variables are returned for default shell, clear collection', async () => { +// when( +// environmentActivationService.getActivatedEnvironmentVariables( +// anything(), +// undefined, +// undefined, +// defaultShell?.shell, +// ), +// ).thenResolve(undefined); + +// when(collection.replace(anything(), anything())).thenResolve(); +// when(collection.delete(anything())).thenResolve(); + +// await terminalEnvVarCollectionService._applyCollection(undefined, defaultShell?.shell); + +// verify(collection.clear()).once(); +// }); +// }); From c27fd0c520ee0157d498db38fe8c9a7d7a05341b Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 12 Apr 2023 20:45:56 -0700 Subject: [PATCH 04/19] Switch package.json --- package.json | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/package.json b/package.json index 8274f15f5e75..4de023fe09c8 100644 --- a/package.json +++ b/package.json @@ -17,13 +17,7 @@ } }, "publisher": "ms-python", - "enabledApiProposals": [ - "contribEditorContentMenu", - "quickPickSortByLabel", - "envShellEvent", - "testObserver", - "envCollectionWorkspace" - ], + "enableProposedApi": true, "author": { "name": "Microsoft Corporation" }, From 40e215471610cd22135b24a51f90728a7011a405 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 13 Apr 2023 00:41:09 -0700 Subject: [PATCH 05/19] More fixes --- package.json | 8 ++++- .../terminalEnvVarCollectionService.ts | 29 +++++++++++-------- src/client/interpreter/serviceRegistry.ts | 4 +-- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index 4de023fe09c8..8274f15f5e75 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,13 @@ } }, "publisher": "ms-python", - "enableProposedApi": true, + "enabledApiProposals": [ + "contribEditorContentMenu", + "quickPickSortByLabel", + "envShellEvent", + "testObserver", + "envCollectionWorkspace" + ], "author": { "name": "Microsoft Corporation" }, diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 408cbb2094e2..dc73d0520db6 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -3,7 +3,7 @@ import { inject, injectable } from 'inversify'; import { ProgressOptions, ProgressLocation } from 'vscode'; -import { IExtensionSingleActivationService } from '../../activation/types'; +import { IExtensionActivationService } from '../../activation/types'; import { IApplicationShell, IApplicationEnvironment, IWorkspaceService } from '../../common/application/types'; import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers'; import { IPlatformService } from '../../common/platform/types'; @@ -21,9 +21,10 @@ import { traceDecoratorVerbose, traceVerbose } from '../../logging'; import { IInterpreterService } from '../contracts'; import { defaultShells } from './service'; import { IEnvironmentActivationService } from './types'; +import { PythonSettings } from '../../common/configSettings'; @injectable() -export class TerminalEnvVarCollectionService implements IExtensionSingleActivationService { +export class TerminalEnvVarCollectionService implements IExtensionActivationService { public readonly supportedWorkspaceTypes = { untrustedWorkspace: false, virtualWorkspace: false, @@ -44,13 +45,8 @@ export class TerminalEnvVarCollectionService implements IExtensionSingleActivati @inject(IEnvironmentActivationService) private environmentActivationService: IEnvironmentActivationService, @inject(IWorkspaceService) private workspaceService: IWorkspaceService, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, - ) {} - - public async activate(): Promise { - if (!inTerminalEnvVarExperiment(this.experimentService)) { - this.context.environmentVariableCollection.clear(); - return; - } + ) { + this.context.environmentVariableCollection.clear(); this.interpreterService.onDidChangeInterpreter( async (resource) => { this.showProgress(); @@ -70,13 +66,22 @@ export class TerminalEnvVarCollectionService implements IExtensionSingleActivati }, this, this.disposables, - ); + );} - this._applyCollection(undefined).ignoreErrors(); + public async activate(resource: Resource): Promise { + if (!inTerminalEnvVarExperiment(this.experimentService)) { + this.context.environmentVariableCollection.clear(); + return; + } + this._applyCollection(resource).ignoreErrors(); } public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise { - const workspaceFolder = this.workspaceService.getWorkspaceFolder(resource); + let workspaceFolder = this.workspaceService.getWorkspaceFolder(resource); + if (!workspaceFolder && Array.isArray(this.workspaceService.workspaceFolders) && this.workspaceService.workspaceFolders.length > 0) { + workspaceFolder = this.workspaceService.workspaceFolders[0]; + } + console.log('Use workspace folder', workspaceFolder?.uri.fsPath); const settings = this.configurationService.getSettings(resource); if (!settings.terminal.activateEnvironment) { traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath); diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index 15dd6de7b722..04af15415b04 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -109,8 +109,8 @@ export function registerTypes(serviceManager: IServiceManager): void { IEnvironmentActivationService, EnvironmentActivationService, ); - serviceManager.addSingleton( - IExtensionSingleActivationService, + serviceManager.addSingleton( + IExtensionActivationService, TerminalEnvVarCollectionService, ); } From 4f9a51e1538b566817d04e4c1d9bc67cec8a2704 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 13 Apr 2023 00:47:20 -0700 Subject: [PATCH 06/19] oops --- .../interpreter/activation/terminalEnvVarCollectionService.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index dc73d0520db6..f1adbdaa4a4f 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -21,7 +21,6 @@ import { traceDecoratorVerbose, traceVerbose } from '../../logging'; import { IInterpreterService } from '../contracts'; import { defaultShells } from './service'; import { IEnvironmentActivationService } from './types'; -import { PythonSettings } from '../../common/configSettings'; @injectable() export class TerminalEnvVarCollectionService implements IExtensionActivationService { From 460e5babc81c3ba642474853f2067f7ea818e3e8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 13 Apr 2023 13:09:26 -0700 Subject: [PATCH 07/19] Mkae env collection simpler for testing --- .../terminalEnvVarCollectionService.ts | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index f1adbdaa4a4f..4deaaa414874 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -21,6 +21,7 @@ import { traceDecoratorVerbose, traceVerbose } from '../../logging'; import { IInterpreterService } from '../contracts'; import { defaultShells } from './service'; import { IEnvironmentActivationService } from './types'; +import { EnvironmentVariables } from '../../common/variables/types'; @injectable() export class TerminalEnvVarCollectionService implements IExtensionActivationService { @@ -33,6 +34,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ private previousEnvVars = _normCaseKeys(process.env); + public random = this._environmentActivationService; + constructor( @inject(IPlatformService) private readonly platform: IPlatformService, @inject(IInterpreterService) private interpreterService: IInterpreterService, @@ -41,7 +44,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ @inject(IExperimentService) private experimentService: IExperimentService, @inject(IApplicationEnvironment) private applicationEnvironment: IApplicationEnvironment, @inject(IDisposableRegistry) private disposables: IDisposableRegistry, - @inject(IEnvironmentActivationService) private environmentActivationService: IEnvironmentActivationService, + @inject(IEnvironmentActivationService) private _environmentActivationService: IEnvironmentActivationService, @inject(IWorkspaceService) private workspaceService: IWorkspaceService, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, ) { @@ -56,16 +59,17 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ this.disposables, ); this.applicationEnvironment.onDidChangeShell( - async (shell: string) => { + async (s: string) => { this.showProgress(); // Pass in the shell where known instead of relying on the application environment, because of bug // on VSCode: https://github.com/microsoft/vscode/issues/160694 - await this._applyCollection(undefined, shell); + await this._applyCollection(undefined, s); this.hideProgress(); }, this, this.disposables, - );} + ); + } public async activate(resource: Resource): Promise { if (!inTerminalEnvVarExperiment(this.experimentService)) { @@ -77,8 +81,12 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise { let workspaceFolder = this.workspaceService.getWorkspaceFolder(resource); - if (!workspaceFolder && Array.isArray(this.workspaceService.workspaceFolders) && this.workspaceService.workspaceFolders.length > 0) { - workspaceFolder = this.workspaceService.workspaceFolders[0]; + if ( + !workspaceFolder && + Array.isArray(this.workspaceService.workspaceFolders) && + this.workspaceService.workspaceFolders.length > 0 + ) { + [workspaceFolder] = this.workspaceService.workspaceFolders; } console.log('Use workspace folder', workspaceFolder?.uri.fsPath); const settings = this.configurationService.getSettings(resource); @@ -86,12 +94,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath); return; } - const env = await this.environmentActivationService.getActivatedEnvironmentVariables( - resource, - undefined, - undefined, - shell, - ); + const env: EnvironmentVariables = { HELLO: workspaceFolder?.name }; if (!env) { const shellType = identifyShellFromShellPath(shell); const defaultShell = defaultShells[this.platform.osType]; @@ -113,10 +116,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ if (prevValue !== value) { if (value !== undefined) { traceVerbose(`Setting environment variable ${key} in collection to ${value}`); - this.context.environmentVariableCollection.replace(key, value, {workspaceFolder}); + this.context.environmentVariableCollection.replace(key, value, { workspaceFolder }); } else { traceVerbose(`Clearing environment variable ${key} from collection`); - this.context.environmentVariableCollection.delete(key, {workspaceFolder}); + this.context.environmentVariableCollection.delete(key, { workspaceFolder }); } } }); @@ -124,7 +127,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ // If the previous env var is not in the current env, clear it from collection. if (!(key in env)) { traceVerbose(`Clearing environment variable ${key} from collection`); - this.context.environmentVariableCollection.delete(key, {workspaceFolder}); + this.context.environmentVariableCollection.delete(key, { workspaceFolder }); } }); } From 28b934732b2ab207bb93a091b6da0ca90fda48bb Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 14 Apr 2023 14:02:05 -0700 Subject: [PATCH 08/19] Revert "Mkae env collection simpler for testing" This reverts commit 460e5babc81c3ba642474853f2067f7ea818e3e8. --- .../terminalEnvVarCollectionService.ts | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 4deaaa414874..f1adbdaa4a4f 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -21,7 +21,6 @@ import { traceDecoratorVerbose, traceVerbose } from '../../logging'; import { IInterpreterService } from '../contracts'; import { defaultShells } from './service'; import { IEnvironmentActivationService } from './types'; -import { EnvironmentVariables } from '../../common/variables/types'; @injectable() export class TerminalEnvVarCollectionService implements IExtensionActivationService { @@ -34,8 +33,6 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ private previousEnvVars = _normCaseKeys(process.env); - public random = this._environmentActivationService; - constructor( @inject(IPlatformService) private readonly platform: IPlatformService, @inject(IInterpreterService) private interpreterService: IInterpreterService, @@ -44,7 +41,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ @inject(IExperimentService) private experimentService: IExperimentService, @inject(IApplicationEnvironment) private applicationEnvironment: IApplicationEnvironment, @inject(IDisposableRegistry) private disposables: IDisposableRegistry, - @inject(IEnvironmentActivationService) private _environmentActivationService: IEnvironmentActivationService, + @inject(IEnvironmentActivationService) private environmentActivationService: IEnvironmentActivationService, @inject(IWorkspaceService) private workspaceService: IWorkspaceService, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, ) { @@ -59,17 +56,16 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ this.disposables, ); this.applicationEnvironment.onDidChangeShell( - async (s: string) => { + async (shell: string) => { this.showProgress(); // Pass in the shell where known instead of relying on the application environment, because of bug // on VSCode: https://github.com/microsoft/vscode/issues/160694 - await this._applyCollection(undefined, s); + await this._applyCollection(undefined, shell); this.hideProgress(); }, this, this.disposables, - ); - } + );} public async activate(resource: Resource): Promise { if (!inTerminalEnvVarExperiment(this.experimentService)) { @@ -81,12 +77,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise { let workspaceFolder = this.workspaceService.getWorkspaceFolder(resource); - if ( - !workspaceFolder && - Array.isArray(this.workspaceService.workspaceFolders) && - this.workspaceService.workspaceFolders.length > 0 - ) { - [workspaceFolder] = this.workspaceService.workspaceFolders; + if (!workspaceFolder && Array.isArray(this.workspaceService.workspaceFolders) && this.workspaceService.workspaceFolders.length > 0) { + workspaceFolder = this.workspaceService.workspaceFolders[0]; } console.log('Use workspace folder', workspaceFolder?.uri.fsPath); const settings = this.configurationService.getSettings(resource); @@ -94,7 +86,12 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath); return; } - const env: EnvironmentVariables = { HELLO: workspaceFolder?.name }; + const env = await this.environmentActivationService.getActivatedEnvironmentVariables( + resource, + undefined, + undefined, + shell, + ); if (!env) { const shellType = identifyShellFromShellPath(shell); const defaultShell = defaultShells[this.platform.osType]; @@ -116,10 +113,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ if (prevValue !== value) { if (value !== undefined) { traceVerbose(`Setting environment variable ${key} in collection to ${value}`); - this.context.environmentVariableCollection.replace(key, value, { workspaceFolder }); + this.context.environmentVariableCollection.replace(key, value, {workspaceFolder}); } else { traceVerbose(`Clearing environment variable ${key} from collection`); - this.context.environmentVariableCollection.delete(key, { workspaceFolder }); + this.context.environmentVariableCollection.delete(key, {workspaceFolder}); } } }); @@ -127,7 +124,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ // If the previous env var is not in the current env, clear it from collection. if (!(key in env)) { traceVerbose(`Clearing environment variable ${key} from collection`); - this.context.environmentVariableCollection.delete(key, { workspaceFolder }); + this.context.environmentVariableCollection.delete(key, {workspaceFolder}); } }); } From b9261a79732b59718204444fac69a30c106d78a3 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 14 Apr 2023 16:48:59 -0700 Subject: [PATCH 09/19] Support description --- .../terminalEnvVarCollectionService.ts | 25 +++++++++++++------ ...scode.proposed.envCollectionWorkspace.d.ts | 9 +++++-- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index f1adbdaa4a4f..4f3275fd6b30 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -14,6 +14,7 @@ import { Resource, IDisposableRegistry, IConfigurationService, + IPathUtils, } from '../../common/types'; import { Deferred, createDeferred } from '../../common/utils/async'; import { Interpreters } from '../../common/utils/localize'; @@ -44,8 +45,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ @inject(IEnvironmentActivationService) private environmentActivationService: IEnvironmentActivationService, @inject(IWorkspaceService) private workspaceService: IWorkspaceService, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, + @inject(IPathUtils) private readonly pathUtils: IPathUtils, ) { - this.context.environmentVariableCollection.clear(); this.interpreterService.onDidChangeInterpreter( async (resource) => { this.showProgress(); @@ -65,7 +66,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ }, this, this.disposables, - );} + ); + } public async activate(resource: Resource): Promise { if (!inTerminalEnvVarExperiment(this.experimentService)) { @@ -77,10 +79,13 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise { let workspaceFolder = this.workspaceService.getWorkspaceFolder(resource); - if (!workspaceFolder && Array.isArray(this.workspaceService.workspaceFolders) && this.workspaceService.workspaceFolders.length > 0) { - workspaceFolder = this.workspaceService.workspaceFolders[0]; + if ( + !workspaceFolder && + Array.isArray(this.workspaceService.workspaceFolders) && + this.workspaceService.workspaceFolders.length > 0 + ) { + [workspaceFolder] = this.workspaceService.workspaceFolders; } - console.log('Use workspace folder', workspaceFolder?.uri.fsPath); const settings = this.configurationService.getSettings(resource); if (!settings.terminal.activateEnvironment) { traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath); @@ -113,10 +118,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ if (prevValue !== value) { if (value !== undefined) { traceVerbose(`Setting environment variable ${key} in collection to ${value}`); - this.context.environmentVariableCollection.replace(key, value, {workspaceFolder}); + this.context.environmentVariableCollection.replace(key, value, { workspaceFolder }); } else { traceVerbose(`Clearing environment variable ${key} from collection`); - this.context.environmentVariableCollection.delete(key, {workspaceFolder}); + this.context.environmentVariableCollection.delete(key, { workspaceFolder }); } } }); @@ -124,9 +129,13 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ // If the previous env var is not in the current env, clear it from collection. if (!(key in env)) { traceVerbose(`Clearing environment variable ${key} from collection`); - this.context.environmentVariableCollection.delete(key, {workspaceFolder}); + this.context.environmentVariableCollection.delete(key, { workspaceFolder }); } }); + const displayPath = this.pathUtils.getDisplayName(settings.pythonPath, workspaceFolder?.uri.fsPath); + this.context.environmentVariableCollection.setDescription(`Activated environment for ${displayPath}`, { + workspaceFolder, + }); } @traceDecoratorVerbose('Display activating terminals') diff --git a/types/vscode.proposed.envCollectionWorkspace.d.ts b/types/vscode.proposed.envCollectionWorkspace.d.ts index cdabc7764e54..0937a9cdbfa5 100644 --- a/types/vscode.proposed.envCollectionWorkspace.d.ts +++ b/types/vscode.proposed.envCollectionWorkspace.d.ts @@ -14,6 +14,12 @@ declare module 'vscode' { } export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> { + /** + * Sets a description for the environment variable collection, this will be used to describe the changes in the UI. + * @param description A description for the environment variable collection. + * @param scope Specific scope to which this description applies to. + */ + setDescription(description: string | MarkdownString | undefined, scope?: EnvironmentVariableScope): void; replace(variable: string, value: string, scope?: EnvironmentVariableScope): void; append(variable: string, value: string, scope?: EnvironmentVariableScope): void; prepend(variable: string, value: string, scope?: EnvironmentVariableScope): void; @@ -23,8 +29,7 @@ declare module 'vscode' { export type EnvironmentVariableScope = { /** - * The workspace folder to which this environment variable collection applies to. - * If unspecified, the collection applies to all workspace folders. + * The workspace folder to which this collection applies to. If unspecified, collection applies to all workspace folders. */ workspaceFolder?: WorkspaceFolder; }; From 9199dccf4aa4ceea2332a446b9bda96c4ca3fa60 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 15 Apr 2023 16:25:27 -0700 Subject: [PATCH 10/19] Fix --- .../interpreter/activation/terminalEnvVarCollectionService.ts | 2 +- types/vscode.proposed.envCollectionWorkspace.d.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 4f3275fd6b30..e93429f54524 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -106,7 +106,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ await this._applyCollection(resource, defaultShell?.shell); return; } - this.context.environmentVariableCollection.clear(); + this.context.environmentVariableCollection.clear({ workspaceFolder }); this.previousEnvVars = _normCaseKeys(process.env); return; } diff --git a/types/vscode.proposed.envCollectionWorkspace.d.ts b/types/vscode.proposed.envCollectionWorkspace.d.ts index 0937a9cdbfa5..b1176a9d46c2 100644 --- a/types/vscode.proposed.envCollectionWorkspace.d.ts +++ b/types/vscode.proposed.envCollectionWorkspace.d.ts @@ -25,6 +25,8 @@ declare module 'vscode' { prepend(variable: string, value: string, scope?: EnvironmentVariableScope): void; get(variable: string, scope?: EnvironmentVariableScope): EnvironmentVariableMutator | undefined; delete(variable: string, scope?: EnvironmentVariableScope): void; + clear(scope?: EnvironmentVariableScope): void; + } export type EnvironmentVariableScope = { From 633b9b40496506bfcd8d6e3810d75ecd2a5b8ca4 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 15 Apr 2023 16:37:36 -0700 Subject: [PATCH 11/19] Add backticks --- .../interpreter/activation/terminalEnvVarCollectionService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index e93429f54524..efb0b59ffde8 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -133,7 +133,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } }); const displayPath = this.pathUtils.getDisplayName(settings.pythonPath, workspaceFolder?.uri.fsPath); - this.context.environmentVariableCollection.setDescription(`Activated environment for ${displayPath}`, { + this.context.environmentVariableCollection.setDescription(`Activated environment for \`${displayPath}\``, { workspaceFolder, }); } From 1316e6b12ac43a7e4724d7685385c58becc18a78 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 21 Apr 2023 00:59:55 -0700 Subject: [PATCH 12/19] Use markdown string --- .../activation/terminalEnvVarCollectionService.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index efb0b59ffde8..7157614b1d3a 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { inject, injectable } from 'inversify'; -import { ProgressOptions, ProgressLocation } from 'vscode'; +import { ProgressOptions, ProgressLocation, MarkdownString } from 'vscode'; import { IExtensionActivationService } from '../../activation/types'; import { IApplicationShell, IApplicationEnvironment, IWorkspaceService } from '../../common/application/types'; import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers'; @@ -133,7 +133,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } }); const displayPath = this.pathUtils.getDisplayName(settings.pythonPath, workspaceFolder?.uri.fsPath); - this.context.environmentVariableCollection.setDescription(`Activated environment for \`${displayPath}\``, { + const description = new MarkdownString(`Activated environment for \`${displayPath}\``); + this.context.environmentVariableCollection.setDescription(description, { workspaceFolder, }); } From 91ac3bfe4760a8762c90b862a4a8b553243054ed Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 21 Apr 2023 12:46:46 -0700 Subject: [PATCH 13/19] Fix unit tests --- .../terminalEnvVarCollectionService.ts | 47 +- ...rminalEnvVarCollectionService.unit.test.ts | 504 +++++++++--------- 2 files changed, 282 insertions(+), 269 deletions(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 7157614b1d3a..defb060ac2f3 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -32,6 +32,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ private deferred: Deferred | undefined; + private registeredOnce = false; + private previousEnvVars = _normCaseKeys(process.env); constructor( @@ -46,34 +48,35 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ @inject(IWorkspaceService) private workspaceService: IWorkspaceService, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(IPathUtils) private readonly pathUtils: IPathUtils, - ) { - this.interpreterService.onDidChangeInterpreter( - async (resource) => { - this.showProgress(); - await this._applyCollection(resource); - this.hideProgress(); - }, - this, - this.disposables, - ); - this.applicationEnvironment.onDidChangeShell( - async (shell: string) => { - this.showProgress(); - // Pass in the shell where known instead of relying on the application environment, because of bug - // on VSCode: https://github.com/microsoft/vscode/issues/160694 - await this._applyCollection(undefined, shell); - this.hideProgress(); - }, - this, - this.disposables, - ); - } + ) {} public async activate(resource: Resource): Promise { if (!inTerminalEnvVarExperiment(this.experimentService)) { this.context.environmentVariableCollection.clear(); return; } + if (!this.registeredOnce) { + this.interpreterService.onDidChangeInterpreter( + async (r) => { + this.showProgress(); + await this._applyCollection(r); + this.hideProgress(); + }, + this, + this.disposables, + ); + this.applicationEnvironment.onDidChangeShell( + async (shell: string) => { + this.showProgress(); + // Pass in the shell where known instead of relying on the application environment, because of bug + // on VSCode: https://github.com/microsoft/vscode/issues/160694 + await this._applyCollection(undefined, shell); + this.hideProgress(); + }, + this, + this.disposables, + ); + } this._applyCollection(resource).ignoreErrors(); } diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 5baaec41ab74..30d2b36876ee 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -1,247 +1,257 @@ -// // Copyright (c) Microsoft Corporation. All rights reserved. -// // Licensed under the MIT License. - -// 'use strict'; - -// import * as sinon from 'sinon'; -// import { assert, expect } from 'chai'; -// import { cloneDeep } from 'lodash'; -// import { mock, instance, when, anything, verify, reset } from 'ts-mockito'; -// import { EnvironmentVariableCollection, ProgressLocation, Uri } from 'vscode'; -// import { IApplicationShell, IApplicationEnvironment } from '../../../client/common/application/types'; -// import { TerminalEnvVarActivation } from '../../../client/common/experiments/groups'; -// import { IPlatformService } from '../../../client/common/platform/types'; -// import { -// IExtensionContext, -// IExperimentService, -// Resource, -// IConfigurationService, -// IPythonSettings, -// } from '../../../client/common/types'; -// import { Interpreters } from '../../../client/common/utils/localize'; -// import { getOSType } from '../../../client/common/utils/platform'; -// import { defaultShells } from '../../../client/interpreter/activation/service'; -// import { -// TerminalEnvVarCollectionService, -// _normCaseKeys, -// } from '../../../client/interpreter/activation/terminalEnvVarCollectionService'; -// import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; -// import { IInterpreterService } from '../../../client/interpreter/contracts'; - -// suite('Terminal Environment Variable Collection Service', () => { -// let platform: IPlatformService; -// let interpreterService: IInterpreterService; -// let context: IExtensionContext; -// let shell: IApplicationShell; -// let experimentService: IExperimentService; -// let collection: EnvironmentVariableCollection; -// let applicationEnvironment: IApplicationEnvironment; -// let environmentActivationService: IEnvironmentActivationService; -// let terminalEnvVarCollectionService: TerminalEnvVarCollectionService; -// const progressOptions = { -// location: ProgressLocation.Window, -// title: Interpreters.activatingTerminals, -// }; -// const customShell = 'powershell'; -// const defaultShell = defaultShells[getOSType()]; - -// setup(() => { -// platform = mock(); -// when(platform.osType).thenReturn(getOSType()); -// interpreterService = mock(); -// context = mock(); -// shell = mock(); -// collection = mock(); -// when(context.environmentVariableCollection).thenReturn(instance(collection)); -// experimentService = mock(); -// when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true); -// applicationEnvironment = mock(); -// when(applicationEnvironment.shell).thenReturn(customShell); -// when(shell.withProgress(anything(), anything())) -// .thenCall((options, _) => { -// expect(options).to.deep.equal(progressOptions); -// }) -// .thenResolve(); -// environmentActivationService = mock(); -// const configService = mock(); -// when(configService.getSettings(anything())).thenReturn(({ -// terminal: { activateEnvironment: true }, -// } as unknown) as IPythonSettings); -// terminalEnvVarCollectionService = new TerminalEnvVarCollectionService( -// instance(platform), -// instance(interpreterService), -// instance(context), -// instance(shell), -// instance(experimentService), -// instance(applicationEnvironment), -// [], -// instance(environmentActivationService), -// instance(configService), -// ); -// }); - -// teardown(() => { -// sinon.restore(); -// }); - -// test('Apply activated variables to the collection on activation', async () => { -// const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); -// applyCollectionStub.resolves(); -// when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); -// when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); -// await terminalEnvVarCollectionService.activate(); -// assert(applyCollectionStub.calledOnce, 'Collection not applied on activation'); -// }); - -// test('When not in experiment, do not apply activated variables to the collection and clear it instead', async () => { -// reset(experimentService); -// when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(false); -// const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); -// applyCollectionStub.resolves(); -// when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); -// when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); - -// await terminalEnvVarCollectionService.activate(); - -// verify(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).never(); -// verify(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).never(); -// assert(applyCollectionStub.notCalled, 'Collection should not be applied on activation'); - -// verify(collection.clear()).once(); -// }); - -// test('When interpreter changes, apply new activated variables to the collection', async () => { -// const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); -// applyCollectionStub.resolves(); -// const resource = Uri.file('x'); -// let callback: (resource: Resource) => Promise; -// when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenCall((cb) => { -// callback = cb; -// }); -// when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); -// await terminalEnvVarCollectionService.activate(); - -// await callback!(resource); -// assert(applyCollectionStub.calledWithExactly(resource)); -// }); - -// test('When selected shell changes, apply new activated variables to the collection', async () => { -// const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); -// applyCollectionStub.resolves(); -// let callback: (shell: string) => Promise; -// when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenCall((cb) => { -// callback = cb; -// }); -// when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); -// await terminalEnvVarCollectionService.activate(); - -// await callback!(customShell); -// assert(applyCollectionStub.calledWithExactly(undefined, customShell)); -// }); - -// test('If activated variables are returned for custom shell, apply it correctly to the collection', async () => { -// const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; -// delete envVars.PATH; -// when( -// environmentActivationService.getActivatedEnvironmentVariables( -// anything(), -// undefined, -// undefined, -// customShell, -// ), -// ).thenResolve(envVars); - -// when(collection.replace(anything(), anything())).thenResolve(); -// when(collection.delete(anything())).thenResolve(); - -// await terminalEnvVarCollectionService._applyCollection(undefined, customShell); - -// verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda')).once(); -// verify(collection.delete('PATH')).once(); -// }); - -// test('Only relative changes to previously applied variables are applied to the collection', async () => { -// const envVars: NodeJS.ProcessEnv = { -// RANDOM_VAR: 'random', -// CONDA_PREFIX: 'prefix/to/conda', -// ..._normCaseKeys(process.env), -// }; -// when( -// environmentActivationService.getActivatedEnvironmentVariables( -// anything(), -// undefined, -// undefined, -// customShell, -// ), -// ).thenResolve(envVars); - -// when(collection.replace(anything(), anything())).thenResolve(); -// when(collection.delete(anything())).thenResolve(); - -// await terminalEnvVarCollectionService._applyCollection(undefined, customShell); - -// const newEnvVars = cloneDeep(envVars); -// delete newEnvVars.CONDA_PREFIX; -// newEnvVars.RANDOM_VAR = undefined; // Deleting the variable from the collection is the same as setting it to undefined. -// reset(environmentActivationService); -// when( -// environmentActivationService.getActivatedEnvironmentVariables( -// anything(), -// undefined, -// undefined, -// customShell, -// ), -// ).thenResolve(newEnvVars); - -// await terminalEnvVarCollectionService._applyCollection(undefined, customShell); - -// verify(collection.delete('CONDA_PREFIX')).once(); -// verify(collection.delete('RANDOM_VAR')).once(); -// }); - -// test('If no activated variables are returned for custom shell, fallback to using default shell', async () => { -// when( -// environmentActivationService.getActivatedEnvironmentVariables( -// anything(), -// undefined, -// undefined, -// customShell, -// ), -// ).thenResolve(undefined); -// const envVars = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; -// when( -// environmentActivationService.getActivatedEnvironmentVariables( -// anything(), -// undefined, -// undefined, -// defaultShell?.shell, -// ), -// ).thenResolve(envVars); - -// when(collection.replace(anything(), anything())).thenResolve(); -// when(collection.delete(anything())).thenResolve(); - -// await terminalEnvVarCollectionService._applyCollection(undefined, customShell); - -// verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda')).once(); -// verify(collection.delete(anything())).never(); -// }); - -// test('If no activated variables are returned for default shell, clear collection', async () => { -// when( -// environmentActivationService.getActivatedEnvironmentVariables( -// anything(), -// undefined, -// undefined, -// defaultShell?.shell, -// ), -// ).thenResolve(undefined); - -// when(collection.replace(anything(), anything())).thenResolve(); -// when(collection.delete(anything())).thenResolve(); - -// await terminalEnvVarCollectionService._applyCollection(undefined, defaultShell?.shell); - -// verify(collection.clear()).once(); -// }); -// }); +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import * as sinon from 'sinon'; +import { assert, expect } from 'chai'; +import { cloneDeep } from 'lodash'; +import { mock, instance, when, anything, verify, reset } from 'ts-mockito'; +import { EnvironmentVariableCollection, ProgressLocation, Uri } from 'vscode'; +import { + IApplicationShell, + IApplicationEnvironment, + IWorkspaceService, +} from '../../../client/common/application/types'; +import { TerminalEnvVarActivation } from '../../../client/common/experiments/groups'; +import { IPlatformService } from '../../../client/common/platform/types'; +import { + IExtensionContext, + IExperimentService, + Resource, + IConfigurationService, + IPythonSettings, +} from '../../../client/common/types'; +import { Interpreters } from '../../../client/common/utils/localize'; +import { OSType, getOSType } from '../../../client/common/utils/platform'; +import { defaultShells } from '../../../client/interpreter/activation/service'; +import { + TerminalEnvVarCollectionService, + _normCaseKeys, +} from '../../../client/interpreter/activation/terminalEnvVarCollectionService'; +import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; +import { IInterpreterService } from '../../../client/interpreter/contracts'; +import { PathUtils } from '../../../client/common/platform/pathUtils'; + +suite('Terminal Environment Variable Collection Service', () => { + let platform: IPlatformService; + let interpreterService: IInterpreterService; + let context: IExtensionContext; + let shell: IApplicationShell; + let experimentService: IExperimentService; + let collection: EnvironmentVariableCollection; + let applicationEnvironment: IApplicationEnvironment; + let environmentActivationService: IEnvironmentActivationService; + let workspaceService: IWorkspaceService; + let terminalEnvVarCollectionService: TerminalEnvVarCollectionService; + const progressOptions = { + location: ProgressLocation.Window, + title: Interpreters.activatingTerminals, + }; + const customShell = 'powershell'; + const defaultShell = defaultShells[getOSType()]; + + setup(() => { + workspaceService = mock(); + platform = mock(); + when(platform.osType).thenReturn(getOSType()); + interpreterService = mock(); + context = mock(); + shell = mock(); + collection = mock(); + when(context.environmentVariableCollection).thenReturn(instance(collection)); + experimentService = mock(); + when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true); + applicationEnvironment = mock(); + when(applicationEnvironment.shell).thenReturn(customShell); + when(shell.withProgress(anything(), anything())) + .thenCall((options, _) => { + expect(options).to.deep.equal(progressOptions); + }) + .thenResolve(); + environmentActivationService = mock(); + const configService = mock(); + when(configService.getSettings(anything())).thenReturn(({ + terminal: { activateEnvironment: true }, + pythonPath: 'display/path', + } as unknown) as IPythonSettings); + terminalEnvVarCollectionService = new TerminalEnvVarCollectionService( + instance(platform), + instance(interpreterService), + instance(context), + instance(shell), + instance(experimentService), + instance(applicationEnvironment), + [], + instance(environmentActivationService), + instance(workspaceService), + instance(configService), + new PathUtils(getOSType() === OSType.Windows), + ); + }); + + teardown(() => { + sinon.restore(); + }); + + test('Apply activated variables to the collection on activation', async () => { + const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); + applyCollectionStub.resolves(); + when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); + when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); + await terminalEnvVarCollectionService.activate(undefined); + assert(applyCollectionStub.calledOnce, 'Collection not applied on activation'); + }); + + test('When not in experiment, do not apply activated variables to the collection and clear it instead', async () => { + reset(experimentService); + when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(false); + const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); + applyCollectionStub.resolves(); + when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); + when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); + + await terminalEnvVarCollectionService.activate(undefined); + + verify(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).never(); + verify(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).never(); + assert(applyCollectionStub.notCalled, 'Collection should not be applied on activation'); + + verify(collection.clear()).once(); + }); + + test('When interpreter changes, apply new activated variables to the collection', async () => { + const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); + applyCollectionStub.resolves(); + const resource = Uri.file('x'); + let callback: (resource: Resource) => Promise; + when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenCall((cb) => { + callback = cb; + }); + when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenReturn(); + await terminalEnvVarCollectionService.activate(undefined); + + await callback!(resource); + assert(applyCollectionStub.calledWithExactly(resource)); + }); + + test('When selected shell changes, apply new activated variables to the collection', async () => { + const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); + applyCollectionStub.resolves(); + let callback: (shell: string) => Promise; + when(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).thenCall((cb) => { + callback = cb; + }); + when(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).thenReturn(); + await terminalEnvVarCollectionService.activate(undefined); + + await callback!(customShell); + assert(applyCollectionStub.calledWithExactly(undefined, customShell)); + }); + + test('If activated variables are returned for custom shell, apply it correctly to the collection', async () => { + const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; + delete envVars.PATH; + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + customShell, + ), + ).thenResolve(envVars); + + when(collection.replace(anything(), anything(), anything())).thenResolve(); + when(collection.delete(anything(), anything())).thenResolve(); + + await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + + verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once(); + verify(collection.delete('PATH', anything())).once(); + }); + + test('Only relative changes to previously applied variables are applied to the collection', async () => { + const envVars: NodeJS.ProcessEnv = { + RANDOM_VAR: 'random', + CONDA_PREFIX: 'prefix/to/conda', + ..._normCaseKeys(process.env), + }; + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + customShell, + ), + ).thenResolve(envVars); + + when(collection.replace(anything(), anything(), anything())).thenResolve(); + when(collection.delete(anything(), anything())).thenResolve(); + + await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + + const newEnvVars = cloneDeep(envVars); + delete newEnvVars.CONDA_PREFIX; + newEnvVars.RANDOM_VAR = undefined; // Deleting the variable from the collection is the same as setting it to undefined. + reset(environmentActivationService); + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + customShell, + ), + ).thenResolve(newEnvVars); + + await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + + verify(collection.delete('CONDA_PREFIX', anything())).once(); + verify(collection.delete('RANDOM_VAR', anything())).once(); + }); + + test('If no activated variables are returned for custom shell, fallback to using default shell', async () => { + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + customShell, + ), + ).thenResolve(undefined); + const envVars = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + defaultShell?.shell, + ), + ).thenResolve(envVars); + + when(collection.replace(anything(), anything(), anything())).thenResolve(); + when(collection.delete(anything(), anything())).thenResolve(); + + await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + + verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once(); + verify(collection.delete(anything(), anything())).never(); + }); + + test('If no activated variables are returned for default shell, clear collection', async () => { + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + defaultShell?.shell, + ), + ).thenResolve(undefined); + + when(collection.replace(anything(), anything(), anything())).thenResolve(); + when(collection.delete(anything(), anything())).thenResolve(); + + await terminalEnvVarCollectionService._applyCollection(undefined, defaultShell?.shell); + + verify(collection.clear(anything())).once(); + }); +}); From 6ac662b36e27531ca69716716473c9e3929d3785 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 21 Apr 2023 13:08:35 -0700 Subject: [PATCH 14/19] Add tests --- src/client/common/utils/localize.ts | 1 + .../terminalEnvVarCollectionService.ts | 2 +- ...rminalEnvVarCollectionService.unit.test.ts | 73 ++++++++++++++++++- 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 4c3cd7a45d0d..633d9426ba74 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -198,6 +198,7 @@ export namespace Interpreters { 'We noticed you\'re using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change "terminal.integrated.inheritEnv" to false in your user settings. [Learn more](https://aka.ms/AA66i8f).', ); export const activatingTerminals = l10n.t('Reactivating terminals...'); + export const activateTerminalDescription = l10n.t('Activated environment for'); export const activatedCondaEnvLaunch = l10n.t( 'We noticed VS Code was launched from an activated conda environment, would you like to select it?', ); diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index defb060ac2f3..a42a6e32fcb0 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -136,7 +136,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } }); const displayPath = this.pathUtils.getDisplayName(settings.pythonPath, workspaceFolder?.uri.fsPath); - const description = new MarkdownString(`Activated environment for \`${displayPath}\``); + const description = new MarkdownString(`${Interpreters.activateTerminalDescription} \`${displayPath}\``); this.context.environmentVariableCollection.setDescription(description, { workspaceFolder, }); diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 30d2b36876ee..e5883903ae74 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -7,7 +7,7 @@ import * as sinon from 'sinon'; import { assert, expect } from 'chai'; import { cloneDeep } from 'lodash'; import { mock, instance, when, anything, verify, reset } from 'ts-mockito'; -import { EnvironmentVariableCollection, ProgressLocation, Uri } from 'vscode'; +import { EnvironmentVariableCollection, ProgressLocation, Uri, WorkspaceFolder } from 'vscode'; import { IApplicationShell, IApplicationEnvironment, @@ -48,11 +48,15 @@ suite('Terminal Environment Variable Collection Service', () => { location: ProgressLocation.Window, title: Interpreters.activatingTerminals, }; + let configService: IConfigurationService; + const displayPath = 'display/path'; const customShell = 'powershell'; const defaultShell = defaultShells[getOSType()]; setup(() => { workspaceService = mock(); + when(workspaceService.getWorkspaceFolder(anything())).thenReturn(undefined); + when(workspaceService.workspaceFolders).thenReturn(undefined); platform = mock(); when(platform.osType).thenReturn(getOSType()); interpreterService = mock(); @@ -70,10 +74,10 @@ suite('Terminal Environment Variable Collection Service', () => { }) .thenResolve(); environmentActivationService = mock(); - const configService = mock(); + configService = mock(); when(configService.getSettings(anything())).thenReturn(({ terminal: { activateEnvironment: true }, - pythonPath: 'display/path', + pythonPath: displayPath, } as unknown) as IPythonSettings); terminalEnvVarCollectionService = new TerminalEnvVarCollectionService( instance(platform), @@ -170,6 +174,67 @@ suite('Terminal Environment Variable Collection Service', () => { verify(collection.delete('PATH', anything())).once(); }); + test('Verify envs are not applied if env activation is disabled', async () => { + const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; + delete envVars.PATH; + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + customShell, + ), + ).thenResolve(envVars); + + when(collection.replace(anything(), anything(), anything())).thenResolve(); + when(collection.delete(anything(), anything())).thenResolve(); + reset(configService); + when(configService.getSettings(anything())).thenReturn(({ + terminal: { activateEnvironment: false }, + pythonPath: displayPath, + } as unknown) as IPythonSettings); + + await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + + verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).never(); + verify(collection.delete('PATH', anything())).never(); + }); + + test('Verify correct scope is used when applying envs and setting description', async () => { + const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; + delete envVars.PATH; + const resource = Uri.file('a'); + const workspaceFolder: WorkspaceFolder = { + uri: Uri.file('workspacePath'), + name: 'workspace1', + index: 0, + }; + when(workspaceService.getWorkspaceFolder(resource)).thenReturn(workspaceFolder); + when( + environmentActivationService.getActivatedEnvironmentVariables(resource, undefined, undefined, customShell), + ).thenResolve(envVars); + + when(collection.replace(anything(), anything(), anything())).thenCall((_e, _v, scope) => { + assert.deepEqual(scope, { workspaceFolder }); + return Promise.resolve(); + }); + when(collection.delete(anything(), anything())).thenCall((_e, scope) => { + assert.deepEqual(scope, { workspaceFolder }); + return Promise.resolve(); + }); + let description = ''; + when(collection.setDescription(anything(), anything())).thenCall((d, scope) => { + assert.deepEqual(scope, { workspaceFolder }); + description = d.value; + }); + + await terminalEnvVarCollectionService._applyCollection(resource, customShell); + + verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once(); + verify(collection.delete('PATH', anything())).once(); + expect(description).to.equal(`${Interpreters.activateTerminalDescription} \`${displayPath}\``); + }); + test('Only relative changes to previously applied variables are applied to the collection', async () => { const envVars: NodeJS.ProcessEnv = { RANDOM_VAR: 'random', @@ -249,9 +314,11 @@ suite('Terminal Environment Variable Collection Service', () => { when(collection.replace(anything(), anything(), anything())).thenResolve(); when(collection.delete(anything(), anything())).thenResolve(); + when(collection.setDescription(anything(), anything())).thenReturn(); await terminalEnvVarCollectionService._applyCollection(undefined, defaultShell?.shell); verify(collection.clear(anything())).once(); + verify(collection.setDescription(anything(), anything())).never(); }); }); From 869b09001150fd10acc690fbfd074947aeb66610 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 21 Apr 2023 13:10:21 -0700 Subject: [PATCH 15/19] Update vscode engine --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 09f4264241f0..fc29c2b97e0e 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.77.0-20230309" + "vscode": "^1.78.0-20230422" }, "keywords": [ "python", From 59c678d1ffacf39817dde8dbadb7eb5071428ecc Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 21 Apr 2023 13:11:53 -0700 Subject: [PATCH 16/19] Wrap try catch around proposed APIs --- .../interpreter/activation/terminalEnvVarCollectionService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index a42a6e32fcb0..1c5dabe27d37 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -59,7 +59,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ this.interpreterService.onDidChangeInterpreter( async (r) => { this.showProgress(); - await this._applyCollection(r); + await this._applyCollection(r).ignoreErrors(); this.hideProgress(); }, this, @@ -70,7 +70,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ this.showProgress(); // Pass in the shell where known instead of relying on the application environment, because of bug // on VSCode: https://github.com/microsoft/vscode/issues/160694 - await this._applyCollection(undefined, shell); + await this._applyCollection(undefined, shell).ignoreErrors(); this.hideProgress(); }, this, From 2cde3d55cb0195a830e6a6d480ff5a2eee51f278 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 21 Apr 2023 13:13:37 -0700 Subject: [PATCH 17/19] Revert "build vsix" This reverts commit 9ecdd05fbd455d7e880d06c6b308ccff7ba42086. --- .github/workflows/pr-check.yml | 456 +++++++++++++++++++++++++++++++++ 1 file changed, 456 insertions(+) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 6c489b82f265..ade1dcd61234 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -33,3 +33,459 @@ jobs: node_version: ${{ env.NODE_VERSION}} vsix_name: ${{ env.VSIX_NAME }} artifact_name: ${{ env.ARTIFACT_NAME_VSIX }} + + lint: + name: Lint + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v3 + + - name: Lint + uses: ./.github/actions/lint + with: + node_version: ${{ env.NODE_VERSION }} + + check-types: + name: Check Python types + runs-on: ubuntu-latest + steps: + - name: Use Python ${{ env.PYTHON_VERSION }} + uses: actions/setup-python@v4 + with: + python-version: ${{ env.PYTHON_VERSION }} + + - name: Checkout + uses: actions/checkout@v3 + + - name: Install base Python requirements + uses: brettcannon/pip-secure-install@v1 + with: + options: '-t ./pythonFiles/lib/python --no-cache-dir --implementation py' + + - name: Install Jedi requirements + uses: brettcannon/pip-secure-install@v1 + with: + requirements-file: './pythonFiles/jedilsp_requirements/requirements.txt' + options: '-t ./pythonFiles/lib/jedilsp --no-cache-dir --implementation py' + + - name: Install other Python requirements + run: | + python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --no-cache-dir --implementation py --no-deps --upgrade --pre debugpy + python -m pip install --upgrade -r build/test-requirements.txt + + - name: Run Pyright + uses: jakebailey/pyright-action@v1 + with: + working-directory: 'pythonFiles' + + ### Non-smoke tests + tests: + name: Tests + # The value of runs-on is the OS of the current job (specified in the strategy matrix below) instead of being hardcoded. + runs-on: ${{ matrix.os }} + defaults: + run: + working-directory: ${{ env.special-working-directory }} + strategy: + fail-fast: false + matrix: + # We're not running CI on macOS for now because it's one less matrix entry to lower the number of runners used, + # macOS runners are expensive, and we assume that Ubuntu is enough to cover the Unix case. + os: [ubuntu-latest, windows-latest] + # Run the tests on the oldest and most recent versions of Python. + python: ['3.x'] + test-suite: [ts-unit, python-unit, venv, single-workspace, debugger, functional] + + steps: + - name: Checkout + uses: actions/checkout@v3 + with: + path: ${{ env.special-working-directory-relative }} + + - name: Install Node + uses: actions/setup-node@v3 + with: + node-version: ${{ env.NODE_VERSION }} + cache: 'npm' + cache-dependency-path: ${{ env.special-working-directory-relative }}/package-lock.json + + - name: Install dependencies (npm ci) + run: npm ci + + - name: Compile + run: npx gulp prePublishNonBundle + + - name: Use Python ${{ matrix.python }} + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python }} + + - name: Install debugpy + run: | + # We need to have debugpy so that tests relying on it keep passing, but we don't need install_debugpy's logic in the test phase. + python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --no-cache-dir --implementation py --no-deps --upgrade --pre debugpy + + - name: Install base Python requirements + uses: brettcannon/pip-secure-install@v1 + with: + requirements-file: '"${{ env.special-working-directory-relative }}/requirements.txt"' + options: '-t "${{ env.special-working-directory-relative }}/pythonFiles/lib/python" --no-cache-dir --implementation py' + if: startsWith(matrix.python, 3.) + + - name: Install Jedi requirements + uses: brettcannon/pip-secure-install@v1 + with: + requirements-file: '"${{ env.special-working-directory-relative }}/pythonFiles/jedilsp_requirements/requirements.txt"' + options: '-t "${{ env.special-working-directory-relative }}/pythonFiles/lib/jedilsp" --no-cache-dir --implementation py' + if: startsWith(matrix.python, 3.) + + - name: Install test requirements + run: python -m pip install --upgrade -r build/test-requirements.txt + + - name: Install debugpy wheels (Python ${{ matrix.python }}) + run: | + python -m pip install wheel + python -m pip --disable-pip-version-check install -r build/debugger-install-requirements.txt + python ./pythonFiles/install_debugpy.py + shell: bash + if: matrix.test-suite == 'debugger' + + - name: Install functional test requirements + run: python -m pip install --upgrade -r ./build/functional-test-requirements.txt + if: matrix.test-suite == 'functional' + + - name: Prepare pipenv for venv tests + env: + TEST_FILES_SUFFIX: testvirtualenvs + PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' + shell: pwsh + if: matrix.test-suite == 'venv' + run: | + python -m pip install pipenv + python -m pipenv run python ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} pipenvPath + + - name: Prepare poetry for venv tests + env: + TEST_FILES_SUFFIX: testvirtualenvs + shell: pwsh + if: matrix.test-suite == 'venv' + run: | + python -m pip install poetry + Move-Item -Path ".\build\ci\pyproject.toml" -Destination . + poetry env use python + + - name: Prepare virtualenv for venv tests + env: + TEST_FILES_SUFFIX: testvirtualenvs + PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' + shell: pwsh + if: matrix.test-suite == 'venv' + run: | + python -m pip install virtualenv + python -m virtualenv .virtualenv/ + if ('${{ matrix.os }}' -match 'windows-latest') { + & ".virtualenv/Scripts/python.exe" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} virtualEnvPath + } else { + & ".virtualenv/bin/python" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} virtualEnvPath + } + + - name: Prepare venv for venv tests + env: + TEST_FILES_SUFFIX: testvirtualenvs + PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' + shell: pwsh + if: matrix.test-suite == 'venv' && startsWith(matrix.python, 3.) + run: | + python -m venv .venv + if ('${{ matrix.os }}' -match 'windows-latest') { + & ".venv/Scripts/python.exe" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} venvPath + } else { + & ".venv/bin/python" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} venvPath + } + + - name: Prepare conda for venv tests + env: + TEST_FILES_SUFFIX: testvirtualenvs + PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' + shell: pwsh + if: matrix.test-suite == 'venv' + run: | + # 1. For `terminalActivation.testvirtualenvs.test.ts` + if ('${{ matrix.os }}' -match 'windows-latest') { + $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath python.exe + $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath Scripts | Join-Path -ChildPath conda + } else{ + $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath bin | Join-Path -ChildPath python + $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath bin | Join-Path -ChildPath conda + } + & $condaPythonPath ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} condaExecPath $condaExecPath + & $condaPythonPath ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} condaPath + & $condaExecPath init --all + + - name: Set CI_PYTHON_PATH and CI_DISABLE_AUTO_SELECTION + run: | + echo "CI_PYTHON_PATH=python" >> $GITHUB_ENV + echo "CI_DISABLE_AUTO_SELECTION=1" >> $GITHUB_ENV + shell: bash + if: matrix.test-suite != 'ts-unit' + + # Run TypeScript unit tests only for Python 3.X. + - name: Run TypeScript unit tests + run: npm run test:unittests + if: matrix.test-suite == 'ts-unit' && startsWith(matrix.python, 3.) + + # Run the Python tests in our codebase. + - name: Run Python unit tests + run: | + python pythonFiles/tests/run_all.py + if: matrix.test-suite == 'python-unit' + + # The virtual environment based tests use the `testSingleWorkspace` set of tests + # with the environment variable `TEST_FILES_SUFFIX` set to `testvirtualenvs`, + # which is set in the "Prepare environment for venv tests" step. + # We also use a third-party GitHub Action to install xvfb on Linux, + # run tests and then clean up the process once the tests ran. + # See https://github.com/GabrielBB/xvfb-action + - name: Run venv tests + env: + TEST_FILES_SUFFIX: testvirtualenvs + CI_PYTHON_VERSION: ${{ matrix.python }} + uses: GabrielBB/xvfb-action@v1.6 + with: + run: npm run testSingleWorkspace + working-directory: ${{ env.special-working-directory }} + if: matrix.test-suite == 'venv' && matrix.os == 'ubuntu-latest' + + - name: Run single-workspace tests + env: + CI_PYTHON_VERSION: ${{ matrix.python }} + uses: GabrielBB/xvfb-action@v1.6 + with: + run: npm run testSingleWorkspace + working-directory: ${{ env.special-working-directory }} + if: matrix.test-suite == 'single-workspace' + + - name: Run debugger tests + env: + CI_PYTHON_VERSION: ${{ matrix.python }} + uses: GabrielBB/xvfb-action@v1.6 + with: + run: npm run testDebugger + working-directory: ${{ env.special-working-directory }} + if: matrix.test-suite == 'debugger' + + # Run TypeScript functional tests + - name: Run TypeScript functional tests + run: npm run test:functional + if: matrix.test-suite == 'functional' + + smoke-tests: + name: Smoke tests + # The value of runs-on is the OS of the current job (specified in the strategy matrix below) instead of being hardcoded. + runs-on: ${{ matrix.os }} + needs: [build-vsix] + strategy: + fail-fast: false + matrix: + # We're not running CI on macOS for now because it's one less matrix entry to lower the number of runners used, + # macOS runners are expensive, and we assume that Ubuntu is enough to cover the UNIX case. + os: [ubuntu-latest, windows-latest] + steps: + # Need the source to have the tests available. + - name: Checkout + uses: actions/checkout@v3 + + - name: Smoke tests + uses: ./.github/actions/smoke-tests + with: + node_version: ${{ env.NODE_VERSION }} + artifact_name: ${{ env.ARTIFACT_NAME_VSIX }} + + ### Coverage run + coverage: + name: Coverage + # The value of runs-on is the OS of the current job (specified in the strategy matrix below) instead of being hardcoded. + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + # Only run coverage on linux for PRs + os: [ubuntu-latest] + + steps: + - name: Checkout + uses: actions/checkout@v3 + + - name: Install Node + uses: actions/setup-node@v3 + with: + node-version: ${{ env.NODE_VERSION }} + cache: 'npm' + + - name: Install dependencies (npm ci) + run: npm ci + + - name: Compile + run: npx gulp prePublishNonBundle + + - name: Use Python ${{ env.PYTHON_VERSION }} + uses: actions/setup-python@v4 + with: + python-version: ${{ env.PYTHON_VERSION }} + cache: 'pip' + cache-dependency-path: | + requirements.txt + pythonFiles/jedilsp_requirements/requirements.txt + build/test-requirements.txt + build/functional-test-requirements.txt + + - name: Install base Python requirements + uses: brettcannon/pip-secure-install@v1 + with: + options: '-t ./pythonFiles/lib/python --implementation py' + + - name: Install Jedi requirements + uses: brettcannon/pip-secure-install@v1 + with: + requirements-file: './pythonFiles/jedilsp_requirements/requirements.txt' + options: '-t ./pythonFiles/lib/jedilsp --implementation py' + + - name: Install debugpy + run: | + # We need to have debugpy so that tests relying on it keep passing, but we don't need install_debugpy's logic in the test phase. + python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --implementation py --no-deps --upgrade --pre debugpy + + - name: Install test requirements + run: python -m pip install --upgrade -r build/test-requirements.txt + + - name: Install functional test requirements + run: python -m pip install --upgrade -r ./build/functional-test-requirements.txt + + - name: Prepare pipenv for venv tests + env: + TEST_FILES_SUFFIX: testvirtualenvs + PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' + shell: pwsh + run: | + python -m pip install pipenv + python -m pipenv run python ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} pipenvPath + + - name: Prepare poetry for venv tests + env: + TEST_FILES_SUFFIX: testvirtualenvs + shell: pwsh + run: | + python -m pip install poetry + Move-Item -Path ".\build\ci\pyproject.toml" -Destination . + poetry env use python + + - name: Prepare virtualenv for venv tests + env: + TEST_FILES_SUFFIX: testvirtualenvs + PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' + shell: pwsh + run: | + python -m pip install virtualenv + python -m virtualenv .virtualenv/ + if ('${{ matrix.os }}' -match 'windows-latest') { + & ".virtualenv/Scripts/python.exe" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} virtualEnvPath + } else { + & ".virtualenv/bin/python" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} virtualEnvPath + } + + - name: Prepare venv for venv tests + env: + TEST_FILES_SUFFIX: testvirtualenvs + PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' + shell: pwsh + run: | + python -m venv .venv + if ('${{ matrix.os }}' -match 'windows-latest') { + & ".venv/Scripts/python.exe" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} venvPath + } else { + & ".venv/bin/python" ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} venvPath + } + + - name: Prepare conda for venv tests + env: + TEST_FILES_SUFFIX: testvirtualenvs + PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' + shell: pwsh + run: | + # 1. For `terminalActivation.testvirtualenvs.test.ts` + if ('${{ matrix.os }}' -match 'windows-latest') { + $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath python.exe + $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath Scripts | Join-Path -ChildPath conda + } else{ + $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath bin | Join-Path -ChildPath python + $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath bin | Join-Path -ChildPath conda + } + & $condaPythonPath ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} condaExecPath $condaExecPath + & $condaPythonPath ./build/ci/addEnvPath.py ${{ env.PYTHON_VIRTUAL_ENVS_LOCATION }} condaPath + & $condaExecPath init --all + + - name: Run TypeScript unit tests + run: npm run test:unittests:cover + + - name: Run Python unit tests + run: | + python pythonFiles/tests/run_all.py + + # The virtual environment based tests use the `testSingleWorkspace` set of tests + # with the environment variable `TEST_FILES_SUFFIX` set to `testvirtualenvs`, + # which is set in the "Prepare environment for venv tests" step. + # We also use a third-party GitHub Action to install xvfb on Linux, + # run tests and then clean up the process once the tests ran. + # See https://github.com/GabrielBB/xvfb-action + - name: Run venv tests + env: + TEST_FILES_SUFFIX: testvirtualenvs + CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} + CI_DISABLE_AUTO_SELECTION: 1 + uses: GabrielBB/xvfb-action@v1.6 + with: + run: npm run testSingleWorkspace:cover + + - name: Run single-workspace tests + env: + CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} + CI_DISABLE_AUTO_SELECTION: 1 + uses: GabrielBB/xvfb-action@v1.6 + with: + run: npm run testSingleWorkspace:cover + + # Enable these tests when coverage is setup for multiroot workspace tests + # - name: Run multi-workspace tests + # env: + # CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} + # CI_DISABLE_AUTO_SELECTION: 1 + # uses: GabrielBB/xvfb-action@v1.6 + # with: + # run: npm run testMultiWorkspace:cover + + # Enable these tests when coverage is setup for debugger tests + # - name: Run debugger tests + # env: + # CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} + # CI_DISABLE_AUTO_SELECTION: 1 + # uses: GabrielBB/xvfb-action@v1.6 + # with: + # run: npm run testDebugger:cover + + # Run TypeScript functional tests + - name: Run TypeScript functional tests + env: + CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }} + CI_DISABLE_AUTO_SELECTION: 1 + run: npm run test:functional:cover + + - name: Generate coverage reports + run: npm run test:cover:report + + - name: Upload HTML report + uses: actions/upload-artifact@v3 + with: + name: ${{ runner.os }}-coverage-report-html + path: ./coverage + retention-days: 1 From 7be4f9f69c4bdec78fa304987f0b665c64089ba1 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 21 Apr 2023 13:37:38 -0700 Subject: [PATCH 18/19] Use latest insiders --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index fc29c2b97e0e..6ac8ebcfa01c 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.78.0-20230422" + "vscode": "^1.78.0-20230421" }, "keywords": [ "python", From 87409d67d075144135b34e82e7edb0e9ef3d1787 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 21 Apr 2023 15:45:17 -0700 Subject: [PATCH 19/19] Oops --- .../interpreter/activation/terminalEnvVarCollectionService.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 1c5dabe27d37..e492414ecfd1 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -76,6 +76,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ this, this.disposables, ); + this.registeredOnce = true; } this._applyCollection(resource).ignoreErrors(); }