Test startup settings in VS Code with the Python extension (#5226)

lsp/.vscode-test.mjs

@@ -6,6 +6,7 @@
  */
 
 import { defineConfig } from '@vscode/test-cli';
+import { makeStartupSettingsWorkspaces } from './src/test/startup-settings/workspaces.mjs';
 
 // One configuration per window shape. The multi-root tests need two folders
 // open; the failed-start tests open a workspace whose settings break the first
@@ -18,6 +19,8 @@ import { defineConfig } from '@vscode/test-cli';
 // Mocha's 2s default is not enough for that on the slower CI runners.
 const mocha = {timeout: 20000};
 
+const startup = makeStartupSettingsWorkspaces(['instant', 'delay', 'drop']);
+
 export default defineConfig([
 	{
 		label: 'single-root',
@@ -51,5 +54,17 @@ export default defineConfig([
 		// first `client.start()` is made to fail.
 		workspaceFolder: "./src/test/fixtures/failed-start",
 		launchArgs: ['--disable-workspace-trust']
-	}
+	},
+	// The startup-settings tests run the real Python extension, because the
+	// extension asks it for the interpreter while it answers the server's
+	// configuration request. One window per mode of `lsp-shim.js`, because each
+	// mode decides how the server's startup goes.
+	...['instant', 'delay', 'drop'].map(mode => ({
+		label: `startup-settings-${mode}`,
+		files: `dist/test/startup-settings/${mode}/*.test.js`,
+		workspaceFolder: startup[mode],
+		installExtensions: ['ms-python.python'],
+		mocha,
+		launchArgs: ['--disable-workspace-trust']
+	}))
 ]);

lsp/src/test/startup-settings/common.ts

@@ -0,0 +1,193 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import * as assert from 'assert';
+import {promises as fs} from 'fs';
+import {join} from 'path';
+import * as vscode from 'vscode';
+import {PythonEnvironments} from '@vscode/python-environments';
+import {PythonExtension} from '@vscode/python-extension';
+
+export const fixtureFolder = (): vscode.Uri =>
+  vscode.workspace.workspaceFolders![0].uri;
+
+export const mainPy = (): vscode.Uri =>
+  vscode.Uri.joinPath(fixtureFolder(), 'main.py');
+
+/** The client messages in the order that `lsp-shim.js` forwarded them. */
+export async function shimLog(): Promise<Record<string, string>[]> {
+  const text = await fs.readFile(
+    join(fixtureFolder().fsPath, '.vscode', 'lsp-shim.jsonl'),
+    'utf8',
+  );
+  return text
+    .split('\n')
+    .filter(line => line !== '')
+    .map(line => JSON.parse(line));
+}
+
+/**
+ * The interpreter that the extension would send for the fixture folder. This
+ * asks the same APIs in the same order as `PythonEnvironment`: the Python
+ * Environments extension first, then the Python extension.
+ */
+async function selectedInterpreter(): Promise<string | undefined> {
+  try {
+    const api = await PythonEnvironments.api();
+    return (await api.getEnvironment(fixtureFolder()))?.execInfo?.run
+      ?.executable;
+  } catch {
+    const api = await PythonExtension.api();
+    return (await api.environments.getActiveEnvironmentPath(fixtureFolder()))
+      .path;
+  }
+}
+
+/**
+ * Wait until the Python extensions select `python.defaultInterpreterPath`.
+ * Right after they start, they can select the machine's interpreter instead,
+ * and then the client's settings do not name the virtual environment.
+ * The paths are compared as `fsPath`s, because on Windows the Python
+ * extensions report the drive letter in lower case.
+ */
+export async function waitForConfiguredInterpreter(): Promise<void> {
+  const expected = vscode.Uri.file(
+    vscode.workspace
+      .getConfiguration('python', fixtureFolder())
+      .get<string>('defaultInterpreterPath')!,
+  ).fsPath;
+  let actual: string | undefined;
+  await waitFor(
+    'the Python extensions to select python.defaultInterpreterPath',
+    async () => {
+      actual = await selectedInterpreter();
+      return actual !== undefined && vscode.Uri.file(actual).fsPath === expected;
+    },
+    30000,
+  ).catch(error => {
+    throw new Error(`${error.message}; they selected ${actual}`);
+  });
+}
+
+/**
+ * Activate the extension and open `main.py`. The extension starts the server
+ * on activation, so the shim's mode decides whether the editor's requests
+ * reach the server before the client's settings do.
+ */
+export async function openMainPy(): Promise<vscode.TextDocument> {
+  await vscode.extensions.getExtension('meta.pyrefly')!.activate();
+  const document = await vscode.workspace.openTextDocument(mainPy());
+  await vscode.window.showTextDocument(document);
+  return document;
+}
+
+export async function definition(
+  uri: vscode.Uri,
+  position: vscode.Position,
+): Promise<vscode.Uri[]> {
+  const locations = await vscode.commands.executeCommand<
+    (vscode.Location | vscode.LocationLink)[]
+  >('vscode.executeDefinitionProvider', uri, position);
+  return locations.map(location =>
+    'targetUri' in location ? location.targetUri : location.uri,
+  );
+}
+
+/** Every set of Pyrefly diagnostics that VS Code reported for `uri`. */
+export function recordDiagnostics(uri: vscode.Uri): {
+  sets: vscode.Diagnostic[][];
+  dispose: () => void;
+} {
+  const sets: vscode.Diagnostic[][] = [];
+  const subscription = vscode.languages.onDidChangeDiagnostics(event => {
+    if (event.uris.some(changed => changed.toString() === uri.toString())) {
+      sets.push(
+        vscode.languages
+          .getDiagnostics(uri)
+          .filter(diagnostic => diagnostic.source === 'Pyrefly'),
+      );
+    }
+  });
+  return {sets, dispose: () => subscription.dispose()};
+}
+
+export function diagnosticCode(diagnostic: vscode.Diagnostic): string {
+  const code = diagnostic.code;
+  return String(typeof code === 'object' ? code.value : code);
+}
+
+export async function waitFor(
+  description: string,
+  condition: () => boolean | Promise<boolean>,
+  timeoutMs: number,
+): Promise<void> {
+  const deadline = Date.now() + timeoutMs;
+  while (!(await condition())) {
+    if (Date.now() > deadline) {
+      throw new Error(`timed out waiting for ${description}`);
+    }
+    await new Promise(resolve => setTimeout(resolve, 100));
+  }
+}
+
+/**
+ * Open `main.py` and check that startup go-to-definition and every diagnostic
+ * set that the editor shows use the workspace settings. Only the interpreter
+ * from the Python extension can import `fakepkg`, and only the workspace
+ * settings select the preset that reports `bad-assignment`. Returns the client
+ * messages in the order that the shim forwarded them.
+ */
+export async function expectWorkspaceSettingsAtStartup(): Promise<
+  (string | undefined)[]
+> {
+  const diagnostics = recordDiagnostics(mainPy());
+  try {
+    await waitForConfiguredInterpreter();
+    await openMainPy();
+    const targets = await definition(mainPy(), new vscode.Position(0, 8));
+
+    let log: (string | undefined)[] = [];
+    await waitFor(
+      'the shim to forward the configuration response',
+      async () => {
+        log = (await shimLog()).map(entry => entry.method);
+        return log.includes('workspace/configuration response');
+      },
+      30000,
+    );
+    assert.deepStrictEqual(
+      targets.map(target => target.path.split('/').slice(-2).join('/')),
+      ['fakepkg/__init__.py'],
+    );
+
+    await waitFor(
+      'the bad-assignment error in main.py',
+      () =>
+        diagnostics.sets.some(set =>
+          set.some(d => diagnosticCode(d) === 'bad-assignment'),
+        ),
+      30000,
+    ).catch(error => {
+      throw new Error(
+        `${error.message}; the editor showed ${JSON.stringify(
+          diagnostics.sets.map(set => set.map(diagnosticCode)),
+        )}`,
+      );
+    });
+    // Each set is what the editor showed at one moment. Default settings use
+    // the `basic` preset, which hides `bad-assignment`, and do not name the
+    // interpreter that can import `fakepkg`. A set from them is empty or
+    // reports `missing-import`.
+    assert.deepStrictEqual(
+      diagnostics.sets.map(set => set.map(diagnosticCode)),
+      diagnostics.sets.map(() => ['bad-assignment']),
+    );
+    return log;
+  } finally {
+    diagnostics.dispose();
+  }
+}

lsp/src/test/startup-settings/delay/delay.test.ts

@@ -0,0 +1,32 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import * as assert from 'assert';
+import * as vscode from 'vscode';
+import {expectWorkspaceSettingsAtStartup} from '../common';
+
+/**
+ * The shim holds the client's configuration responses for 3 seconds, as a slow
+ * Python extension does, so the editor's startup requests reach the server
+ * before the interpreter does. The server must hold those requests until the
+ * interpreter arrives.
+ */
+suite('startup with a late initial configuration response', () => {
+  test('the Python extension is installed', () => {
+    assert.ok(vscode.extensions.getExtension('ms-python.python'));
+  });
+
+  test('startup requests and diagnostics use the interpreter from the Python extension', async function () {
+    this.timeout(60000);
+    const log = await expectWorkspaceSettingsAtStartup();
+    assert.ok(
+      log.indexOf('textDocument/definition') <
+        log.indexOf('workspace/configuration response'),
+      `the definition request must reach the server before the settings: ${log}`,
+    );
+  });
+});

lsp/src/test/startup-settings/drop/drop.test.ts

@@ -0,0 +1,33 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import * as assert from 'assert';
+import * as vscode from 'vscode';
+import {definition, mainPy, openMainPy, shimLog, waitFor} from '../common';
+
+/**
+ * The shim never forwards the client's first configuration response. The
+ * server must still answer the editor's startup requests, with default
+ * settings, instead of holding them forever.
+ */
+suite('startup without an initial configuration response', () => {
+  test('a startup request is answered with default settings', async function () {
+    this.timeout(60000);
+    await openMainPy();
+    const targets = await definition(mainPy(), new vscode.Position(1, 17));
+
+    assert.deepStrictEqual(
+      targets.map(target => target.path.split('/').pop()),
+      ['bar.py'],
+    );
+    await waitFor(
+      'the shim to drop the first configuration response',
+      async () => (await shimLog()).some(entry => entry.dropped !== undefined),
+      30000,
+    );
+  });
+});

lsp/src/test/startup-settings/instant/instant.test.ts

@@ -0,0 +1,27 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import * as assert from 'assert';
+import * as vscode from 'vscode';
+import {expectWorkspaceSettingsAtStartup} from '../common';
+
+/**
+ * The shim forwards the client's configuration responses at once, as when the
+ * Python extension has the interpreter ready. The server must still answer
+ * startup requests with the workspace settings and never show diagnostics from
+ * default settings.
+ */
+suite('startup with an instant initial configuration response', () => {
+  test('the Python extension is installed', () => {
+    assert.ok(vscode.extensions.getExtension('ms-python.python'));
+  });
+
+  test('startup requests and diagnostics use the interpreter from the Python extension', async function () {
+    this.timeout(60000);
+    await expectWorkspaceSettingsAtStartup();
+  });
+});