diff --git a/CHANGELOG.md b/CHANGELOG.md index e4bce59..5c0c071 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,3 +29,5 @@ - Stabilized TypeScript coverage by waiting for an end-to-end PythonTA diagnostic - Fixed `vsce-package` script failing under pnpm by passing `--no-dependencies`, since dependencies are bundled by webpack - Excluded development-only files (e.g., `.claude/`, `.mypy_cache/`, lockfiles, `scripts/`) from the packaged extension +- Stabilized TypeScript coverage by testing the language client stop-failure path directly +- Fixed flaky settings tests by waiting only for the configuration change each test makes diff --git a/src/common/server.ts b/src/common/server.ts index 49287c9..ce936e2 100644 --- a/src/common/server.ts +++ b/src/common/server.ts @@ -74,6 +74,23 @@ async function createServer( return new LanguageClient(serverId, serverName, serverOptions, clientOptions); } +/** + * Stops the given language client, if any. A failure to stop (e.g. the client is + * still starting, or stopping times out) is logged rather than thrown, so that a + * restart can proceed with a fresh client. + */ +export async function stopServer(lsClient?: LanguageClient): Promise { + if (!lsClient) { + return; + } + traceInfo(`Server: Stop requested`); + try { + await lsClient.stop(); + } catch (ex) { + traceError(`Server: Stop failed: ${ex}`); + } +} + let _disposables: Disposable[] = []; export async function restartServer( serverId: string, @@ -81,14 +98,7 @@ export async function restartServer( outputChannel: LogOutputChannel, lsClient?: LanguageClient, ): Promise { - if (lsClient) { - traceInfo(`Server: Stop requested`); - try { - await lsClient.stop(); - } catch (ex) { - traceError(`Server: Stop failed: ${ex}`); - } - } + await stopServer(lsClient); _disposables.forEach((d) => d.dispose()); _disposables = []; const projectRoot = await getProjectRoot(); diff --git a/src/test/suite/server.test.ts b/src/test/suite/server.test.ts new file mode 100644 index 0000000..4bdabb2 --- /dev/null +++ b/src/test/suite/server.test.ts @@ -0,0 +1,43 @@ +import * as assert from 'assert'; +import * as vscode from 'vscode'; +import { LanguageClient } from 'vscode-languageclient/node'; +import { stopServer } from '../../common/server'; + +/** Returns a stand-in LanguageClient whose `stop()` behaves as given, and a counter of calls to it. */ +function fakeClient(stop: () => Promise): { client: LanguageClient; stopCalls: () => number } { + let calls = 0; + const client = { + stop: () => { + calls += 1; + return stop(); + }, + } as unknown as LanguageClient; + return { client, stopCalls: () => calls }; +} + +suite('server.stopServer', () => { + suiteSetup(async () => { + // Activating the extension registers the output channel logger, so the + // failure path below logs through it just as it does at runtime. + await vscode.extensions.getExtension('david-yz-liu.python-ta')?.activate(); + }); + + test('does nothing when there is no client', async () => { + await stopServer(undefined); + }); + + test('stops the client', async () => { + const { client, stopCalls } = fakeClient(() => Promise.resolve()); + await stopServer(client); + assert.strictEqual(stopCalls(), 1); + }); + + test('logs and swallows errors when the client fails to stop', async () => { + // Mirrors vscode-languageclient's error when a restart races a client that is still starting. + const { client, stopCalls } = fakeClient(() => + Promise.reject(new Error("Client is not running and can't be stopped. It's current state is: starting")), + ); + await stopServer(client); + assert.strictEqual(stopCalls(), 1); + }); +}); diff --git a/src/test/suite/settings.test.ts b/src/test/suite/settings.test.ts index f4cf03e..3ac79db 100644 --- a/src/test/suite/settings.test.ts +++ b/src/test/suite/settings.test.ts @@ -4,6 +4,22 @@ import { checkIfConfigurationChanged, getGlobalSettings } from '../../common/set const NAMESPACE = 'python-ta'; +/** + * Resolves with the first configuration change event that affects `section`. + * Unrelated changes (e.g. other extensions updating settings while they activate) + * are ignored, so the test sees the event caused by its own update. + */ +function waitForConfigurationChange(section: string): Promise { + return new Promise((resolve) => { + const disposable = vscode.workspace.onDidChangeConfiguration((e) => { + if (e.affectsConfiguration(section)) { + disposable.dispose(); + resolve(e); + } + }); + }); +} + suite('settings.getGlobalSettings', () => { test('returns defaults matching package.json when nothing overridden', async () => { const settings = await getGlobalSettings(NAMESPACE, false); @@ -21,12 +37,7 @@ suite('settings.checkIfConfigurationChanged', () => { const config = vscode.workspace.getConfiguration(NAMESPACE); const original = config.get('args'); - const changed = new Promise((resolve) => { - const disposable = vscode.workspace.onDidChangeConfiguration((e) => { - disposable.dispose(); - resolve(e); - }); - }); + const changed = waitForConfigurationChange(`${NAMESPACE}.args`); await config.update('args', ['--foo'], vscode.ConfigurationTarget.Global); try { @@ -41,12 +52,7 @@ suite('settings.checkIfConfigurationChanged', () => { const config = vscode.workspace.getConfiguration('editor'); const original = config.get('wordWrap'); - const changed = new Promise((resolve) => { - const disposable = vscode.workspace.onDidChangeConfiguration((e) => { - disposable.dispose(); - resolve(e); - }); - }); + const changed = waitForConfigurationChange('editor.wordWrap'); await config.update('wordWrap', 'on', vscode.ConfigurationTarget.Global); try {