Skip to content

Commit d4e767d

Browse files
dibarbetdibarbet
committed
Add diagnostics integration tests
1 parent c372c4d commit d4e767d

File tree

6 files changed

+454
-15
lines changed

6 files changed

+454
-15
lines changed

‎src/lsptoolshost/onAutoInsert.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,15 @@ export function registerOnAutoInsert(languageServer: RoslynLanguageServer) {
4040

4141
source.cancel();
4242
source = new vscode.CancellationTokenSource();
43-
await applyAutoInsertEdit(e, changeTrimmed, languageServer, source.token);
43+
try {
44+
await applyAutoInsertEdit(e, changeTrimmed, languageServer, source.token);
45+
} catch (e) {
46+
if (e instanceof vscode.CancellationError) {
47+
return;
48+
}
49+
50+
throw e;
51+
}
4452
}
4553
});
4654
}

‎src/lsptoolshost/restore.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,19 @@ async function chooseProjectAndRestore(
4141
languageServer: RoslynLanguageServer,
4242
restoreChannel: vscode.OutputChannel
4343
): Promise<void> {
44-
const projects = await languageServer.sendRequest0(
45-
RestorableProjects.type,
46-
new vscode.CancellationTokenSource().token
47-
);
44+
let projects: string[];
45+
try {
46+
projects = await languageServer.sendRequest0(
47+
RestorableProjects.type,
48+
new vscode.CancellationTokenSource().token
49+
);
50+
} catch (e) {
51+
if (e instanceof vscode.CancellationError) {
52+
return;
53+
}
54+
55+
throw e;
56+
}
4857

4958
const items = projects.map((p) => {
5059
const projectName = path.basename(p);

‎src/lsptoolshost/roslynLanguageServer.ts

Lines changed: 54 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
RAL,
2929
CancellationToken,
3030
RequestHandler,
31+
ResponseError,
3132
} from 'vscode-languageclient/node';
3233
import { PlatformInformation } from '../shared/platform';
3334
import { readConfigurations } from './configurationMiddleware';
@@ -270,8 +271,12 @@ export class RoslynLanguageServer {
270271
throw new Error('Tried to send request while server is not started.');
271272
}
272273

273-
const response = await this._languageClient.sendRequest(type, params, token);
274-
return response;
274+
try {
275+
const response = await this._languageClient.sendRequest(type, params, token);
276+
return response;
277+
} catch (e) {
278+
throw this.convertServerError(type.method, e);
279+
}
275280
}
276281

277282
/**
@@ -285,8 +290,12 @@ export class RoslynLanguageServer {
285290
throw new Error('Tried to send request while server is not started.');
286291
}
287292

288-
const response = await this._languageClient.sendRequest(type, token);
289-
return response;
293+
try {
294+
const response = await this._languageClient.sendRequest(type, token);
295+
return response;
296+
} catch (e) {
297+
throw this.convertServerError(type.method, e);
298+
}
290299
}
291300

292301
public async sendRequestWithProgress<P extends PartialResultParams, R, PR, E, RO>(
@@ -302,10 +311,15 @@ export class RoslynLanguageServer {
302311
const disposable = this._languageClient.onProgress(type, partialResultToken, async (partialResult) => {
303312
await onProgress(partialResult);
304313
});
305-
const response = await this._languageClient
306-
.sendRequest(type, params, cancellationToken)
307-
.finally(() => disposable.dispose());
308-
return response;
314+
315+
try {
316+
const response = await this._languageClient.sendRequest(type, params, cancellationToken);
317+
return response;
318+
} catch (e) {
319+
throw this.convertServerError(type.method, e);
320+
} finally {
321+
disposable.dispose();
322+
}
309323
}
310324

311325
/**
@@ -368,6 +382,25 @@ export class RoslynLanguageServer {
368382
}
369383
}
370384

385+
private convertServerError(request: string, e: any): Error {
386+
let error: Error;
387+
if (e instanceof ResponseError && e.code === -32800) {
388+
// Convert the LSP RequestCancelled error (code -32800) to a CancellationError so we can handle cancellation uniformly.
389+
error = new vscode.CancellationError();
390+
} else if (e instanceof Error) {
391+
error = e;
392+
} else if (typeof e === 'string') {
393+
error = new Error(e);
394+
} else {
395+
error = new Error(`Unknown error: ${e.toString()}`);
396+
}
397+
398+
if (!(error instanceof vscode.CancellationError)) {
399+
_channel.appendLine(`Error making ${request} request: ${error.message}`);
400+
}
401+
return error;
402+
}
403+
371404
private async openDefaultSolutionOrProjects(): Promise<void> {
372405
// If Dev Kit isn't installed, then we are responsible for picking the solution to open, assuming the user hasn't explicitly
373406
// disabled it.
@@ -718,8 +751,19 @@ export class RoslynLanguageServer {
718751
// When a file is opened process any build diagnostics that may be shown
719752
this._languageClient.addDisposable(
720753
vscode.workspace.onDidOpenTextDocument(async (event) => {
721-
const buildIds = await this.getBuildOnlyDiagnosticIds(CancellationToken.None);
722-
this._buildDiagnosticService._onFileOpened(event, buildIds);
754+
try {
755+
const buildIds = await this.getBuildOnlyDiagnosticIds(CancellationToken.None);
756+
this._buildDiagnosticService._onFileOpened(event, buildIds);
757+
} catch (e) {
758+
if (e instanceof vscode.CancellationError) {
759+
// The request was cancelled (not due to us) - this means the server is no longer accepting requests
760+
// so there is nothing for us to do here.
761+
return;
762+
}
763+
764+
// Let non-cancellation errors bubble up.
765+
throw e;
766+
}
723767
})
724768
);
725769
}

‎test/integrationTests/diagnosticsHelpers.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
import * as vscode from 'vscode';
6+
import { expect } from '@jest/globals';
7+
import { restartLanguageServer } from './integrationHelpers';
8+
9+
function sortDiagnostics(diagnostics: vscode.Diagnostic[]): vscode.Diagnostic[] {
10+
return diagnostics.sort((a, b) => {
11+
const rangeCompare = a.range.start.compareTo(b.range.start);
12+
if (rangeCompare !== 0) {
13+
return rangeCompare;
14+
}
15+
16+
return getCode(a).localeCompare(getCode(b));
17+
});
18+
}
19+
20+
export async function waitForExpectedDiagnostics(
21+
assertExpectedDiagnostics: (input: [vscode.Uri, vscode.Diagnostic[]][]) => void,
22+
file?: vscode.Uri
23+
): Promise<void> {
24+
let duration = 30 * 1000;
25+
const step = 500;
26+
let error: any = undefined;
27+
while (duration > 0) {
28+
const diagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [];
29+
if (file) {
30+
diagnostics.push([file, sortDiagnostics(vscode.languages.getDiagnostics(file))]);
31+
} else {
32+
const allDiagnostics = vscode.languages.getDiagnostics();
33+
for (const [uri, uriDiagnostics] of allDiagnostics) {
34+
diagnostics.push([uri, sortDiagnostics(uriDiagnostics)]);
35+
}
36+
}
37+
38+
try {
39+
assertExpectedDiagnostics(diagnostics);
40+
return;
41+
} catch (e) {
42+
error = e;
43+
// Wait for a bit and try again.
44+
await new Promise((r) => setTimeout(r, step));
45+
duration -= step;
46+
}
47+
}
48+
49+
throw new Error(`Polling did not succeed within the alotted duration: ${error}`);
50+
}
51+
52+
export async function setBackgroundAnalysisScopes(scopes: { compiler: string; analyzer: string }): Promise<void> {
53+
const dotnetConfig = vscode.workspace.getConfiguration('dotnet');
54+
await dotnetConfig.update('backgroundAnalysis.compilerDiagnosticsScope', scopes.compiler);
55+
await dotnetConfig.update('backgroundAnalysis.analyzerDiagnosticsScope', scopes.analyzer);
56+
57+
// Restart the language server to ensure diagnostics are reported with the correct configuration.
58+
// While in normal user scenarios it isn't necessary to restart the server to pickup diagnostic config changes,
59+
// we need to do it in integration tests for two reasons:
60+
// 1. We don't know when the server finishes processing the config change (its sent as a notification from client -> server).
61+
// 2. Even if we processed the config change, the VSCode API does not provide a way to re-request diagnostics after the config change.
62+
await restartLanguageServer();
63+
}
64+
65+
export function getCode(diagnostic: vscode.Diagnostic): string {
66+
const code: {
67+
value: string | number;
68+
target: vscode.Uri;
69+
} = diagnostic.code as any;
70+
expect(code).toBeDefined();
71+
return code.value.toString();
72+
}

‎test/integrationTests/documentDiagnostics.integration.test.ts

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import * as vscode from 'vscode';
7+
import { describe, test, beforeAll, afterAll, expect } from '@jest/globals';
8+
import testAssetWorkspace from './testAssets/testAssetWorkspace';
9+
import { AnalysisSetting } from '../../src/lsptoolshost/buildDiagnosticsService';
10+
import * as integrationHelpers from './integrationHelpers';
11+
import path = require('path');
12+
import { getCode, setBackgroundAnalysisScopes, waitForExpectedDiagnostics } from './diagnosticsHelpers';
13+
describe(`[${testAssetWorkspace.description}] Test diagnostics`, function () {
14+
beforeAll(async function () {
15+
await integrationHelpers.activateCSharpExtension();
16+
});
17+
18+
afterAll(async () => {
19+
await testAssetWorkspace.cleanupWorkspace();
20+
});
21+
22+
describe('Open document diagnostics', () => {
23+
let file: vscode.Uri;
24+
beforeAll(async () => {
25+
file = await integrationHelpers.openFileInWorkspaceAsync(path.join('src', 'app', 'diagnostics.cs'));
26+
});
27+
28+
test('Compiler and analyzer diagnostics reported for open file when set to OpenFiles', async () => {
29+
await setBackgroundAnalysisScopes({
30+
compiler: AnalysisSetting.OpenFiles,
31+
analyzer: AnalysisSetting.OpenFiles,
32+
});
33+
34+
await waitForExpectedFileDiagnostics((diagnostics) => {
35+
expect(diagnostics).toHaveLength(4);
36+
37+
expect(getCode(diagnostics[0])).toBe('IDE0005');
38+
expect(diagnostics[0].message).toBe('Using directive is unnecessary.');
39+
expect(diagnostics[0].range).toEqual(new vscode.Range(0, 0, 0, 16));
40+
expect(diagnostics[0].severity).toBe(vscode.DiagnosticSeverity.Hint);
41+
42+
expect(getCode(diagnostics[1])).toBe('CA1822');
43+
expect(diagnostics[1].message).toBe(
44+
"Member 'FooBarBar' does not access instance data and can be marked as static"
45+
);
46+
expect(diagnostics[1].range).toEqual(new vscode.Range(6, 20, 6, 29));
47+
expect(diagnostics[1].severity).toBe(vscode.DiagnosticSeverity.Hint);
48+
49+
expect(getCode(diagnostics[2])).toBe('CS0219');
50+
expect(diagnostics[2].message).toBe("The variable 'notUsed' is assigned but its value is never used");
51+
expect(diagnostics[2].range).toEqual(new vscode.Range(8, 16, 8, 23));
52+
expect(diagnostics[2].severity).toBe(vscode.DiagnosticSeverity.Warning);
53+
54+
expect(getCode(diagnostics[3])).toBe('IDE0059');
55+
expect(diagnostics[3].message).toBe("Unnecessary assignment of a value to 'notUsed'");
56+
expect(diagnostics[3].range).toEqual(new vscode.Range(8, 16, 8, 23));
57+
expect(diagnostics[3].severity).toBe(vscode.DiagnosticSeverity.Hint);
58+
}, file);
59+
});
60+
61+
test('Compiler diagnostics reported for open file when set to FullSolution', async () => {
62+
await setBackgroundAnalysisScopes({
63+
compiler: AnalysisSetting.FullSolution,
64+
analyzer: AnalysisSetting.OpenFiles,
65+
});
66+
67+
await waitForExpectedFileDiagnostics((diagnostics) => {
68+
expect(diagnostics).toHaveLength(4);
69+
70+
expect(getCode(diagnostics[2])).toBe('CS0219');
71+
expect(diagnostics[2].message).toBe("The variable 'notUsed' is assigned but its value is never used");
72+
expect(diagnostics[2].range).toEqual(new vscode.Range(8, 16, 8, 23));
73+
expect(diagnostics[2].severity).toBe(vscode.DiagnosticSeverity.Warning);
74+
}, file);
75+
});
76+
77+
test('No compiler diagnostics reported for open file when set to None', async () => {
78+
await setBackgroundAnalysisScopes({
79+
compiler: AnalysisSetting.None,
80+
analyzer: AnalysisSetting.OpenFiles,
81+
});
82+
83+
await integrationHelpers.restartLanguageServer();
84+
85+
await waitForExpectedFileDiagnostics((diagnostics) => {
86+
expect(diagnostics).toHaveLength(3);
87+
88+
expect(diagnostics.some((d) => getCode(d).startsWith('CS'))).toBe(false);
89+
}, file);
90+
});
91+
92+
test('Analyzer diagnostics reported for open file when set to FullSolution', async () => {
93+
await setBackgroundAnalysisScopes({
94+
compiler: AnalysisSetting.OpenFiles,
95+
analyzer: AnalysisSetting.FullSolution,
96+
});
97+
98+
await waitForExpectedFileDiagnostics((diagnostics) => {
99+
expect(diagnostics).toHaveLength(4);
100+
101+
expect(getCode(diagnostics[0])).toBe('IDE0005');
102+
expect(diagnostics[0].message).toBe('Using directive is unnecessary.');
103+
expect(diagnostics[0].range).toEqual(new vscode.Range(0, 0, 0, 16));
104+
expect(diagnostics[0].severity).toBe(vscode.DiagnosticSeverity.Hint);
105+
106+
expect(getCode(diagnostics[1])).toBe('CA1822');
107+
expect(diagnostics[1].message).toBe(
108+
"Member 'FooBarBar' does not access instance data and can be marked as static"
109+
);
110+
expect(diagnostics[1].range).toEqual(new vscode.Range(6, 20, 6, 29));
111+
expect(diagnostics[1].severity).toBe(vscode.DiagnosticSeverity.Hint);
112+
113+
expect(getCode(diagnostics[3])).toBe('IDE0059');
114+
expect(diagnostics[3].message).toBe("Unnecessary assignment of a value to 'notUsed'");
115+
expect(diagnostics[3].range).toEqual(new vscode.Range(8, 16, 8, 23));
116+
expect(diagnostics[3].severity).toBe(vscode.DiagnosticSeverity.Hint);
117+
}, file);
118+
});
119+
120+
test('No analyzer diagnostics reported for open file when set to None', async () => {
121+
await setBackgroundAnalysisScopes({
122+
compiler: AnalysisSetting.OpenFiles,
123+
analyzer: AnalysisSetting.None,
124+
});
125+
126+
await waitForExpectedFileDiagnostics((diagnostics) => {
127+
expect(diagnostics).toHaveLength(1);
128+
129+
expect(diagnostics.some((d) => getCode(d).startsWith('IDE') || getCode(d).startsWith('CA'))).toBe(
130+
false
131+
);
132+
}, file);
133+
});
134+
});
135+
});
136+
137+
async function waitForExpectedFileDiagnostics(
138+
assertExpectedDiagnostics: (input: vscode.Diagnostic[]) => void,
139+
file: vscode.Uri
140+
): Promise<void> {
141+
return waitForExpectedDiagnostics((diagnostics) => {
142+
const fileDiagnostics = diagnostics.find((d) => d[0].toString() === file.toString());
143+
expect(fileDiagnostics).toBeDefined();
144+
145+
return assertExpectedDiagnostics(fileDiagnostics![1]);
146+
});
147+
}

0 commit comments

Comments
 (0)