Turn insiders back on (#7640)

* Turn insiders back on

* Need experiments on to use insiders fix

* Experiments were being manually disabled

* Try forcing experiments on

* Force experiments and logging on

* Try some more ideas and print out status in jupyter extension

* Turn off jupyter experiments

* Logging level can't be set

* Try using env variable for logging

* Seems can't update without an error

* Log experimentation service too

* Activate python extension before tests to verify tests will actually run

* Update json to be actual json

* Use custom extra logging python extension

* Add kernel logging on jupyter side

* Add more logging to kernel spec finding

* More logging. Get rid of ci interpreter cache

* Remove cache but put the logging back

* Add more logging

* Fix preferred check in finder

* Undo extra installs

* Remove workspace interpreter cache. Active interpreter does not seem to update

* Fix linter

* pythonPath is no longer being used

* Smoke tests still using stable
Fix searching algorithm to use full display

* Use all workspace folders for venvs

* Wait for all interpreters

* Fix weird output items crash

* Dont need exact match for foo (might have markdown?)

* Put back active interpreter cache and use latest python insiders

.github/workflows/build-test.yml

@@ -34,6 +34,7 @@ env:
   CACHE_OUT_DIRECTORY: cache-out-directory
   CACHE_PIP_DEPS: cache-pip
   VSC_JUPYTER_FORCE_LOGGING: 'true'
+  VSC_PYTHON_FORCE_LOGGING: 'true'
   VSC_JUPYTER_CI_RUN_NON_PYTHON_NB_TEST: 'true'
   # Key for the cache created at the end of the the 'Cache ./pythonFiles/lib/python' step.
   CACHE_PYTHONFILES: cache-pvsc-pythonFiles
@@ -44,7 +45,8 @@ env:
   DISABLE_INSIDERS_EXTENSION: 1 # Disable prompts to install Insiders in tests (else it blocks activation of extension).
   VSC_JUPYTER_INSTRUMENT_CODE_FOR_COVERAGE: true
   VSIX_NAME_PYTHON: 'ms-python-insiders.vsix'
-  VSC_JUPTYER_PYTHON_EXTENSION_VERSION: 'stable'
+  VSC_JUPTYER_PYTHON_EXTENSION_VERSION: 'insiders'
+  VSC_JUPYTER_LOG_KERNEL_OUTPUT: true
 
 jobs:
   pick_environment:

src/client/api/pythonApi.ts

@@ -12,13 +12,13 @@
 // Licensed under the MIT License.
 
 import { inject, injectable, named } from 'inversify';
-import { CancellationToken, Disposable, Event, EventEmitter, Memento, Uri } from 'vscode';
+import { CancellationToken, Disposable, Event, EventEmitter, Memento, Uri, workspace } from 'vscode';
 import { IApplicationShell, ICommandManager, IWorkspaceService } from '../common/application/types';
 import { isCI } from '../common/constants';
 import { trackPackageInstalledIntoInterpreter } from '../common/installer/productInstaller';
 import { ProductNames } from '../common/installer/productNames';
 import { InterpreterUri } from '../common/installer/types';
-import { traceInfo } from '../common/logger';
+import { traceInfo, traceInfoIf } from '../common/logger';
 import {
     GLOBAL_MEMENTO,
     IDisposableRegistry,
@@ -39,6 +39,7 @@ import { IInterpreterQuickPickItem, IInterpreterSelector } from '../interpreter/
 import { IInterpreterService } from '../interpreter/contracts';
 import { IWindowsStoreInterpreter } from '../interpreter/locators/types';
 import { PythonEnvironment } from '../pythonEnvironments/info';
+import { areInterpreterPathsSame } from '../pythonEnvironments/info/interpreter';
 import { captureTelemetry, sendTelemetryEvent } from '../telemetry';
 import {
     ILanguageServer,
@@ -96,6 +97,12 @@ export class PythonApiProvider implements IPythonApiProvider {
             return;
         }
         this.api.resolve(api);
+
+        // Log experiment status here. Python extension is definitely loaded at this point.
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        const pythonConfig = workspace.getConfiguration('python', (null as any) as Uri);
+        const experimentsSection = pythonConfig.get('experiments');
+        traceInfo(`Experiment status for python is ${JSON.stringify(experimentsSection)}`);
     }
 
     private async init() {
@@ -315,17 +322,17 @@ export class InterpreterSelector implements IInterpreterSelector {
     }
 }
 
-const interpreterCacheForCI = new Map<string, PythonEnvironment[]>();
 // eslint-disable-next-line max-classes-per-file
 @injectable()
 export class InterpreterService implements IInterpreterService {
     private readonly didChangeInterpreter = new EventEmitter<void>();
     private eventHandlerAdded?: boolean;
+    private interpreterListCachePromise: Promise<PythonEnvironment[]> | undefined = undefined;
     constructor(
         @inject(IPythonApiProvider) private readonly apiProvider: IPythonApiProvider,
         @inject(IPythonExtensionChecker) private extensionChecker: IPythonExtensionChecker,
         @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
-        @inject(IWorkspaceService) private readonly workspace: IWorkspaceService
+        @inject(IWorkspaceService) private workspace: IWorkspaceService
     ) {
         if (this.extensionChecker.isPythonExtensionInstalled) {
             if (!this.extensionChecker.isPythonExtensionActive) {
@@ -338,6 +345,7 @@ export class InterpreterService implements IInterpreterService {
                 );
             }
         }
+        this.workspace.onDidChangeWorkspaceFolders(this.onDidChangeWorkspaceFolders, this, disposables);
     }
 
     public get onDidChangeInterpreter(): Event<void> {
@@ -348,25 +356,13 @@ export class InterpreterService implements IInterpreterService {
     @captureTelemetry(Telemetry.InterpreterListingPerf)
     public getInterpreters(resource?: Uri): Promise<PythonEnvironment[]> {
         this.hookupOnDidChangeInterpreterEvent();
-        const promise = this.apiProvider.getApi().then((api) => api.getInterpreters(resource));
-        if (isCI) {
-            promise
-                .then((items) => {
-                    const current = interpreterCacheForCI.get(resource?.toString() || '');
-                    const itemToStore = items;
-                    if (
-                        current &&
-                        (itemToStore === current || JSON.stringify(itemToStore) === JSON.stringify(current))
-                    ) {
-                        return;
-                    }
-                    interpreterCacheForCI.set(resource?.toString() || '', itemToStore);
-                    traceInfo(`Interpreter list for ${resource?.toString()} is ${JSON.stringify(items)}`);
-                })
-                .catch(noop);
+        // Cache result as it only changes when the interpreter list changes or we add more workspace folders
+        if (!this.interpreterListCachePromise) {
+            this.interpreterListCachePromise = this.getInterpretersImpl(resource);
         }
-        return promise;
+        return this.interpreterListCachePromise;
     }
+
     private workspaceCachedActiveInterpreter = new Map<string, Promise<PythonEnvironment | undefined>>();
     @captureTelemetry(Telemetry.ActiveInterpreterListingPerf)
     public getActiveInterpreter(resource?: Uri): Promise<PythonEnvironment | undefined> {
@@ -405,6 +401,29 @@ export class InterpreterService implements IInterpreterService {
             return undefined;
         }
     }
+
+    private onDidChangeWorkspaceFolders() {
+        this.interpreterListCachePromise = undefined;
+    }
+    private async getInterpretersImpl(resource?: Uri): Promise<PythonEnvironment[]> {
+        // Python uses the resource to look up the workspace folder. For Jupyter
+        // we want all interpreters regardless of workspace folder so call this multiple times
+        const folders = this.workspace.workspaceFolders;
+        const all = folders
+            ? await Promise.all(folders.map((f) => this.apiProvider.getApi().then((api) => api.getInterpreters(f.uri))))
+            : await Promise.all([this.apiProvider.getApi().then((api) => api.getInterpreters(undefined))]);
+
+        // Remove dupes
+        const result: PythonEnvironment[] = [];
+        all.flat().forEach((p) => {
+            if (!result.find((r) => areInterpreterPathsSame(r.path, p.path))) {
+                result.push(p);
+            }
+        });
+        traceInfoIf(isCI, `Interpreter list for ${resource?.toString()} is ${result.map((i) => i.path).join('\n')}`);
+        return result;
+    }
+
     private hookupOnDidChangeInterpreterEvent() {
         // Only do this once.
         if (this.eventHandlerAdded) {
@@ -424,8 +443,9 @@ export class InterpreterService implements IInterpreterService {
                     this.eventHandlerAdded = true;
                     api.onDidChangeInterpreter(
                         () => {
-                            this.didChangeInterpreter.fire();
+                            this.interpreterListCachePromise = undefined;
                             this.workspaceCachedActiveInterpreter.clear();
+                            this.didChangeInterpreter.fire();
                         },
                         this,
                         this.disposables

src/client/common/experiments/service.ts

@@ -10,6 +10,7 @@ import { sendTelemetryEvent } from '../../telemetry';
 import { EventName } from '../../telemetry/constants';
 import { IApplicationEnvironment } from '../application/types';
 import { JVSC_EXTENSION_ID, STANDARD_OUTPUT_CHANNEL } from '../constants';
+import { traceInfo } from '../logger';
 import {
     GLOBAL_MEMENTO,
     IConfigurationService,
@@ -81,6 +82,8 @@ export class ExperimentService implements IExperimentService {
             this.globalState
         );
 
+        traceInfo(`Experimentation service retrieved: ${this.experimentationService}`);
+
         this.logExperiments();
     }
 

src/client/datascience/jupyter/kernels/helpers.ts

@@ -702,12 +702,12 @@ export function findPreferredKernel(
             // Trace score for kernel
             traceInfo(`findPreferredKernel score for ${getDisplayNameOrNameOfKernelConnection(metadata)} is ${score}`);
 
-            // If we have a score of 2, this can only happen if we match against language and find a Python 3 kernel.
+            // If we have a score of 5, this can only happen if we match against language and find a Python 3 kernel.
             // In such cases, use our preferred interpreter kernel if we have one.
             // I.e. give preference to the preferred interpreter kernelspec if we dont have any matches.
             if (
-                subScore === 2 &&
-                score === 2 &&
+                subScore === 5 && // This is a bit flakey. Number isn't consistent. Should probably just make the order of kernelspecs have the preferred one first
+                score === 5 &&
                 (metadata.kind === 'startUsingPythonInterpreter' ||
                     (metadata.kind === 'startUsingKernelSpec' && metadata.kernelSpec.language === PYTHON_LANGUAGE)) &&
                 preferredInterpreterKernelSpecIndex >= 0 &&

src/client/datascience/kernel-launcher/localKernelFinder.ts

@@ -98,7 +98,8 @@ export class LocalKernelFinder implements ILocalKernelFinder {
                     resourceType,
                     language: telemetrySafeLanguage
                 },
-                ex,
+                // eslint-disable-next-line @typescript-eslint/no-explicit-any
+                ex as any,
                 true
             );
             traceError(`findKernel crashed`, ex);

src/client/datascience/kernel-launcher/localPythonAndRelatedNonPythonKernelSpecFinder.ts

@@ -65,6 +65,12 @@ export class LocalPythonAndRelatedNonPythonKernelSpecFinder extends LocalKernelS
         const interpreters = this.extensionChecker.isPythonExtensionInstalled
             ? await this.interpreterService.getInterpreters(resource)
             : [];
+
+        traceInfoIf(
+            !!process.env.VSC_JUPYTER_LOG_KERNEL_OUTPUT,
+            `listKernelsImplementation for ${resource?.toString()}: ${interpreters.map((i) => i.path).join('\n')}`
+        );
+
         // If we don't have Python extension installed or don't discover any Python interpreters
         // then list all of the global python kernel specs.
         if (interpreters.length === 0 || !this.extensionChecker.isPythonExtensionInstalled) {
@@ -378,8 +384,20 @@ export class LocalPythonAndRelatedNonPythonKernelSpecFinder extends LocalKernelS
         interpreters: PythonEnvironment[],
         cancelToken?: CancellationToken
     ): Promise<IJupyterKernelSpec[]> {
+        traceInfoIf(
+            !!process.env.VSC_JUPYTER_LOG_KERNEL_OUTPUT,
+            `Finding kernel specs for interpreters: ${interpreters.map((i) => i.path).join('\n')}`
+        );
         // Find all the possible places to look for this resource
         const paths = await this.findKernelPathsOfAllInterpreters(interpreters);
+        traceInfoIf(
+            !!process.env.VSC_JUPYTER_LOG_KERNEL_OUTPUT,
+            `Finding kernel specs for paths: ${paths
+                // eslint-disable-next-line @typescript-eslint/no-explicit-any
+                .map((p) => ((p as any).interpreter ? (p as any).interpreter.path : p))
+                .join('\n')}`
+        );
+
         const searchResults = await this.findKernelSpecsInPaths(paths, cancelToken);
         let results: IJupyterKernelSpec[] = [];
         await Promise.all(
@@ -424,6 +442,11 @@ export class LocalPythonAndRelatedNonPythonKernelSpecFinder extends LocalKernelS
             }
         });
 
+        traceInfoIf(
+            !!process.env.VSC_JUPYTER_LOG_KERNEL_OUTPUT,
+            `Finding kernel specs unique results: ${unique.map((u) => u.interpreterPath!).join('\n')}`
+        );
+
         return unique;
     }
 

src/client/datascience/notebook/helpers/helpers.ts

@@ -787,7 +787,7 @@ export function translateErrorOutput(output?: nbformat.IError): NotebookCellOutp
 }
 
 export function getTextOutputValue(output: NotebookCellOutput): string {
-    const item = output.items.find(
+    const item = output?.items?.find(
         (opit) =>
             opit.mime === CellOutputMimeTypes.stdout ||
             opit.mime === CellOutputMimeTypes.stderr ||

src/test/common.ts

@@ -139,10 +139,10 @@ async function setGlobalPathToInterpreter(pythonPath?: string): Promise<void> {
     await pythonConfig.update('defaultInterpreterPath', pythonPath, true);
     await disposePythonSettings();
 }
-export async function disableExperimentsInPythonExtension(): Promise<void> {
+export async function adjustSettingsInPythonExtension(): Promise<void> {
     const vscode = require('vscode') as typeof import('vscode');
     const pythonConfig = vscode.workspace.getConfiguration('python', (null as any) as Uri);
-    await pythonConfig.update('experiments.enabled', false, vscode.ConfigurationTarget.Global).then(noop, noop);
+    await pythonConfig.update('experiments.enabled', 'true', vscode.ConfigurationTarget.Global).then(noop, noop);
 }
 export const resetGlobalPythonPathSetting = async () => retryAsync(restoreGlobalPythonPathSetting)();
 
@@ -222,8 +222,9 @@ async function setPythonPathInWorkspace(
     const value = settings.inspect<string>('pythonPath');
     const prop: 'workspaceFolderValue' | 'workspaceValue' =
         config === vscode.ConfigurationTarget.Workspace ? 'workspaceValue' : 'workspaceFolderValue';
-    if (value && value[prop] !== pythonPath) {
+    if (!value || value[prop] !== pythonPath) {
         await settings.update('pythonPath', pythonPath, config).then(noop, noop);
+        await settings.update('defaultInterpreterPath', pythonPath, config).then(noop, noop);
         if (config === vscode.ConfigurationTarget.Global) {
             await settings.update('defaultInterpreterPath', pythonPath, config).then(noop, noop);
         }

src/test/datascience/.vscode/settings.json

@@ -26,8 +26,8 @@
     "jupyter.logging.level": "debug",
     "python.logging.level": "debug",
     "webview.experimental.useIframes": true,
-    // Disable all experiments in python insiders
-    "python.experiments.enabled": "false",
     "jupyter.interactiveWindowMode": "multiple",
-    "jupyter.magicCommandsAsComments": false
+    "jupyter.magicCommandsAsComments": false,
+    // Python experiments have to be on for insiders to work
+    "python.experiments.enabled": true
 }

src/test/datascience/notebook/executionService.vscode.test.ts

@@ -242,7 +242,7 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () {
 
         assert.lengthOf(displayCell.outputs, 1, 'Incorrect output');
         expect(displayCell.executionSummary?.executionOrder).to.be.greaterThan(0, 'Execution count should be > 0');
-        assertHasTextOutputInVSCode(displayCell, 'foo', 0, true);
+        await waitForTextOutput(displayCell, 'foo', 0, false);
     });
     test('Clearing output while executing will ensure output is cleared', async function () {
         // Assume you are executing a cell that prints numbers 1-100.

src/test/datascience/notebook/intellisense/completion.vscode.test.ts

@@ -40,9 +40,6 @@ suite('DataScience - VSCode Intellisense Notebook and Interactive Code Completio
         if (!(await canRunNotebookTests())) {
             return this.skip();
         }
-        // Skip for now. We need python insiders and it's not working
-        return this.skip();
-
         await startJupyterServer();
         await prewarmNotebooks();
         sinon.restore();

src/test/datascience/notebook/intellisense/diagnostics.vscode.test.ts

@@ -35,8 +35,6 @@ suite('DataScience - VSCode Intellisense Notebook Diagnostics', function () {
         if (!(await canRunNotebookTests())) {
             return this.skip();
         }
-        // Skip for now. We need python insiders and it's not working
-        return this.skip();
         sinon.restore();
         vscodeNotebook = api.serviceContainer.get<IVSCodeNotebook>(IVSCodeNotebook);
         traceInfo(`Start Suite (Completed) Diagnostics`);

src/test/datascience/notebook/intellisense/hover.vscode.test.ts

@@ -36,9 +36,6 @@ suite('DataScience - VSCode Intellisense Notebook Hover', function () {
         if (!(await canRunNotebookTests())) {
             return this.skip();
         }
-        // Skip for now. We need python insiders and it's not working
-        return this.skip();
-
         sinon.restore();
         vscodeNotebook = api.serviceContainer.get<IVSCodeNotebook>(IVSCodeNotebook);
         traceInfo(`Start Suite (Completed) Hover`);

src/test/datascience/notebook/kernelSelection.vscode.test.ts

@@ -92,6 +92,8 @@ suite('DataScience - VSCode Notebook - Kernel Selection', function () {
         }
 
         const interpreterService = api.serviceContainer.get<IInterpreterService>(IInterpreterService);
+        // Wait for all interpreters so we can make sure we can get details on the paths we have
+        await interpreterService.getInterpreters();
         const [activeInterpreter, interpreter1, interpreter2, interpreter3] = await Promise.all([
             interpreterService.getActiveInterpreter(),
             interpreterService.getInterpreterDetails(venvNoKernelPython),
@@ -105,7 +107,7 @@ suite('DataScience - VSCode Notebook - Kernel Selection', function () {
         venvNoKernelPythonPath = interpreter1.path;
         venvKernelPythonPath = interpreter2.path;
         venvNoRegPythonPath = interpreter3.path;
-        venvNoKernelDisplayName = IS_REMOTE_NATIVE_TEST ? interpreter1.displayName || '.venvnokernel' : '.venvnokernel';
+        venvNoKernelDisplayName = interpreter1.displayName || '.venvnokernel';
         activeIntepreterSearchString =
             activeInterpreter.displayName === interpreter1.displayName
                 ? venvNoKernelSearchString

src/test/index.ts

@@ -184,13 +184,9 @@ export async function run(): Promise<void> {
 
     /* eslint-disable no-console */
     console.time('Time taken to activate the extension');
-    try {
-        console.log('Starting & waiting for Python extension to activate');
-        await activateExtensionScript();
-        console.timeEnd('Time taken to activate the extension');
-    } catch (ex) {
-        console.error('Failed to activate python extension without errors', ex);
-    }
+    console.log('Starting & waiting for Python extension to activate');
+    await activateExtensionScript();
+    console.timeEnd('Time taken to activate the extension');
 
     try {
         // Run the tests.

src/test/initialize.ts

@@ -3,10 +3,11 @@ import * as vscode from 'vscode';
 import type { IExtensionApi } from '../client/api';
 import { disposeAllDisposables } from '../client/common/helpers';
 import type { IDisposable } from '../client/common/types';
+import { PythonExtension } from '../client/datascience/constants';
 import { clearPendingChainedUpdatesForTests } from '../client/datascience/notebook/helpers/notebookUpdater';
 import {
+    adjustSettingsInPythonExtension,
     clearPendingTimers,
-    disableExperimentsInPythonExtension,
     IExtensionTestApi,
     PYTHON_PATH,
     setPythonPathInWorkspaceRoot
@@ -25,6 +26,11 @@ process.env.VSC_JUPYTER_CI_TEST = '1';
 // Ability to use custom python environments for testing
 export async function initializePython() {
     await setPythonPathInWorkspaceRoot(PYTHON_PATH);
+    // Make sure the python extension can load if this test allows it
+    if (!process.env.VSC_JUPYTER_CI_TEST_DO_NOT_INSTALL_PYTHON_EXT) {
+        const extension = vscode.extensions.getExtension(PythonExtension)!;
+        await extension.activate();
+    }
 }
 
 export function isInsiders() {
@@ -34,7 +40,7 @@ export function isInsiders() {
 let jupyterServerStarted = false;
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 export async function initialize(): Promise<IExtensionTestApi> {
-    await disableExperimentsInPythonExtension();
+    await adjustSettingsInPythonExtension();
     await initializePython();
     const api = await activateExtension();
     // Ensure we start jupyter server before opening any notebooks or the like.