Code review bug

Author: Kartik Raj

src/client/common/insidersBuild/downloadChannelService.ts

@@ -27,14 +27,14 @@ export class ExtensionChannelService implements IExtensionChannelService {
         this.isThisFirstSessionState = this.persistentStateFactory.createGlobalPersistentState(isThisFirstSessionStateKey, true);
         disposables.push(this.workspaceService.onDidChangeConfiguration(this.onDidChangeConfiguration.bind(this)));
     }
-    public get channel(): ExtensionChannels {
+    public async getChannel(): Promise<ExtensionChannels> {
         const settings = this.workspaceService.getConfiguration('python').inspect<ExtensionChannels>(insidersChannelSetting);
         if (!settings) {
             throw new Error(`WorkspaceConfiguration.inspect returns 'undefined' for setting 'python.${insidersChannelSetting}'`);
         }
         if (settings.globalValue === undefined) {
             const isThisFirstSession = this.isThisFirstSessionState.value;
-            this.isThisFirstSessionState.updateValue(false);
+            await this.isThisFirstSessionState.updateValue(false);
             // "Official" VSC default setting value is stable. To keep the official value to be in sync with what is being used,
             // Use Insiders default as 'InsidersWeekly' only for the first session (insiders gets installed for the first session).
             return this.appEnvironment.channel === 'insiders' && isThisFirstSession ? ExtensionChannel.insidersDefaultForTheFirstSession : 'Stable';

src/client/common/insidersBuild/insidersExtensionService.ts

@@ -30,7 +30,7 @@ export class InsidersExtensionService implements IExtensionActivationService {
         }
         this.registerCommandsAndHandlers();
         this.activatedOnce = true;
-        const installChannel = this.extensionChannelService.channel;
+        const installChannel = await this.extensionChannelService.getChannel();
         const newExtensionChannel: Channel = installChannel === 'Stable' ? 'stable' : 'insiders';
         this.handleChannel(installChannel, newExtensionChannel !== this.appEnvironment.extensionChannel).ignoreErrors();
     }

src/client/common/insidersBuild/types.ts

@@ -15,7 +15,7 @@ export interface IExtensionChannelRule {
 export const IExtensionChannelService = Symbol('IExtensionChannelService');
 export interface IExtensionChannelService {
     readonly onDidChannelChange: Event<ExtensionChannels>;
-    readonly channel: ExtensionChannels;
+    getChannel(): Promise<ExtensionChannels>;
     updateChannel(value: ExtensionChannels): Promise<void>;
 }
 

src/test/common/insidersBuild/downloadChannelService.unit.test.ts

@@ -92,7 +92,8 @@ suite('Download channel service', () => {
                 .setup(u => u.updateValue(false))
                 .returns(() => Promise.resolve());
             when(appEnvironment.channel).thenReturn(testParams.vscodeChannel as any);
-            expect(channelService.channel).to.equal(testParams.expectedResult);
+            const channel = await channelService.getChannel();
+            expect(channel).to.equal(testParams.expectedResult);
             workspaceConfig.verifyAll();
         });
     });
@@ -114,7 +115,8 @@ suite('Download channel service', () => {
             .setup(u => u.updateValue(false))
             .returns(() => Promise.resolve());
         when(appEnvironment.channel).thenReturn('insiders');
-        expect(channelService.channel).to.equal('Stable');
+        const channel = await channelService.getChannel();
+        expect(channel).to.equal('Stable');
         workspaceConfig.verifyAll();
     });
 
@@ -128,7 +130,7 @@ suite('Download channel service', () => {
         workspaceConfig.setup(c => c.inspect<ExtensionChannels>(insidersChannelSetting))
             .returns(() => settings as any)
             .verifiable(TypeMoq.Times.once());
-        expect(() => channelService.channel).to.throw();
+        await expect(channelService.getChannel()).to.eventually.be.rejected;
         workspaceConfig.verifyAll();
     });
 

src/test/common/insidersBuild/insidersExtensionService.unit.test.ts

@@ -157,11 +157,11 @@ suite('Insiders Extension Service - Activation', () => {
             handleChannel = sinon.stub(InsidersExtensionService.prototype, 'handleChannel');
             handleChannel.callsFake(() => Promise.resolve());
             insidersExtensionService = new InsidersExtensionService(instance(extensionChannelService), instance(insidersPrompt), instance(appEnvironment), instance(cmdManager), instance(serviceContainer), []);
-            when(extensionChannelService.channel).thenReturn(testParams.installChannel);
+            when(extensionChannelService.getChannel()).thenResolve(testParams.installChannel);
             when(appEnvironment.extensionChannel).thenReturn(testParams.extensionChannel);
             await insidersExtensionService.activate(Uri.parse('r'));
             expect(handleChannel.args[0][1]).to.equal(testParams.expectedResult);
-            verify(extensionChannelService.channel).once();
+            verify(extensionChannelService.getChannel()).once();
             verify(appEnvironment.extensionChannel).once();
             expect(insidersExtensionService.activatedOnce).to.equal(true, 'Variable should be set to true');
         });
@@ -172,7 +172,7 @@ suite('Insiders Extension Service - Activation', () => {
         handleChannel = sinon.stub(InsidersExtensionService.prototype, 'handleChannel');
         handleChannel.callsFake(() => handleChannelsDeferred.promise);
         insidersExtensionService = new InsidersExtensionService(instance(extensionChannelService), instance(insidersPrompt), instance(appEnvironment), instance(cmdManager), instance(serviceContainer), []);
-        when(extensionChannelService.channel).thenReturn('InsidersDaily');
+        when(extensionChannelService.getChannel()).thenResolve('InsidersDaily');
         when(appEnvironment.extensionChannel).thenReturn('insiders');
 
         const promise = insidersExtensionService.activate(Uri.parse('r'));
@@ -185,7 +185,7 @@ suite('Insiders Extension Service - Activation', () => {
         handleChannelsDeferred.resolve();
         await sleep(1);
 
-        verify(extensionChannelService.channel).once();
+        verify(extensionChannelService.getChannel()).once();
         verify(appEnvironment.extensionChannel).once();
         expect(insidersExtensionService.activatedOnce).to.equal(true, 'Variable should be set to true');
     });