Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions .github/workflows/pr-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,6 @@ jobs:
- name: Compile Tests
run: npm run compile-tests

- name: Configure Test Settings
run: |
mkdir -p .vscode-test/user-data/User
echo '{"python.useEnvironmentsExtension": true}' > .vscode-test/user-data/User/settings.json
shell: bash

- name: Run Smoke Tests (Linux)
if: runner.os == 'Linux'
uses: GabrielBB/xvfb-action@v1
Expand Down Expand Up @@ -244,12 +238,6 @@ jobs:
- name: Compile Tests
run: npm run compile-tests

- name: Configure Test Settings
run: |
mkdir -p .vscode-test/user-data/User
echo '{"python.useEnvironmentsExtension": true}' > .vscode-test/user-data/User/settings.json
shell: bash

- name: Run E2E Tests (Linux)
if: runner.os == 'Linux'
uses: GabrielBB/xvfb-action@v1
Expand Down Expand Up @@ -337,12 +325,6 @@ jobs:
- name: Compile Tests
run: npm run compile-tests

- name: Configure Test Settings
run: |
mkdir -p .vscode-test/user-data/User
echo '{"python.useEnvironmentsExtension": true}' > .vscode-test/user-data/User/settings.json
shell: bash

- name: Run Integration Tests (Linux)
if: runner.os == 'Linux'
uses: GabrielBB/xvfb-action@v1
Expand Down Expand Up @@ -387,12 +369,6 @@ jobs:
- name: Compile Tests
run: npm run compile-tests

- name: Configure Test Settings
run: |
mkdir -p .vscode-test/user-data/User
echo '{"python.useEnvironmentsExtension": true}' > .vscode-test/user-data/User/settings.json
shell: bash

- name: Run Integration Tests Multi-Root (Linux)
if: runner.os == 'Linux'
uses: GabrielBB/xvfb-action@v1
Expand Down
18 changes: 0 additions & 18 deletions .github/workflows/push-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,6 @@ jobs:
- name: Compile Tests
run: npm run compile-tests

- name: Configure Test Settings
run: |
mkdir -p .vscode-test/user-data/User
echo '{"python.useEnvironmentsExtension": true}' > .vscode-test/user-data/User/settings.json
shell: bash

- name: Run Smoke Tests (Linux)
if: runner.os == 'Linux'
uses: GabrielBB/xvfb-action@v1
Expand Down Expand Up @@ -245,12 +239,6 @@ jobs:
- name: Compile Tests
run: npm run compile-tests

- name: Configure Test Settings
run: |
mkdir -p .vscode-test/user-data/User
echo '{"python.useEnvironmentsExtension": true}' > .vscode-test/user-data/User/settings.json
shell: bash

- name: Run E2E Tests (Linux)
if: runner.os == 'Linux'
uses: GabrielBB/xvfb-action@v1
Expand Down Expand Up @@ -338,12 +326,6 @@ jobs:
- name: Compile Tests
run: npm run compile-tests

- name: Configure Test Settings
run: |
mkdir -p .vscode-test/user-data/User
echo '{"python.useEnvironmentsExtension": true}' > .vscode-test/user-data/User/settings.json
shell: bash

- name: Run Integration Tests (Linux)
if: runner.os == 'Linux'
uses: GabrielBB/xvfb-action@v1
Expand Down
18 changes: 16 additions & 2 deletions .vscode-test.mjs
Original file line number Diff line number Diff line change
@@ -1,8 +1,22 @@
import { defineConfig } from '@vscode/test-cli';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';

// Explicit user data directory - ensures VS Code reads our settings.json
const userDataDir = path.resolve('.vscode-test/user-data');
// Keep this path short: macOS caps Unix-domain socket paths at 103 chars and
// VS Code creates `<userDataDir>/<x.y>-main.sock`. An in-workspace location
// (e.g. /Users/runner/work/<repo>/<repo>/.vscode-test/user-data) overflows.
const userDataDir = path.join(os.tmpdir(), 'vsct-ud');
Comment thread
eleanorjboyd marked this conversation as resolved.

// Seed user settings.json so the extension actually activates: without
// `python.useEnvironmentsExtension=true` it short-circuits during activation
// and the smoke/e2e/integration tests see no registered managers.
const userDir = path.join(userDataDir, 'User');
fs.mkdirSync(userDir, { recursive: true });
fs.writeFileSync(
path.join(userDir, 'settings.json'),
JSON.stringify({ 'python.useEnvironmentsExtension': true }) + '\n',
);

export default defineConfig([
{
Expand Down
43 changes: 43 additions & 0 deletions src/managers/common/fastPath.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import * as fs from 'fs';
import { Uri } from 'vscode';
import { GetEnvironmentScope, PythonEnvironment, PythonEnvironmentApi } from '../../api';
import { traceError, traceVerbose, traceWarn } from '../../common/logging';
Expand Down Expand Up @@ -48,6 +49,28 @@ export function getProjectFsPathForScope(api: Pick<PythonEnvironmentApi, 'getPyt
return api.getPythonProject(scope)?.uri.fsPath ?? scope.fsPath;
}

/**
* Cheap local existence check on the persisted path. If the cached interpreter has
* been deleted/renamed/moved we can short-circuit before spawning PET (which holds
* the cache mutex and can take up to the spawn timeout to fail).
*
* Returns true if the path exists (or if the check itself errors — in which case we
* fall through to the normal resolve so we don't introduce a new failure mode).
*/
async function persistedPathExists(persistedPath: string): Promise<boolean> {
try {
await fs.promises.access(persistedPath);
return true;
} catch (err) {
const code = (err as NodeJS.ErrnoException)?.code;
if (code === 'ENOENT' || code === 'ENOTDIR') {
return false;
}
// Unknown error (e.g. EACCES) — don't treat as missing; let resolve() decide.
return true;
}
}

/**
* Attempts fast-path resolution for manager.get(): if full initialization hasn't completed yet
* and there's a persisted environment for the workspace, resolve it directly via nativeFinder
Expand Down Expand Up @@ -100,6 +123,18 @@ export async function tryFastPathGet(opts: FastPathOptions): Promise<FastPathRes
const persistedPath = await getGlobalPersistedPath();

if (persistedPath) {
// Cheap local existence check — avoids spawning PET (and waiting on the
// bounded timeout) when the cached interpreter has been removed.
if (!(await persistedPathExists(persistedPath))) {
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch.elapsedTime, {
managerLabel: opts.label,
result: 'stale',
});
traceVerbose(
`[${opts.label}] Fast path: persisted path '${persistedPath}' does not exist, falling through to slow path`,
);
return undefined;
}
try {
const resolved = await opts.resolve(persistedPath);
if (resolved) {
Expand Down Expand Up @@ -136,6 +171,14 @@ export async function tryFastPathGet(opts: FastPathOptions): Promise<FastPathRes
const persistedPath = await opts.getPersistedPath(opts.getProjectFsPath(scope));

if (persistedPath) {
// Cheap local existence check — avoids spawning PET (and waiting on the
// bounded timeout) when the cached interpreter has been removed.
if (!(await persistedPathExists(persistedPath))) {
traceVerbose(
`[${opts.label}] Fast path: persisted path '${persistedPath}' does not exist, falling through to slow path`,
);
return undefined;
}
try {
const resolved = await opts.resolve(persistedPath);
if (resolved) {
Expand Down
45 changes: 38 additions & 7 deletions src/test/managers/common/fastPath.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ interface FastPathTestOptions {

function createOpts(overrides?: Partial<FastPathOptions>): FastPathTestOptions {
const setInitialized = sinon.stub();
const persistedPath = path.resolve('persisted', 'path');
const persistedPath = __filename;
return {
opts: {
initialized: undefined,
Expand Down Expand Up @@ -77,7 +77,7 @@ suite('tryFastPathGet', () => {
});

test('returns resolved env for global scope when getGlobalPersistedPath returns a path', async () => {
const globalPath = path.resolve('usr', 'bin', 'python3');
const globalPath = __filename;
const resolve = sinon.stub().resolves(createMockEnv(globalPath));
const { opts } = createOpts({
scope: undefined,
Expand Down Expand Up @@ -116,7 +116,7 @@ suite('tryFastPathGet', () => {
});

test('reports stale when global cached path resolves to undefined', async () => {
const globalPath = path.resolve('usr', 'bin', 'python3');
const globalPath = __filename;
const { opts } = createOpts({
scope: undefined,
getGlobalPersistedPath: sinon.stub().resolves(globalPath),
Expand All @@ -132,7 +132,7 @@ suite('tryFastPathGet', () => {
});

test('returns undefined for global scope when cached path resolve fails', async () => {
const globalPath = path.resolve('usr', 'bin', 'python3');
const globalPath = __filename;
const { opts } = createOpts({
scope: undefined,
getGlobalPersistedPath: sinon.stub().resolves(globalPath),
Expand All @@ -150,7 +150,7 @@ suite('tryFastPathGet', () => {
});

test('global scope fast path starts background init when initialized is undefined', async () => {
const globalPath = path.resolve('usr', 'bin', 'python3');
const globalPath = __filename;
const startBackgroundInit = sinon.stub().resolves();
const { opts, setInitialized } = createOpts({
scope: undefined,
Expand Down Expand Up @@ -201,6 +201,37 @@ suite('tryFastPathGet', () => {
assert.strictEqual(result, undefined);
});

test('skips resolve and falls through when workspace persisted path no longer exists on disk', async () => {
const missingPath = path.resolve('does', 'not', 'exist', 'python-missing');
const resolve = sinon.stub().resolves(createMockEnv(missingPath));
const { opts } = createOpts({
getPersistedPath: sinon.stub().resolves(missingPath),
resolve,
});
const result = await tryFastPathGet(opts);

assert.strictEqual(result, undefined, 'Should fall through when cached path is missing');
assert.ok(resolve.notCalled, 'Should not invoke resolve (and thus PET) when path is missing');
});

test('skips resolve and reports stale when global persisted path no longer exists on disk', async () => {
const missingPath = path.resolve('does', 'not', 'exist', 'python-missing');
const resolve = sinon.stub().resolves(createMockEnv(missingPath));
const { opts } = createOpts({
scope: undefined,
getGlobalPersistedPath: sinon.stub().resolves(missingPath),
resolve,
});
const result = await tryFastPathGet(opts);

assert.strictEqual(result, undefined, 'Should fall through when cached global path is missing');
assert.ok(resolve.notCalled, 'Should not invoke resolve (and thus PET) when path is missing');
assert.ok(sendTelemetryStub.calledOnce, 'Should send telemetry for stale global cache');
const [eventName, , props] = sendTelemetryStub.firstCall.args;
assert.strictEqual(eventName, EventNames.GLOBAL_ENV_CACHE);
assert.strictEqual(props.result, 'stale');
});

test('calls getProjectFsPath with the scope Uri', async () => {
const scope = Uri.file(path.resolve('my', 'project'));
const getProjectFsPath = sinon.stub().returns(scope.fsPath);
Expand All @@ -214,7 +245,7 @@ suite('tryFastPathGet', () => {
test('passes project fsPath to getPersistedPath', async () => {
const projectPath = path.resolve('project', 'path');
const getProjectFsPath = sinon.stub().returns(projectPath);
const getPersistedPath = sinon.stub().resolves(path.resolve('persisted'));
const getPersistedPath = sinon.stub().resolves(__filename);
const { opts } = createOpts({
getProjectFsPath,
getPersistedPath,
Expand Down Expand Up @@ -266,7 +297,7 @@ suite('tryFastPathGet', () => {
const getPersistedPath = sinon.stub().callsFake(
() =>
new Promise<string | undefined>((resolve) => {
releasePersistedRead = () => resolve(path.resolve('persisted', 'path'));
releasePersistedRead = () => resolve(__filename);
}),
);
const { opts, setInitialized } = createOpts({ getPersistedPath });
Expand Down
4 changes: 4 additions & 0 deletions src/test/managers/fastPath.get.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

import * as assert from 'assert';
import * as fs from 'fs';
import * as path from 'path';
import * as sinon from 'sinon';
import { Uri } from 'vscode';
Expand Down Expand Up @@ -284,6 +285,9 @@ suite('Manager get() fast path', () => {
setup(() => {
sandbox = sinon.createSandbox();
sandbox.stub(windowApis, 'withProgress').callsFake((_opts, cb) => cb(undefined as never, undefined as never));
// fastPath.ts now does a real fs.access on the persisted path; these tests use
// synthetic paths that don't exist on disk, so pretend every path is present.
sandbox.stub(fs.promises, 'access').resolves();
});

teardown(() => {
Expand Down
Loading