diff --git a/.github/workflows/unit.yml b/.github/workflows/unit.yml index e1199ace5..349203cbe 100644 --- a/.github/workflows/unit.yml +++ b/.github/workflows/unit.yml @@ -22,18 +22,22 @@ jobs: - name: Publish Test Summary Results if: ${{ always() }} run: | + if [ ! -f test-reports/vitest.junit.xml ]; then + echo "No JUnit report found" + mkdir -p pr-comment + exit 1 + fi + npm run report:junit2ctrf npm run report:ctrfsummary sed -i 's/

Test Summary<\/h3>/

Unit Test Summary<\/h3>/' $GITHUB_STEP_SUMMARY npm run report:prcomment # The junit-to-ctrf npm package exits with a 0 status code even if - # it fails to parse the JUnit report, so check for the file manually + # it fails to parse the JUnit report, so check for the CTRF file manually # and explicilty exit with a non-zero status code if it's not found. - # We do this after npm run report:prcomment so that the PR number can - # still be associated in subsequent steps. - if [ ! -e test-reports/vitest.junit.xml ]; then - echo "No JUnit report found at test-reports/vitest.junit.xml" + if [ ! -f test-reports/ctrf-report.json ]; then + echo "CTRF report not created - junit-to-ctrf may have failed to parse JUnit report" exit 1 fi - name: Save PR Number diff --git a/docs/workspace-setup.md b/docs/workspace-setup.md index 59b7c60b8..5a6670803 100644 --- a/docs/workspace-setup.md +++ b/docs/workspace-setup.md @@ -14,6 +14,8 @@ A `requirements.txt` file can be generated containing all of the packages instal ## Managed pip Servers (Community only) +> **Requirement:** Managed pip servers require the [Python Environments](https://marketplace.visualstudio.com/items?itemName=ms-python.vscode-python-envs) extension (`ms-python.vscode-python-envs`) to be installed in VS Code. + If you want to manage Deephaven servers from within the extension, include `deephaven-server` in the venv pip installation. Once installed, clicking the `refresh` button in the server tree panel should reveal a `Managed` servers node. diff --git a/src/controllers/PipServerController.spec.ts b/src/controllers/PipServerController.spec.ts new file mode 100644 index 000000000..45a4ff0ed --- /dev/null +++ b/src/controllers/PipServerController.spec.ts @@ -0,0 +1,314 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import * as vscode from 'vscode'; +import { PipServerController } from './PipServerController'; +import type { PythonEnvironment, PythonEnvironmentApi } from '../util'; +import type { IServerManager, IToastService } from '../types'; + +// See __mocks__/vscode.ts for the mock implementation +vi.mock('vscode'); + +vi.mock('../util', async () => { + const actual = await vi.importActual('../util'); + return { + ...actual, + getPythonEnvsExtensionApi: vi.fn(), + }; +}); + +vi.mock('../services', async () => { + const actual = + await vi.importActual('../services'); + return { + ...actual, + pollUntilTrue: vi + .fn() + .mockReturnValue({ promise: Promise.resolve(), cancel: vi.fn() }), + }; +}); + +vi.mock('../dh/dhc', () => ({ + isDhcServerRunning: vi.fn().mockResolvedValue(true), +})); + +// Import after mocks are set up +const { getPythonEnvsExtensionApi } = await import('../util'); + +const mockEnvironment: PythonEnvironment = { + envId: { id: 'env1', managerId: 'venv' }, + name: 'myenv', + displayName: 'My Env', + displayPath: '/path/to/env', + version: '3.11.0', + environmentPath: {} as vscode.Uri, + execInfo: { + run: { executable: '/path/to/env/bin/python' }, + }, + sysPrefix: '/path/to/env', +}; + +const mockPackages = [ + { + pkgId: { id: 'dh', managerId: 'pip', environmentId: 'env1' }, + name: 'deephaven-server', + displayName: 'Deephaven Server', + version: '0.36.0', + }, +]; + +function createMockExtension( + isActive: boolean, + envResult: PythonEnvironment | undefined, + packagesResult: typeof mockPackages | undefined +): vscode.Extension { + const api = { + getEnvironment: vi.fn().mockResolvedValue(envResult), + getPackages: vi.fn().mockResolvedValue(packagesResult), + onDidChangePackages: vi.fn().mockReturnValue({ dispose: vi.fn() }), + }; + return { + isActive, + activate: vi.fn().mockResolvedValue(undefined), + exports: api, + } as unknown as vscode.Extension; +} + +function createController(): PipServerController { + const context = { + subscriptions: [], + extension: { packageJSON: { version: '1.0.0' } }, + } as unknown as vscode.ExtensionContext; + + const serverManager = { + onDidLoadConfig: vi.fn(), + syncManagedServers: vi.fn().mockResolvedValue(undefined), + canStartServer: false, + getServers: vi.fn().mockReturnValue([]), + getServer: vi.fn(), + disconnectFromServer: vi.fn(), + updateStatus: vi.fn(), + } as unknown as IServerManager; + + const outputChannel = { + appendLine: vi.fn(), + } as unknown as vscode.OutputChannel; + + const toastService = { + error: vi.fn(), + info: vi.fn(), + } as unknown as IToastService; + + return new PipServerController( + context, + serverManager, + outputChannel, + toastService + ); +} + +beforeEach(() => { + vi.clearAllMocks(); + + // Add missing properties to the vscode.window mock + Object.assign(vscode.window, { + onDidCloseTerminal: vi + .fn() + .mockName('onDidCloseTerminal') + .mockReturnValue({ dispose: vi.fn() }), + terminals: [], + createTerminal: vi.fn().mockReturnValue({ + sendText: vi.fn(), + exitStatus: undefined, + dispose: vi.fn(), + }), + }); +}); + +describe('getPythonInterpreterPath', () => { + it('returns null when Python Environments extension is not found', async () => { + vi.mocked(getPythonEnvsExtensionApi).mockReturnValue(undefined); + + const controller = createController(); + const result = await controller.getPythonInterpreterPath(); + + expect(result).toBeNull(); + }); + + it('returns null when getEnvironment returns undefined', async () => { + const mockExt = createMockExtension(true, undefined, undefined); + vi.mocked(getPythonEnvsExtensionApi).mockReturnValue( + mockExt as unknown as ReturnType + ); + + const controller = createController(); + const result = await controller.getPythonInterpreterPath(); + + expect(result).toBeNull(); + }); + + it('returns executable path when environment is found (extension already active)', async () => { + const mockExt = createMockExtension(true, mockEnvironment, undefined); + vi.mocked(getPythonEnvsExtensionApi).mockReturnValue( + mockExt as unknown as ReturnType + ); + + const controller = createController(); + // Clear mocks after constructor to only track calls from getPythonInterpreterPath + vi.clearAllMocks(); + vi.mocked(getPythonEnvsExtensionApi).mockReturnValue( + mockExt as unknown as ReturnType + ); + + const result = await controller.getPythonInterpreterPath(); + + expect(result).toBe('/path/to/env/bin/python'); + // When extension is already active, activate should not be called + expect(mockExt.activate).not.toHaveBeenCalled(); + expect(mockExt.exports.getEnvironment).toHaveBeenCalledWith(undefined); + }); + + it('activates extension and returns executable path when extension is not yet active', async () => { + const mockExt = createMockExtension(false, mockEnvironment, undefined); + vi.mocked(getPythonEnvsExtensionApi).mockReturnValue( + mockExt as unknown as ReturnType + ); + + const controller = createController(); + const result = await controller.getPythonInterpreterPath(); + + expect(mockExt.activate).toHaveBeenCalled(); + expect(result).toBe('/path/to/env/bin/python'); + }); +}); + +describe('checkPipInstall', () => { + it('returns isAvailable false on unsupported platform', async () => { + vi.stubEnv('PLATFORM', 'win32'); + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { + value: 'win32', + configurable: true, + }); + + const controller = createController(); + const result = await controller.checkPipInstall(); + + expect(result.isAvailable).toBe(false); + + Object.defineProperty(process, 'platform', { + value: originalPlatform, + configurable: true, + }); + }); + + it('returns isAvailable false when Python interpreter not found', async () => { + vi.mocked(getPythonEnvsExtensionApi).mockReturnValue(undefined); + + const controller = createController(); + const result = await controller.checkPipInstall(); + + expect(result.isAvailable).toBe(false); + }); + + it('returns isAvailable false when Python extension not found for package check', async () => { + // First call (getPythonInterpreterPath) returns extension, second call (checkPipInstall body) returns undefined + const mockExt = createMockExtension(true, mockEnvironment, undefined); + vi.mocked(getPythonEnvsExtensionApi) + .mockReturnValueOnce( + mockExt as unknown as ReturnType + ) + .mockReturnValueOnce(undefined); + + const controller = createController(); + const result = await controller.checkPipInstall(); + + expect(result.isAvailable).toBe(false); + }); + + it('returns isAvailable false when getEnvironment returns null during package check', async () => { + const mockExtWithEnv = createMockExtension( + true, + mockEnvironment, + undefined + ); + const mockExtNoEnv = createMockExtension(true, undefined, undefined); + + vi.mocked(getPythonEnvsExtensionApi) + .mockReturnValueOnce( + mockExtWithEnv as unknown as ReturnType< + typeof getPythonEnvsExtensionApi + > + ) + .mockReturnValueOnce( + mockExtNoEnv as unknown as ReturnType + ); + + const controller = createController(); + const result = await controller.checkPipInstall(); + + expect(result.isAvailable).toBe(false); + }); + + it('returns isAvailable false when deephaven-server is not in packages', async () => { + const packagesWithoutDh = [ + { + pkgId: { id: 'np', managerId: 'pip', environmentId: 'env1' }, + name: 'numpy', + displayName: 'NumPy', + version: '1.26.0', + }, + ]; + const mockExt = createMockExtension( + true, + mockEnvironment, + packagesWithoutDh + ); + vi.mocked(getPythonEnvsExtensionApi).mockReturnValue( + mockExt as unknown as ReturnType + ); + + const controller = createController(); + const result = await controller.checkPipInstall(); + + expect(result.isAvailable).toBe(false); + }); + + it('returns isAvailable false when getPackages returns undefined', async () => { + const mockExt = createMockExtension(true, mockEnvironment, undefined); + vi.mocked(getPythonEnvsExtensionApi).mockReturnValue( + mockExt as unknown as ReturnType + ); + + const controller = createController(); + const result = await controller.checkPipInstall(); + + expect(result.isAvailable).toBe(false); + }); + + it('returns isAvailable true with interpreter path and environment when deephaven-server is installed', async () => { + const mockExt = createMockExtension(true, mockEnvironment, mockPackages); + vi.mocked(getPythonEnvsExtensionApi).mockReturnValue( + mockExt as unknown as ReturnType + ); + + const controller = createController(); + const result = await controller.checkPipInstall(); + + expect(result.isAvailable).toBe(true); + if (result.isAvailable) { + expect(result.interpreterPath).toBe('/path/to/env/bin/python'); + expect(result.environment).toBe(mockEnvironment); + } + }); + + it('calls getPackages with the environment returned by getEnvironment', async () => { + const mockExt = createMockExtension(true, mockEnvironment, mockPackages); + vi.mocked(getPythonEnvsExtensionApi).mockReturnValue( + mockExt as unknown as ReturnType + ); + + const controller = createController(); + await controller.checkPipInstall(); + + expect(mockExt.exports.getPackages).toHaveBeenCalledWith(mockEnvironment); + }); +}); diff --git a/src/controllers/PipServerController.ts b/src/controllers/PipServerController.ts index 7c0b9d5df..a6df6fb95 100644 --- a/src/controllers/PipServerController.ts +++ b/src/controllers/PipServerController.ts @@ -1,11 +1,12 @@ import * as vscode from 'vscode'; -import { execFileSync } from 'node:child_process'; import path from 'node:path'; import { - getMsPythonExtensionApi, + getPythonEnvsExtensionApi, getPipServerUrl, Logger, + PackageChangeKind, parsePort, + type PythonEnvironment, } from '../util'; import type { IDisposable, @@ -58,6 +59,28 @@ export class PipServerController implements IDisposable { ); this._serverManager.onDidLoadConfig(this.onDidLoadConfig); + + const pythonExtension = getPythonEnvsExtensionApi(); + if (pythonExtension != null) { + console.log('[TESTING]', pythonExtension.exports); + void pythonExtension.activate().then(() => { + pythonExtension.exports.onDidChangePackages( + ({ changes }) => { + const deephavenServerChanged = changes.some( + ({ pkg, kind }) => + pkg.name === 'deephaven-server' && + (kind === PackageChangeKind.add || + kind === PackageChangeKind.remove) + ); + if (deephavenServerChanged) { + void this.syncManagedServers({ forceCheck: true }); + } + }, + undefined, + this._context.subscriptions + ); + }); + } } private readonly _context: vscode.ExtensionContext; @@ -84,8 +107,12 @@ export class PipServerController implements IDisposable { * servers can be managed from the extension. */ checkPipInstall = async (): Promise< - | { isAvailable: true; interpreterPath: string } - | { isAvailable: false; interpreterPath?: never } + | { + isAvailable: true; + interpreterPath: string; + environment: PythonEnvironment; + } + | { isAvailable: false; interpreterPath?: never; environment?: never } > => { if (!PIP_SERVER_SUPPORTED_PLATFORMS.has(process.platform)) { logger.debug(`Pip server not supported on platform: ${process.platform}`); @@ -102,12 +129,35 @@ export class PipServerController implements IDisposable { logger.debug('Using Python interpreter:', pythonInterpreterPath); - try { - execFileSync(pythonInterpreterPath, ['-c', 'import deephaven_server']); - return { isAvailable: true, interpreterPath: pythonInterpreterPath }; - } catch (err) { + const pythonExtension = getPythonEnvsExtensionApi(); + if (pythonExtension == null) { + return { isAvailable: false }; + } + + if (!pythonExtension.isActive) { + await pythonExtension.activate(); + } + + const api = pythonExtension.exports; + const env = await api.getEnvironment(undefined); + if (env == null) { + return { isAvailable: false }; + } + + const packages = await api.getPackages(env); + const hasDeephavenServer = packages?.some( + pkg => pkg.name === 'deephaven-server' + ); + + if (!hasDeephavenServer) { return { isAvailable: false }; } + + return { + isAvailable: true, + interpreterPath: pythonInterpreterPath, + environment: env, + }; }; /** @@ -159,11 +209,11 @@ export class PipServerController implements IDisposable { }; /** - * Get Python interpreter path from the MS Python extension. + * Get Python interpreter path from the Python Environments extension (ms-python.vscode-python-envs). * @returns The Python interpreter path or `null` if not found. */ getPythonInterpreterPath = async (): Promise => { - const pythonExtension = getMsPythonExtensionApi(); + const pythonExtension = getPythonEnvsExtensionApi(); if (pythonExtension == null) { logger.debug('Python extension not found'); @@ -174,11 +224,11 @@ export class PipServerController implements IDisposable { await pythonExtension.activate(); } - const pythonApi = pythonExtension.exports; - const interpreter = await pythonApi.environments.getActiveEnvironmentPath(); - logger.debug('Python interpreter:', interpreter); + const api = pythonExtension.exports; + const env = await api.getEnvironment(undefined); + logger.debug('Python interpreter:', env?.execInfo.run.executable); - return interpreter?.path ?? null; + return env?.execInfo.run.executable ?? null; }; /** @@ -248,7 +298,8 @@ export class PipServerController implements IDisposable { } // In case pip env has changed since last server check - const { isAvailable, interpreterPath } = await this.checkPipInstall(); + const { isAvailable, interpreterPath, environment } = + await this.checkPipInstall(); this._isPipServerInstalled = isAvailable; if (!isAvailable) { @@ -257,7 +308,6 @@ export class PipServerController implements IDisposable { } const interpreterBinDirPath = path.dirname(interpreterPath); - const interpreterEnvPath = path.dirname(interpreterBinDirPath); const terminal = vscode.window.createTerminal({ name: `Deephaven (${port})`, @@ -270,7 +320,7 @@ export class PipServerController implements IDisposable { // the workspace. PYTHONPATH: './', // Venv activation typically sets this, so mimic that here. - VIRTUAL_ENV: interpreterEnvPath, + VIRTUAL_ENV: environment.sysPrefix, /* eslint-enable @typescript-eslint/naming-convention */ }, isTransient: true, diff --git a/src/util/extensionApiUtils.ts b/src/util/extensionApiUtils.ts index f7bdc675b..6c92cfa9f 100644 --- a/src/util/extensionApiUtils.ts +++ b/src/util/extensionApiUtils.ts @@ -2,13 +2,77 @@ import * as vscode from 'vscode'; import type { ExtensionInfo, ExtensionVersion, McpVersion } from '../types'; import { uniqueId } from './idUtils'; -const MS_PYTHON_EXTENSION_ID = 'ms-python.python'; +const MS_PYTHON_ENVS_EXTENSION_ID = 'ms-python.vscode-python-envs'; -/** Microsoft Python extension api */ -interface MsPythonExtensionApi { - environments: { - getActiveEnvironmentPath: () => Promise<{ path: string }>; - }; +/** Options for executing a Python executable */ +interface PythonCommandRunConfiguration { + executable: string; + args?: string[]; +} + +/** Execution details for a Python environment */ +interface PythonEnvironmentExecutionInfo { + run: PythonCommandRunConfiguration; + activatedRun?: PythonCommandRunConfiguration; + activation?: PythonCommandRunConfiguration[]; + deactivation?: PythonCommandRunConfiguration[]; +} + +/** Unique identifier for a Python environment */ +interface PythonEnvironmentId { + id: string; + managerId: string; +} + +/** A Python environment from the ms-python.vscode-python-envs extension */ +export interface PythonEnvironment { + readonly envId: PythonEnvironmentId; + readonly name: string; + readonly displayName: string; + readonly displayPath: string; + readonly version: string; + readonly environmentPath: vscode.Uri; + readonly execInfo: PythonEnvironmentExecutionInfo; + readonly sysPrefix: string; + readonly description?: string; +} + +/** Unique identifier for a Python package */ +interface PackageId { + id: string; + managerId: string; + environmentId: string; +} + +/** A Python package from the ms-python.vscode-python-envs extension */ +interface Package { + readonly pkgId: PackageId; + readonly name: string; + readonly displayName: string; + readonly version?: string; + readonly description?: string; +} + +export enum PackageChangeKind { + add = 'add', + remove = 'remove', +} + +/** Arguments for the onDidChangePackages event */ +interface DidChangePackagesEventArgs { + environment: PythonEnvironment; + changes: { kind: PackageChangeKind; pkg: Package }[]; +} + +export type GetEnvironmentScope = undefined | vscode.Uri; + +/** Python Environments extension API (ms-python.vscode-python-envs) */ +export interface PythonEnvironmentApi { + getEnvironment( + scope: GetEnvironmentScope + ): Promise; + getPackages(environment: PythonEnvironment): Promise; + onDidChangePackages: vscode.Event; } /** Create ExtensionInfo from the ExtensionContext */ @@ -46,11 +110,11 @@ export function getExtensionVersion( return version as ExtensionVersion; } -/** Get Microsoft Python extension api */ -export function getMsPythonExtensionApi(): - | vscode.Extension +/** Get the Python Environments extension api (ms-python.vscode-python-envs) */ +export function getPythonEnvsExtensionApi(): + | vscode.Extension | undefined { - return vscode.extensions.getExtension( - MS_PYTHON_EXTENSION_ID + return vscode.extensions.getExtension( + MS_PYTHON_ENVS_EXTENSION_ID ); }