Skip to content

Commit 02bda8d

Browse files
dibarbetdibarbet
committed
Refactor options so that the provider *always* returns the latest option values
1 parent f8fd0c9 commit 02bda8d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+860
-1166
lines changed

‎omnisharptest/omnisharpJestTests/optionChangeObserver.test.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
import { timeout } from 'rxjs/operators';
77
import { from as observableFrom, Subject, BehaviorSubject } from 'rxjs';
8-
import { Options } from '../../src/shared/options';
98
import { registerOmnisharpOptionChanges } from '../../src/omnisharp/omnisharpOptionChanges';
109

1110
import * as jestLib from '@jest/globals';
@@ -19,11 +18,11 @@ jestLib.describe('OmniSharpConfigChangeObserver', () => {
1918
let commandDone: Promise<void> | undefined;
2019
let infoMessage: string | undefined;
2120
let invokedCommand: string | undefined;
22-
let optionObservable: Subject<Options>;
21+
let optionObservable: Subject<void>;
2322

2423
jestLib.beforeEach(() => {
2524
resetMocks();
26-
optionObservable = new BehaviorSubject<Options>(Options.Read(vscode));
25+
optionObservable = new BehaviorSubject<void>(undefined);
2726
infoMessage = undefined;
2827
invokedCommand = undefined;
2928
commandDone = new Promise<void>((resolve) => {
@@ -46,7 +45,7 @@ jestLib.describe('OmniSharpConfigChangeObserver', () => {
4645
jestLib.expect(infoMessage).toBe(undefined);
4746
jestLib.expect(invokedCommand).toBe(undefined);
4847
updateConfig(vscode, elem.config, elem.section, elem.value);
49-
optionObservable.next(Options.Read(vscode));
48+
optionObservable.next();
5049
});
5150

5251
jestLib.test(`The information message is shown`, async () => {
@@ -85,7 +84,7 @@ jestLib.describe('OmniSharpConfigChangeObserver', () => {
8584
jestLib.expect(infoMessage).toBe(undefined);
8685
jestLib.expect(invokedCommand).toBe(undefined);
8786
updateConfig(vscode, elem.config, elem.section, elem.value);
88-
optionObservable.next(Options.Read(vscode));
87+
optionObservable.next();
8988
});
9089

9190
jestLib.test(`The information message is shown`, async () => {
@@ -132,7 +131,7 @@ jestLib.describe('OmniSharpConfigChangeObserver', () => {
132131
jestLib.expect(infoMessage).toBe(undefined);
133132
jestLib.expect(invokedCommand).toBe(undefined);
134133
updateConfig(vscode, elem.config, elem.section, elem.value);
135-
optionObservable.next(Options.Read(vscode));
134+
optionObservable.next();
136135
jestLib.expect(infoMessage).toBe(undefined);
137136
});
138137
});

‎omnisharptest/omnisharpUnitTests/fakes/fakeOptions.ts

Lines changed: 0 additions & 76 deletions
This file was deleted.

‎omnisharptest/omnisharpUnitTests/features/reportIssue.test.ts

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,9 @@
66
import reportIssue from '../../../src/shared/reportIssue';
77
import { expect } from 'chai';
88
import { vscode } from '../../../src/vscodeAdapter';
9-
import { Options } from '../../../src/shared/options';
109
import { FakeMonoResolver, fakeMonoInfo } from '../fakes/fakeMonoResolver';
1110
import { FakeDotnetResolver } from '../fakes/fakeDotnetResolver';
1211
import { DotnetInfo } from '../../../src/shared/utils/dotnetInfo';
13-
import { getEmptyOptions } from '../fakes/fakeOptions';
1412
import { getFakeVsCode } from '../../../test/unitTests/fakes';
1513

1614
suite(`${reportIssue.name}`, () => {
@@ -50,7 +48,6 @@ suite(`${reportIssue.name}`, () => {
5048
let fakeMonoResolver: FakeMonoResolver;
5149
let fakeDotnetResolver: FakeDotnetResolver;
5250
const getDotnetInfo = async () => Promise.resolve(fakeDotnetInfo);
53-
let options: Options;
5451
let issueBody: string;
5552

5653
setup(() => {
@@ -64,7 +61,6 @@ suite(`${reportIssue.name}`, () => {
6461
vscode.extensions.all = [extension1, extension2];
6562
fakeMonoResolver = new FakeMonoResolver();
6663
fakeDotnetResolver = new FakeDotnetResolver();
67-
options = getEmptyOptions();
6864
});
6965

7066
suite('The body is passed to the vscode clipboard and', () => {
@@ -74,7 +70,6 @@ suite(`${reportIssue.name}`, () => {
7470
csharpExtVersion,
7571
getDotnetInfo,
7672
isValidForMono,
77-
options,
7873
fakeDotnetResolver,
7974
fakeMonoResolver
8075
);
@@ -87,7 +82,6 @@ suite(`${reportIssue.name}`, () => {
8782
csharpExtVersion,
8883
getDotnetInfo,
8984
isValidForMono,
90-
options,
9185
fakeDotnetResolver,
9286
fakeMonoResolver
9387
);
@@ -100,7 +94,6 @@ suite(`${reportIssue.name}`, () => {
10094
csharpExtVersion,
10195
getDotnetInfo,
10296
isValidForMono,
103-
options,
10497
fakeDotnetResolver,
10598
fakeMonoResolver
10699
);
@@ -113,7 +106,6 @@ suite(`${reportIssue.name}`, () => {
113106
csharpExtVersion,
114107
getDotnetInfo,
115108
isValidForMono,
116-
options,
117109
fakeDotnetResolver,
118110
fakeMonoResolver
119111
);
@@ -126,7 +118,6 @@ suite(`${reportIssue.name}`, () => {
126118
csharpExtVersion,
127119
getDotnetInfo,
128120
isValidForMono,
129-
options,
130121
fakeDotnetResolver,
131122
fakeMonoResolver
132123
);
@@ -135,15 +126,7 @@ suite(`${reportIssue.name}`, () => {
135126
});
136127

137128
test('mono information is not obtained when it is not a valid mono platform', async () => {
138-
await reportIssue(
139-
vscode,
140-
csharpExtVersion,
141-
getDotnetInfo,
142-
false,
143-
options,
144-
fakeDotnetResolver,
145-
fakeMonoResolver
146-
);
129+
await reportIssue(vscode, csharpExtVersion, getDotnetInfo, false, fakeDotnetResolver, fakeMonoResolver);
147130
expect(fakeMonoResolver.getMonoCalled).to.be.equal(false);
148131
});
149132

@@ -153,7 +136,6 @@ suite(`${reportIssue.name}`, () => {
153136
csharpExtVersion,
154137
getDotnetInfo,
155138
isValidForMono,
156-
options,
157139
fakeDotnetResolver,
158140
fakeMonoResolver
159141
);

‎omnisharptest/omnisharpUnitTests/logging/informationMessageObserver.test.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,10 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { InformationMessageObserver } from '../../../src/observers/informationMessageObserver';
7-
import OptionProvider from '../../../src/shared/observers/optionProvider';
87
import { expect, should } from 'chai';
98
import { getUnresolvedDependenices, updateConfig, getVSCodeWithConfig } from '../../../test/unitTests/fakes';
109
import { Subject, from as observableFrom } from 'rxjs';
1110
import { timeout } from 'rxjs/operators';
12-
import { Options } from '../../../src/shared/options';
1311

1412
suite('InformationMessageObserver', () => {
1513
suiteSetup(() => should());
@@ -19,11 +17,10 @@ suite('InformationMessageObserver', () => {
1917
let signalCommandDone: () => void;
2018
let commandDone: Promise<void> | undefined;
2119
const vscode = getVsCode();
22-
const optionObservable = new Subject<Options>();
23-
const optionProvider = new OptionProvider(optionObservable);
20+
const optionObservable = new Subject<void>();
2421
let infoMessage: string | undefined;
2522
let invokedCommand: string | undefined;
26-
const observer: InformationMessageObserver = new InformationMessageObserver(vscode, optionProvider);
23+
const observer: InformationMessageObserver = new InformationMessageObserver(vscode);
2724

2825
setup(() => {
2926
infoMessage = undefined;
@@ -45,7 +42,7 @@ suite('InformationMessageObserver', () => {
4542
suite('Suppress Dotnet Restore Notification is true', () => {
4643
setup(() => {
4744
updateConfig(vscode, 'csharp', 'suppressDotnetRestoreNotification', true);
48-
optionObservable.next(Options.Read(vscode));
45+
optionObservable.next();
4946
});
5047

5148
test('The information message is not shown', () => {
@@ -57,7 +54,7 @@ suite('InformationMessageObserver', () => {
5754
suite('Suppress Dotnet Restore Notification is false', () => {
5855
setup(() => {
5956
updateConfig(vscode, 'csharp', 'suppressDotnetRestoreNotification', false);
60-
optionObservable.next(Options.Read(vscode));
57+
optionObservable.next();
6158
});
6259

6360
test('The information message is shown', async () => {

‎omnisharptest/omnisharpUnitTests/logging/omnisharpChannelObserver.test.ts

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,40 +13,34 @@ import {
1313
OmnisharpRestart,
1414
OmnisharpServerOnStdErr,
1515
} from '../../../src/omnisharp/loggingEvents';
16-
import OptionProvider from '../../../src/shared/observers/optionProvider';
1716
import { Subject } from 'rxjs';
18-
import { Options } from '../../../src/shared/options';
1917

2018
suite('OmnisharpChannelObserver', () => {
2119
let hasShown: boolean;
2220
let hasCleared: boolean;
2321
let preserveFocus: boolean | undefined;
2422
let vscode: vscode;
25-
const optionObservable = new Subject<Options>();
26-
const optionProvider = new OptionProvider(optionObservable);
23+
const optionObservable = new Subject<void>();
2724
let observer: OmnisharpChannelObserver;
2825

2926
setup(() => {
3027
hasShown = false;
3128
hasCleared = false;
3229
preserveFocus = false;
3330
vscode = getVSCodeWithConfig();
34-
observer = new OmnisharpChannelObserver(
35-
{
36-
...getNullChannel(),
37-
show: (preserve) => {
38-
hasShown = true;
39-
preserveFocus = preserve;
40-
},
41-
clear: () => {
42-
hasCleared = true;
43-
},
31+
observer = new OmnisharpChannelObserver({
32+
...getNullChannel(),
33+
show: (preserve) => {
34+
hasShown = true;
35+
preserveFocus = preserve;
4436
},
45-
optionProvider
46-
);
37+
clear: () => {
38+
hasCleared = true;
39+
},
40+
});
4741

4842
updateConfig(vscode, 'csharp', 'showOmnisharpLogOnError', true);
49-
optionObservable.next(Options.Read(vscode));
43+
optionObservable.next();
5044
});
5145

5246
[
@@ -64,7 +58,7 @@ suite('OmnisharpChannelObserver', () => {
6458

6559
test(`OmnisharpServerOnStdErr: Channel is not shown when disabled in configuration`, () => {
6660
updateConfig(vscode, 'csharp', 'showOmnisharpLogOnError', false);
67-
optionObservable.next(Options.Read(vscode));
61+
optionObservable.next();
6862

6963
expect(hasShown).to.be.false;
7064
observer.post(new OmnisharpServerOnStdErr('std err'));

‎omnisharptest/omnisharpUnitTests/omnisharp/omniSharpMonoResolver.test.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,11 @@
55

66
import { OmniSharpMonoResolver } from '../../../src/omnisharp/omniSharpMonoResolver';
77

8-
import { Options } from '../../../src/shared/options';
98
import { expect } from 'chai';
109
import { join } from 'path';
11-
import { getEmptyOptions } from '../fakes/fakeOptions';
1210

1311
suite(`${OmniSharpMonoResolver.name}`, () => {
1412
let getMonoCalled: boolean;
15-
let options: Options;
1613

1714
const monoPath = 'monoPath';
1815

@@ -27,15 +24,14 @@ suite(`${OmniSharpMonoResolver.name}`, () => {
2724

2825
setup(() => {
2926
getMonoCalled = false;
30-
options = getEmptyOptions();
3127
});
3228

3329
test(`it returns the path and version if the version is greater than or equal to ${requiredMonoVersion}`, async () => {
3430
const monoResolver = new OmniSharpMonoResolver(getMono(higherMonoVersion));
35-
const monoInfo = await monoResolver.getHostExecutableInfo({
31+
const monoInfo = await monoResolver.getHostExecutableInfo(/*{
3632
...options,
3733
omnisharpOptions: { ...options.omnisharpOptions, monoPath: monoPath },
38-
});
34+
}*/);
3935

4036
expect(monoInfo.version).to.be.equal(higherMonoVersion);
4137
expect(monoInfo.path).to.be.equal(monoPath);
@@ -45,19 +41,19 @@ suite(`${OmniSharpMonoResolver.name}`, () => {
4541
const monoResolver = new OmniSharpMonoResolver(getMono(lowerMonoVersion));
4642

4743
await expect(
48-
monoResolver.getHostExecutableInfo({
44+
monoResolver.getHostExecutableInfo(/*{
4945
...options,
5046
omnisharpOptions: { ...options.omnisharpOptions, monoPath: monoPath },
51-
})
47+
}*/)
5248
).to.be.rejected;
5349
});
5450

5551
test('sets the environment with the monoPath', async () => {
5652
const monoResolver = new OmniSharpMonoResolver(getMono(requiredMonoVersion));
57-
const monoInfo = await monoResolver.getHostExecutableInfo({
53+
const monoInfo = await monoResolver.getHostExecutableInfo(/*{
5854
...options,
5955
omnisharpOptions: { ...options.omnisharpOptions, monoPath: monoPath },
60-
});
56+
}*/);
6157

6258
expect(getMonoCalled).to.be.equal(true);
6359
expect(monoInfo.env['PATH']).to.contain(join(monoPath, 'bin'));

0 commit comments

Comments
 (0)