Skip to content

Commit 36f342b

Browse files
dibarbetdibarbet
committed
Review feedback
1 parent ed2899a commit 36f342b

15 files changed

+87
-51
lines changed

‎test/featureTests/assets.test.ts‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import * as jsonc from 'jsonc-parser';
1212
import { AssetGenerator, ProgramLaunchType, replaceCommentPropertiesWithComments, updateJsonWithComments } from '../../src/assets';
1313
import { parse } from 'jsonc-parser';
1414
import { use as chaiUse, should } from 'chai';
15+
import { isNotNull } from '../testUtil';
1516

1617
chaiUse(require('chai-string'));
1718

@@ -24,7 +25,9 @@ suite("Asset generation: csproj", () => {
2425
let generator = new AssetGenerator(info, createMockWorkspaceFolder(rootPath));
2526
generator.setStartupProject(0);
2627
let tasksJson = generator.createTasksConfiguration();
27-
let buildPath = tasksJson.tasks![0].args![1];
28+
isNotNull(tasksJson.tasks);
29+
isNotNull(tasksJson.tasks[0].args);
30+
let buildPath = tasksJson.tasks[0].args[1];
2831

2932
// ${workspaceFolder}/project.json
3033
let segments = buildPath.split(path.posix.sep);
@@ -37,9 +40,10 @@ suite("Asset generation: csproj", () => {
3740
let generator = new AssetGenerator(info, createMockWorkspaceFolder(rootPath));
3841
generator.setStartupProject(0);
3942
let tasksJson = generator.createTasksConfiguration();
43+
isNotNull(tasksJson.tasks);
4044

4145
// We do not check the watch task since this parameter can break hot reload scenarios.
42-
tasksJson.tasks!
46+
tasksJson.tasks
4347
.filter(task => task.label !== "watch")
4448
.forEach(task => task.args!.should.contain("/property:GenerateFullPaths=true"));
4549
});
@@ -50,9 +54,10 @@ suite("Asset generation: csproj", () => {
5054
let generator = new AssetGenerator(info, createMockWorkspaceFolder(rootPath));
5155
generator.setStartupProject(0);
5256
let tasksJson = generator.createTasksConfiguration();
57+
isNotNull(tasksJson.tasks);
5358

5459
// We do not check the watch task since this parameter can break hot reload scenarios.
55-
tasksJson.tasks!
60+
tasksJson.tasks
5661
.filter(task => task.label !== "watch")
5762
.forEach(task => task.args!.should.contain("/consoleloggerparameters:NoSummary"));
5863
});
@@ -63,9 +68,11 @@ suite("Asset generation: csproj", () => {
6368
let generator = new AssetGenerator(info, createMockWorkspaceFolder(rootPath));
6469
generator.setStartupProject(0);
6570
let tasksJson = generator.createTasksConfiguration();
71+
isNotNull(tasksJson.tasks);
6672

67-
const watchTask = tasksJson.tasks!.find(task => task.label === "watch");
68-
watchTask!.args!.should.not.contain("/property:GenerateFullPaths=true");
73+
const watchTask = tasksJson.tasks.find(task => task.label === "watch");
74+
isNotNull(watchTask?.args);
75+
watchTask.args.should.not.contain("/property:GenerateFullPaths=true");
6976
});
7077

7178
test("Generated 'watch' task does not have the consoleloggerparameters argument set to NoSummary", () => {
@@ -76,7 +83,8 @@ suite("Asset generation: csproj", () => {
7683
let tasksJson = generator.createTasksConfiguration();
7784

7885
const watchTask = tasksJson.tasks!.find(task => task.label === "watch");
79-
watchTask!.args!.should.not.contain("/consoleloggerparameters:NoSummary");
86+
isNotNull(watchTask?.args);
87+
watchTask.args.should.not.contain("/consoleloggerparameters:NoSummary");
8088
});
8189

8290
test("Create tasks.json for nested project opened in workspace", () => {
@@ -85,7 +93,9 @@ suite("Asset generation: csproj", () => {
8593
let generator = new AssetGenerator(info, createMockWorkspaceFolder(rootPath));
8694
generator.setStartupProject(0);
8795
let tasksJson = generator.createTasksConfiguration();
88-
let buildPath = tasksJson.tasks![0].args![1];
96+
isNotNull(tasksJson.tasks);
97+
isNotNull(tasksJson.tasks[0].args);
98+
let buildPath = tasksJson.tasks[0].args[1];
8999

90100
// ${workspaceFolder}/nested/project.json
91101
let segments = buildPath.split(path.posix.sep);

‎test/integrationTests/codeActionRename.integration.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { activateCSharpExtension, isRazorWorkspace, isSlnWithGenerator } from '.
1010
import testAssetWorkspace from './testAssets/testAssetWorkspace';
1111
import * as path from 'path';
1212
import { assertWithPoll } from './poll';
13+
import { isNotNull } from '../testUtil';
1314

1415
const chai = require('chai');
1516
chai.use(require('chai-arrays'));
@@ -43,8 +44,10 @@ suite(`Code Action Rename ${testAssetWorkspace.description}`, function () {
4344
const codeActions = await vscode.commands.executeCommand<vscode.CodeAction[]>("vscode.executeCodeActionProvider", fileUri, new vscode.Range(0, 7, 0, 7));
4445
const codeAction = codeActions.find(codeAction => codeAction.title == "Rename file to C.cs");
4546
expect(codeAction, "Didn't find rename class code action");
47+
isNotNull(codeAction?.command?.command);
48+
isNotNull(codeAction?.command?.arguments);
4649

47-
await vscode.commands.executeCommand(codeAction!.command!.command, ...codeAction!.command!.arguments!);
50+
await vscode.commands.executeCommand(codeAction.command.command, ...codeAction.command.arguments);
4851

4952
await assertWithPoll(() => { }, 15 * 1000, 500, _ => expect(vscode.window.activeTextEditor!.document.fileName).contains("C.cs"));
5053
});

‎test/integrationTests/codeLensProvider.integration.test.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import * as path from 'path';
99
import { should, expect } from 'chai';
1010
import { activateCSharpExtension, isSlnWithCsproj, isSlnWithGenerator } from './integrationHelpers';
1111
import testAssetWorkspace from './testAssets/testAssetWorkspace';
12+
import { isNotNull } from '../testUtil';
1213

1314
const chai = require('chai');
1415
chai.use(require('chai-arrays'));
@@ -60,8 +61,8 @@ suite(`CodeLensProvider: ${testAssetWorkspace.description}`, function () {
6061

6162
for (let codeLens of codeLenses) {
6263
expect(codeLens.isResolved).to.be.true;
63-
expect(codeLens.command).not.to.be.undefined;
64-
expect(codeLens.command!.title).to.equal("0 references");
64+
isNotNull(codeLens.command);
65+
expect(codeLens.command.title).to.equal("0 references");
6566
}
6667
});
6768
});
@@ -104,9 +105,9 @@ suite(`CodeLensProvider options: ${testAssetWorkspace.description}`, function ()
104105

105106
for (let codeLens of codeLenses) {
106107
expect(codeLens.isResolved).to.be.true;
107-
expect(codeLens.command).not.to.be.undefined;
108-
expect(codeLens.command!.command).to.be.oneOf(['dotnet.test.run', 'dotnet.classTests.run', 'dotnet.test.debug', 'dotnet.classTests.debug']);
109-
expect(codeLens.command!.title).to.be.oneOf(['Run Test', 'Run All Tests', 'Debug Test', 'Debug All Tests']);
108+
isNotNull(codeLens.command);
109+
expect(codeLens.command.command).to.be.oneOf(['dotnet.test.run', 'dotnet.classTests.run', 'dotnet.test.debug', 'dotnet.classTests.debug']);
110+
expect(codeLens.command.title).to.be.oneOf(['Run Test', 'Run All Tests', 'Debug Test', 'Debug All Tests']);
110111
}
111112
});
112113

@@ -120,9 +121,9 @@ suite(`CodeLensProvider options: ${testAssetWorkspace.description}`, function ()
120121

121122
for (let codeLens of codeLenses) {
122123
expect(codeLens.isResolved).to.be.true;
123-
expect(codeLens.command).not.to.be.undefined;
124-
expect(codeLens.command!.command).to.be.equal('editor.action.showReferences');
125-
expect(codeLens.command!.title).to.equal('0 references');
124+
isNotNull(codeLens.command);
125+
expect(codeLens.command.command).to.be.equal('editor.action.showReferences');
126+
expect(codeLens.command.title).to.equal('0 references');
126127
}
127128
});
128129

‎test/integrationTests/diagnostics.integration.test.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { should, expect } from 'chai';
1010
import { activateCSharpExtension, isRazorWorkspace, isSlnWithGenerator, restartOmniSharpServer } from './integrationHelpers';
1111
import testAssetWorkspace from './testAssets/testAssetWorkspace';
1212
import { poll, assertWithPoll, pollDoesNotHappen } from './poll';
13+
import { isNotNull } from '../testUtil';
1314

1415
const chai = require('chai');
1516
chai.use(require('chai-arrays'));
@@ -115,8 +116,8 @@ suite(`DiagnosticProvider: ${testAssetWorkspace.description}`, function () {
115116
result => result.find(x => x.code === "CS0219") != undefined);
116117

117118
let cs0219 = result.find(x => x.code === "CS0219");
118-
expect(cs0219).to.not.be.undefined;
119-
expect(cs0219!.tags).to.include(vscode.DiagnosticTag.Unnecessary);
119+
isNotNull(cs0219);
120+
expect(cs0219.tags).to.include(vscode.DiagnosticTag.Unnecessary);
120121
});
121122

122123
test("Return unnecessary tag in case of unnesessary using", async function () {
@@ -127,8 +128,8 @@ suite(`DiagnosticProvider: ${testAssetWorkspace.description}`, function () {
127128
result => result.find(x => x.code === "CS8019") != undefined);
128129

129130
let cs8019 = result.find(x => x.code === "CS8019");
130-
expect(cs8019).to.not.be.undefined;
131-
expect(cs8019!.tags).to.include(vscode.DiagnosticTag.Unnecessary);
131+
isNotNull(cs8019);
132+
expect(cs8019.tags).to.include(vscode.DiagnosticTag.Unnecessary);
132133
});
133134

134135
test("Return fadeout diagnostics like unused variables based on roslyn analyzers", async function () {
@@ -139,8 +140,8 @@ suite(`DiagnosticProvider: ${testAssetWorkspace.description}`, function () {
139140
result => result.find(x => x.code === "IDE0059") != undefined);
140141

141142
let ide0059 = result.find(x => x.code === "IDE0059");
142-
expect(ide0059).to.not.be.undefined;
143-
expect(ide0059!.tags).to.include(vscode.DiagnosticTag.Unnecessary);
143+
isNotNull(ide0059);
144+
expect(ide0059.tags).to.include(vscode.DiagnosticTag.Unnecessary);
144145
});
145146

146147
test("On small workspaces also show/fetch closed document analysis results", async function () {

‎test/integrationTests/dotnetTest.integration.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { EventStream } from '../../src/EventStream';
1313
import { EventType } from '../../src/omnisharp/EventType';
1414
import { OmnisharpRequestMessage } from '../../src/omnisharp/loggingEvents';
1515
import { V2 } from '../../src/omnisharp/protocol';
16+
import { isNotNull } from '../testUtil';
1617

1718
const chai = require('chai');
1819
chai.use(require('chai-arrays'));
@@ -95,7 +96,7 @@ suite(`DotnetTest: ${testAssetWorkspace.description}`, function () {
9596
const event = await eventWaiter;
9697
const runTestsRequest = <V2.RunTestsInContextRequest>event!.request.data;
9798

98-
expect(runTestsRequest.RunSettings).to.be.not.null;
99+
isNotNull(runTestsRequest.RunSettings);
99100
expect(runTestsRequest.RunSettings!.endsWith(endingPath), "Path includes relative path").to.be.true;
100101
expect(path.isAbsolute(runTestsRequest.RunSettings!), "Path is absolute").to.be.true;
101102
});

‎test/integrationTests/inlayHints.integration.test.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { activateCSharpExtension, isRazorWorkspace, isSlnWithGenerator, restartO
1010
import testAssetWorkspace from './testAssets/testAssetWorkspace';
1111
import * as path from 'path';
1212
import { InlayHint, LinePositionSpanTextChange } from '../../src/omnisharp/protocol';
13+
import { isNotNull } from '../testUtil';
1314

1415
const chai = require('chai');
1516
chai.use(require('chai-arrays'));
@@ -82,10 +83,11 @@ suite(`Inlay Hints ${testAssetWorkspace.description}`, function () {
8283
return;
8384
}
8485

85-
assert.equal(actual.textEdits.length, expected.TextEdits!.length);
86+
isNotNull(expected.TextEdits);
87+
assert.equal(actual.textEdits.length, expected.TextEdits.length);
8688
for (let i = 0; i < actual.textEdits.length; i++) {
8789
const actualTextEdit = actual.textEdits[i];
88-
const expectedTextEdit = expected.TextEdits![i];
90+
const expectedTextEdit = expected.TextEdits[i];
8991

9092
assertTextEditEqual(actualTextEdit, expectedTextEdit);
9193
}

‎test/integrationTests/launchConfiguration.integration.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { should, expect } from 'chai';
1010
import { activateCSharpExtension, isRazorWorkspace, isSlnWithGenerator } from './integrationHelpers';
1111
import testAssetWorkspace from './testAssets/testAssetWorkspace';
1212
import { poll } from './poll';
13+
import { isNotNull } from '../testUtil';
1314

1415
const chai = require('chai');
1516
chai.use(require('chai-arrays'));
@@ -40,8 +41,8 @@ suite(`Tasks generation: ${testAssetWorkspace.description}`, function () {
4041

4142
const onChangeSubscription = vscode.debug.onDidChangeActiveDebugSession((e) => {
4243
onChangeSubscription.dispose();
43-
expect(vscode.debug.activeDebugSession).not.to.be.undefined;
44-
expect(vscode.debug.activeDebugSession!.type).to.equal("coreclr");
44+
isNotNull(vscode.debug.activeDebugSession);
45+
expect(vscode.debug.activeDebugSession.type).to.equal("coreclr");
4546
});
4647

4748
let result = await vscode.debug.startDebugging(vscode.workspace.workspaceFolders![0], ".NET Core Launch (console)");

‎test/testUtil.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
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+
export function isNotNull<T>(value: T) : asserts value is NonNullable<T> {
7+
if (value === null || value === undefined) {
8+
throw new Error("value is null or undefined.");
9+
}
10+
}

‎test/unitTests/Fakes/FakeDotnetResolver.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { HostExecutableInformation } from "../../../src/constants/HostExecutable
99
export const fakeMonoInfo: HostExecutableInformation = {
1010
version: "someDotNetVersion",
1111
path: "someDotNetPath",
12-
env: undefined!
12+
env: { }
1313
};
1414

1515
export class FakeDotnetResolver implements IHostExecutableResolver {

‎test/unitTests/Fakes/FakeMonoResolver.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { HostExecutableInformation } from "../../../src/constants/HostExecutable
99
export const fakeMonoInfo: HostExecutableInformation = {
1010
version: "someMonoVersion",
1111
path: "somePath",
12-
env: undefined!
12+
env: { }
1313
};
1414

1515
export class FakeMonoResolver implements IHostExecutableResolver {

0 commit comments

Comments
 (0)