Skip to content

Commit

Permalink
Remove Positron patches to make Python tests work with TypeScript v5 (#…
Browse files Browse the repository at this point in the history 
…5309)

Addresses #4932. We no longer need these since they've also upgraded
upstream and applied their own fixes.
  • Loading branch information
@seeM
seeM authored Nov 12, 2024
1 parent 70e96ec commit d753d0d
Show file tree
Hide file tree
Showing 10 changed files with 25 additions and 108 deletions.
5 changes: 1 addition & 4 deletions extensions/positron-python/src/test/common/process/pythonToolService.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,7 @@ suite('Process - Python tool execution service', () => {
when(pythonService.execModule(anything(), anything(), anything())).thenResolve(executionResult);
const pythonServiceInstance = instance(pythonService);

// --- Start Positron ---
// No longer needed and errors since we already set `.then` in positron/initialize/patchMockingLibs.
// (pythonServiceInstance as any).then = undefined;
// --- End Positron ---
(pythonServiceInstance as any).then = undefined;

executionFactory = mock(PythonExecutionFactory);
when(executionFactory.create(anything())).thenResolve(pythonServiceInstance);
Expand Down
5 changes: 1 addition & 4 deletions extensions/positron-python/src/test/common/serviceRegistry.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,7 @@ suite('Common - Service Registry', () => {
if (mapping.length === 2) {
serviceManager
.setup((s) =>
// --- Start Positron ---
// Fix for Typescript v5
s.addSingleton<unknown>(
// --- End Positron ---
s.addSingleton(
typemoq.It.isValue(mapping[0] as any),
typemoq.It.is((value: any) => mapping[1] === value),
),
Expand Down
8 changes: 0 additions & 8 deletions extensions/positron-python/src/test/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ import { IS_CI_SERVER_TEST_DEBUGGER, MOCHA_REPORTER_JUNIT } from './ciConstants'
import { IS_MULTI_ROOT_TEST, MAX_EXTENSION_ACTIVATION_TIME, TEST_RETRYCOUNT, TEST_TIMEOUT } from './constants';
import { initialize } from './initialize';
import { initializeLogger } from './testLogger';
// --- Start Positron ---
import { patchMockingLibs } from './positron/initialize';
// --- End Positron ---

initializeLogger();

Expand Down Expand Up @@ -162,11 +159,6 @@ export async function run(): Promise<void> {
console.error('Failed to activate python extension without errors', ex);
}

// --- Start Positron ---
// See patchMockingLibs docs for why this is necessary.
patchMockingLibs();
// --- End Positron ---

// Run the tests.
await new Promise<void>((resolve, reject) => {
mocha.run((failures) => {
Expand Down
4 changes: 0 additions & 4 deletions ...tron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,6 @@ suite('Terminal Environment Variable Collection Service', () => {
// --- End Positron ---

reset(experimentService);
// --- Start Positron ---
// Use globalCollection instead of collection since we're using a later version of @types/vscode
when(context.environmentVariableCollection).thenReturn(instance(globalCollection));
// --- End Positron ---
when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(false);
const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection');
applyCollectionStub.resolves();
Expand Down
4 changes: 1 addition & 3 deletions extensions/positron-python/src/test/mocks/helper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ export function createTypeMoq<T>(
// Use typemoqs for those things that are resolved as promises. mockito doesn't allow nesting of mocks. ES6 Proxy class
// is the problem. We still need to make it thenable though. See this issue: https://github.com/florinn/typemoq/issues/67
const result = TypeMoq.Mock.ofType<T>(targetCtor, behavior, shouldOverrideTarget, ...targetCtorArgs);
// --- Start Positron ---
// result.setup((x: any) => x.then).returns(() => undefined);
// --- End Positron ---
result.setup((x: any) => x.then).returns(() => undefined);
return result;
}
15 changes: 8 additions & 7 deletions extensions/positron-python/src/test/positron/createEnvApi.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { CreateEnvironmentOptionsInternal } from '../../client/pythonEnvironment
import { IPythonRuntimeManager } from '../../client/positron/manager';
import { IInterpreterQuickPick } from '../../client/interpreter/configuration/types';
import { createEnvironmentAndRegister } from '../../client/positron/createEnvApi';
import { createTypeMoq } from '../mocks/helper';

chaiUse(chaiAsPromised.default);

Expand All @@ -33,7 +34,7 @@ suite('Positron Create Environment APIs', () => {
let handleCreateEnvironmentCommandStub: sinon.SinonStub;

const disposables: IDisposableRegistry = [];
const mockProvider = typemoq.Mock.ofType<CreateEnvironmentProvider>();
const mockProvider = createTypeMoq<CreateEnvironmentProvider>();
const mockProviders = [mockProvider.object];

let pythonRuntimeManager: typemoq.IMock<IPythonRuntimeManager>;
Expand Down Expand Up @@ -67,10 +68,10 @@ suite('Positron Create Environment APIs', () => {
registerCommandStub = sinon.stub(commandApis, 'registerCommand');
handleCreateEnvironmentCommandStub = sinon.stub(createEnvironmentApis, 'handleCreateEnvironmentCommand');

pythonRuntimeManager = typemoq.Mock.ofType<IPythonRuntimeManager>();
pathUtils = typemoq.Mock.ofType<IPathUtils>();
interpreterQuickPick = typemoq.Mock.ofType<IInterpreterQuickPick>();
interpreterPathService = typemoq.Mock.ofType<IInterpreterPathService>();
pythonRuntimeManager = createTypeMoq<IPythonRuntimeManager>();
pathUtils = createTypeMoq<IPathUtils>();
interpreterQuickPick = createTypeMoq<IInterpreterQuickPick>();
interpreterPathService = createTypeMoq<IInterpreterPathService>();

registerCommandStub.callsFake((_command: string, _callback: (...args: any[]) => any) => ({
dispose: () => {
Expand Down Expand Up @@ -98,7 +99,7 @@ suite('Positron Create Environment APIs', () => {
const resultPath = '/path/to/created/env';
pythonRuntimeManager
.setup((p) => p.registerLanguageRuntimeFromPath(resultPath))
.returns(() => Promise.resolve(typemoq.Mock.ofType<positron.LanguageRuntimeMetadata>().object))
.returns(() => Promise.resolve(createTypeMoq<positron.LanguageRuntimeMetadata>().object))
.verifiable(typemoq.Times.once());
handleCreateEnvironmentCommandStub.returns(Promise.resolve({ path: resultPath }));

Expand All @@ -117,7 +118,7 @@ suite('Positron Create Environment APIs', () => {
test(`Environment creation fails when options are missing: ${optionsName} `, async () => {
pythonRuntimeManager
.setup((p) => p.registerLanguageRuntimeFromPath(typemoq.It.isAny()))
.returns(() => Promise.resolve(typemoq.Mock.ofType<positron.LanguageRuntimeMetadata>().object))
.returns(() => Promise.resolve(createTypeMoq<positron.LanguageRuntimeMetadata>().object))
.verifiable(typemoq.Times.never());

const result = await createEnvironmentAndRegister(mockProviders, pythonRuntimeManager.object, options);
Expand Down
49 changes: 1 addition & 48 deletions extensions/positron-python/src/test/positron/initialize.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
*--------------------------------------------------------------------------------------------*/

import * as TypeMoq from 'typemoq';
import * as tsMockito from 'ts-mockito';

// Initialize Positron for Python extension integration tests.
/** Initialize Positron for Python extension integration tests. */
export function initializePositron(): void {
// Use a late import since this module may be imported without Positron being installed
// e.g. in unit tests.
Expand All @@ -18,47 +15,3 @@ export function initializePositron(): void {
.getConfiguration('positron.interpreters')
.update('automaticStartup', false, vscode.ConfigurationTarget.Global);
}

// Save a reference to the original patched objects.
const originalTypeMoqMockOfType = TypeMoq.Mock.ofType;
const originalTsMockitoMock = tsMockito.mock;

/**
* InversifyJS v6 (required by TypeScript v5) tries to await bound objects if they
* look like promises. TypeMoq's dynamic mocks and ts-mockito instances unfortunately
* do look like promises by default (they are functions and their properties are functions,
* including `then`). This causes unexpected behavior in InversifyJS.
*
* Here, we patch `TypeMoq.Mock.ofType` and `tsMockito.mock` to setup the `then` property to return
* undefined to avoid the above behavior.
*/
export function patchMockingLibs(): void {
// Patch TypeMoq.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
TypeMoq.Mock.ofType = function <U>(targetConstructor?: any, behavior?: any, ...args: never[]): TypeMoq.IMock<U> {
const mock = originalTypeMoqMockOfType(targetConstructor, behavior, ...args);

// Only setup `then` if the target constructor is undefined, meaning the mock is dynamic
if (targetConstructor === undefined) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const returnsResult = mock.setup((x: any) => x.then).returns(() => undefined);

// If a user configures 'strict' mock behavior, all setups are expected to be called
// once. Override this by allowing `then` to be called any number of times.
if (behavior === TypeMoq.MockBehavior.Strict) {
returnsResult.verifiable(TypeMoq.Times.atLeast(0));
}
}

return mock as TypeMoq.IMock<U>;
};

// Patch ts-mockito.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(tsMockito as any).mock = function <T>(...args: never[]): T {
const mocked = originalTsMockitoMock(...args);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
tsMockito.when((mocked as any).then).thenReturn(undefined);
return mocked as T;
};
}
19 changes: 10 additions & 9 deletions extensions/positron-python/src/test/positron/manager.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { PythonRuntimeManager } from '../../client/positron/manager';
import { IInterpreterService } from '../../client/interpreter/contracts';
import { PythonEnvironment } from '../../client/pythonEnvironments/info';
import { mockedPositronNamespaces } from '../vscode-mock';
import { createTypeMoq } from '../mocks/helper';

suite('Python runtime manager', () => {
const pythonPath = 'pythonPath';
Expand All @@ -34,12 +35,12 @@ suite('Python runtime manager', () => {
let disposables: IDisposable[];

setup(() => {
runtimeMetadata = TypeMoq.Mock.ofType<positron.LanguageRuntimeMetadata>();
interpreter = TypeMoq.Mock.ofType<PythonEnvironment>();
configService = TypeMoq.Mock.ofType<IConfigurationService>();
envVarsProvider = TypeMoq.Mock.ofType<IEnvironmentVariablesProvider>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
runtimeMetadata = createTypeMoq<positron.LanguageRuntimeMetadata>();
interpreter = createTypeMoq<PythonEnvironment>();
configService = createTypeMoq<IConfigurationService>();
envVarsProvider = createTypeMoq<IEnvironmentVariablesProvider>();
interpreterService = createTypeMoq<IInterpreterService>();
serviceContainer = createTypeMoq<IServiceContainer>();

runtimeMetadata.setup((r) => r.runtimeId).returns(() => 'runtimeId');
runtimeMetadata
Expand Down Expand Up @@ -99,7 +100,7 @@ suite('Python runtime manager', () => {
// });

test('restoreSession: creates and returns a Python runtime session', async () => {
const sessionMetadata = TypeMoq.Mock.ofType<positron.RuntimeSessionMetadata>();
const sessionMetadata = createTypeMoq<positron.RuntimeSessionMetadata>();
const pythonRuntimeSession = sinon.stub(session, 'PythonRuntimeSession');

const result = await pythonRuntimeManager.restoreSession(runtimeMetadata.object, sessionMetadata.object);
Expand Down Expand Up @@ -130,7 +131,7 @@ suite('Python runtime manager', () => {
pythonRuntimeManager.registeredPythonRuntimes.set(pythonPath, runtimeMetadata.object);

// Create a metadata fragment (only contains extra data python path).
const runtimeMetadataFragment = TypeMoq.Mock.ofType<positron.LanguageRuntimeMetadata>();
const runtimeMetadataFragment = createTypeMoq<positron.LanguageRuntimeMetadata>();
runtimeMetadataFragment.setup((r) => r.extraRuntimeData).returns(() => ({ pythonPath }));

// Override the pathExists stub to return true and validate the metadata.
Expand All @@ -142,7 +143,7 @@ suite('Python runtime manager', () => {
});

test('validateMetadata: throws if extra data is missing', async () => {
const invalidRuntimeMetadata = TypeMoq.Mock.ofType<positron.LanguageRuntimeMetadata>();
const invalidRuntimeMetadata = createTypeMoq<positron.LanguageRuntimeMetadata>();
assert.rejects(() => pythonRuntimeManager.validateMetadata(invalidRuntimeMetadata.object));
});

Expand Down
15 changes: 3 additions & 12 deletions extensions/positron-python/src/test/terminals/serviceRegistry.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,7 @@ suite('Terminal - Service Registry', () => {
if (args.length === 2) {
services
.setup((s) =>
// --- Start Positron ---
// Fix for Typescript v5
s.addSingleton<unknown>(
// --- End Positron ---
s.addSingleton(
typemoq.It.is((v: any) => args[0] === v),
typemoq.It.is((value: any) => args[1] === value),
),
Expand All @@ -56,10 +53,7 @@ suite('Terminal - Service Registry', () => {
} else {
services
.setup((s) =>
// --- Start Positron ---
// Fix for Typescript v5
s.addSingleton<unknown>(
// --- End Positron ---
s.addSingleton(
typemoq.It.is((v: any) => args[0] === v),
typemoq.It.is((value: any) => args[1] === value),

Expand All @@ -71,10 +65,7 @@ suite('Terminal - Service Registry', () => {
});
services
.setup((s) =>
// --- Start Positron ---
// Fix for Typescript v5
s.addBinding<ITerminalEnvVarCollectionService, IExtensionActivationService>(
// --- End Positron ---
s.addBinding(
typemoq.It.is((v: any) => ITerminalEnvVarCollectionService === v),
typemoq.It.is((value: any) => IExtensionActivationService === value),
),
Expand Down
9 changes: 0 additions & 9 deletions extensions/positron-python/src/test/vscode-mock.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ type Positron = typeof positron;
const mockedPositron: Partial<Positron> = {};
export const mockedPositronNamespaces: { [P in keyof Positron]?: Positron[P] } = {};

// TODO(seem): mockedPositron is currently empty. We can update it as needed as we add tests.

import { patchMockingLibs } from './positron/initialize';

function generatePositronMock<K extends keyof Positron>(name: K): void {
const mockedObj = mock<Positron[K]>();
(mockedPositron as any)[name] = instance(mockedObj);
Expand Down Expand Up @@ -109,11 +105,6 @@ export function initialize() {
}
return originalLoad.apply(this, arguments);
};

// --- Start Positron ---
// See patchMockingLibs docs for why this is necessary.
patchMockingLibs();
// --- End Positron ---
}

mockedVSCode.ThemeIcon = vscodeMocks.ThemeIcon;
Expand Down

0 comments on commit d753d0d

Please sign in to comment.