Fix widget test failures (#15029)

* Add logging for widget test failure * More logging * hello * More logging * more logging * add more logging * more logging * Log the widgets * Re-enable test * oops * oops * Ad logging * Fixes * Misc * Fixes * oops * Misc * Fixes * More fixes * Fixes * Misc * Misc * Misc * Fixes
microsoft · Jan 18, 2024 · 27bb068 · 27bb068
1 parent 091fc02
commit 27bb068
Show file tree

Hide file tree

Showing 13 changed files with 60 additions and 34 deletions.
diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml
@@ -556,8 +556,6 @@ jobs:
           python -m jupyter nbextension uninstall --sys-prefix --py ipympl
           sudo $PYTHON_EXECUTABLE -m jupyter nbextension install --system --py ipympl
 
-        # This step is slow.
-
       # Run npm install (we need chrome to get downloaded)
       - name: npm ci
         if: matrix.jupyterConnection == 'web'

diff --git a/src/kernels/raw/finder/contributedKerneFinder.node.unit.test.ts b/src/kernels/raw/finder/contributedKerneFinder.node.unit.test.ts
@@ -10,7 +10,6 @@ import * as uriPath from '../../../platform/vscode-path/resources';
 import * as sinon from 'sinon';
 import { anything, instance, mock, when, verify } from 'ts-mockito';
 import { IPlatformService } from '../../../platform/common/platform/types';
-import { IInterpreterService } from '../../../platform/interpreter/contracts';
 import { CustomEnvironmentVariablesProvider } from '../../../platform/common/variables/customEnvironmentVariablesProvider.node';
 import { InterpreterService } from '../../../platform/api/pythonApi';
 import {
@@ -58,7 +57,6 @@ import { IPythonExecutionService, IPythonExecutionFactory } from '../../../platf
 [false, true].forEach((isWindows) => {
     suite(`Contributed Local Kernel Spec Finder ${isWindows ? 'Windows' : 'Unix'}`, () => {
         let kernelFinder: KernelFinder;
-        let interpreterService: IInterpreterService;
         let platformService: IPlatformService;
         let fs: FileSystem;
         let extensionChecker: IPythonExtensionChecker;
@@ -96,7 +94,7 @@ import { IPythonExecutionService, IPythonExecutionFactory } from '../../../platf
             cancelToken = new CancellationTokenSource();
             const getOSTypeStub = sinon.stub(platform, 'getOSType');
             getOSTypeStub.returns(isWindows ? platform.OSType.Windows : platform.OSType.Linux);
-            interpreterService = mock(InterpreterService);
+            const interpreterService = mock(InterpreterService);
             onDidChangeInterpreter = new EventEmitter<PythonEnvironment | undefined>();
             onDidChangeInterpreters = new EventEmitter<PythonEnvironment[]>();
             onDidChangeInterpreterStatus = new EventEmitter<void>();
@@ -125,6 +123,7 @@ import { IPythonExecutionService, IPythonExecutionFactory } from '../../../platf
             when(interpreterService.resolvedEnvironments).thenReturn(Array.from(distinctInterpreters));
             when(interpreterService.getActiveInterpreter(anything())).thenResolve(activeInterpreter);
             when(interpreterService.getInterpreterDetails(anything())).thenResolve();
+            when(interpreterService.getInterpreterDetails(anything(), anything())).thenResolve();
             platformService = mock(PlatformService);
             when(platformService.isWindows).thenReturn(isWindows);
             when(platformService.isLinux).thenReturn(!isWindows);
@@ -163,7 +162,8 @@ import { IPythonExecutionService, IPythonExecutionFactory } from '../../../platf
                 instance(memento),
                 instance(fs),
                 instance(context),
-                instance(pythonExecFactory)
+                instance(pythonExecFactory),
+                instance(interpreterService)
             );
 
             const kernelSpecsBySpecFile = new Map<string, KernelSpec.ISpecModel>();

diff --git a/src/kernels/raw/finder/jupyterPaths.node.ts b/src/kernels/raw/finder/jupyterPaths.node.ts
@@ -7,7 +7,14 @@ import * as uriPath from '../../../platform/vscode-path/resources';
 import { CancellationToken, Memento, Uri } from 'vscode';
 import { IFileSystem, IPlatformService } from '../../../platform/common/platform/types';
 import { IFileSystemNode } from '../../../platform/common/platform/types.node';
-import { ignoreLogging, logValue, traceError, traceVerbose, traceWarning } from '../../../platform/logging';
+import {
+    ignoreLogging,
+    logValue,
+    traceError,
+    traceInfoIfCI,
+    traceVerbose,
+    traceWarning
+} from '../../../platform/logging';
 import {
     IDisposableRegistry,
     IMemento,
@@ -26,6 +33,7 @@ import { IPythonExecutionFactory } from '../../../platform/interpreter/types.nod
 import { getDisplayPath } from '../../../platform/common/platform/fs-paths';
 import { StopWatch } from '../../../platform/common/utils/stopWatch';
 import { ResourceMap, ResourceSet } from '../../../platform/common/utils/map';
+import { IInterpreterService } from '../../../platform/interpreter/contracts';
 
 const winJupyterPath = path.join('AppData', 'Roaming', 'jupyter', 'kernels');
 const linuxJupyterPath = path.join('.local', 'share', 'jupyter', 'kernels');
@@ -54,7 +62,8 @@ export class JupyterPaths {
         @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalState: Memento,
         @inject(IFileSystemNode) private readonly fs: IFileSystem,
         @inject(IExtensionContext) private readonly context: IExtensionContext,
-        @inject(IPythonExecutionFactory) private readonly pythonExecFactory: IPythonExecutionFactory
+        @inject(IPythonExecutionFactory) private readonly pythonExecFactory: IPythonExecutionFactory,
+        @inject(IInterpreterService) private readonly interpreters: IInterpreterService
     ) {
         this.envVarsProvider.onDidEnvironmentVariablesChange(
             () => {
@@ -152,7 +161,10 @@ export class JupyterPaths {
         return this.cachedDataDirs.get(key)!;
     }
 
-    @traceDecoratorVerbose('getDataDirsImpl', TraceOptions.BeforeCall | TraceOptions.Arguments)
+    @traceDecoratorVerbose(
+        'getDataDirsImpl',
+        TraceOptions.BeforeCall | TraceOptions.Arguments | TraceOptions.ReturnValue
+    )
     private async getDataDirsImpl(
         resource: Resource,
         @logValue<PythonEnvironment>('id') interpreter?: PythonEnvironment
@@ -171,6 +183,7 @@ export class JupyterPaths {
         // 2. Add the paths based on ENABLE_USER_SITE
         if (interpreter) {
             try {
+                traceInfoIfCI(`Getting Jupyter Data Dir for ${interpreter.uri.fsPath}`);
                 const factory = await this.pythonExecFactory.createActivatedEnvironment({
                     interpreter,
                     resource
@@ -184,15 +197,23 @@ export class JupyterPaths {
                             dataDir.set(sitePath, dataDir.size);
                         }
                     } else {
-                        traceWarning(`Got a non-existent Jupyer Data Dir ${sitePath}`);
+                        traceWarning(`Got a non-existent Jupyter Data Dir ${sitePath}`);
                     }
+                } else {
+                    traceWarning(`Got an empty Jupyter Data Dir from ${interpreter.id}, stderr = ${result.stderr}`);
                 }
             } catch (ex) {
                 traceError(`Failed to get DataDir based on ENABLE_USER_SITE for ${interpreter.displayName}`, ex);
             }
         }
 
         // 3. Add the paths based on user and env data directories
+        if (interpreter && !interpreter.sysPrefix) {
+            traceWarning(`sysPrefix was not set for ${interpreter.id}`);
+            const details = await this.interpreters.getInterpreterDetails(interpreter.uri);
+            interpreter.sysPrefix = details?.sysPrefix || interpreter.sysPrefix;
+            traceInfoIfCI(`sysPrefix after getting details ${interpreter.sysPrefix}`);
+        }
         const possibleEnvJupyterPath = interpreter?.sysPrefix
             ? Uri.joinPath(Uri.file(interpreter.sysPrefix), 'share', 'jupyter')
             : undefined;

diff --git a/src/kernels/raw/finder/jupyterPaths.node.unit.test.ts b/src/kernels/raw/finder/jupyterPaths.node.unit.test.ts
@@ -19,6 +19,7 @@ import { PythonEnvironment } from '../../../platform/pythonEnvironments/info';
 import * as path from '../../../platform/vscode-path/path';
 import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../../test/constants.node';
 import { uriEquals } from '../../../test/datascience/helpers';
+import { IInterpreterService } from '../../../platform/interpreter/contracts';
 
 suite('Jupyter Paths', () => {
     let disposables: IDisposable[] = [];
@@ -64,14 +65,17 @@ suite('Jupyter Paths', () => {
         context = mock<ExtensionContext>();
         when(context.extensionUri).thenReturn(extensionUri);
         when(envVarsProvider.getEnvironmentVariables(anything(), anything())).thenResolve(process.env);
+        const interpreterService = mock<IInterpreterService>();
+        when(interpreterService.getInterpreterDetails(anything())).thenResolve();
         jupyterPaths = new JupyterPaths(
             instance(platformService),
             instance(envVarsProvider),
             instance(disposables),
             instance(memento),
             instance(fs),
             instance(context),
-            instance(pythonExecFactory)
+            instance(pythonExecFactory),
+            instance(interpreterService)
         );
         delete process.env['JUPYTER_PATH'];
         delete process.env['APPDATA'];

diff --git a/src/kernels/types.ts b/src/kernels/types.ts
@@ -33,6 +33,7 @@ import {
 } from '../platform/common/constants';
 import { sendTelemetryEvent } from '../telemetry';
 import { generateIdFromRemoteProvider } from './jupyter/jupyterUtils';
+import { traceInfoIfCI } from '../platform/logging';
 
 export type WebSocketData = string | Buffer | ArrayBuffer | Buffer[];
 
@@ -286,6 +287,13 @@ export class PythonKernelConnectionMetadata {
         };
     }
     public updateInterpreter(interpreter: PythonEnvironment) {
+        if (!interpreter.sysPrefix) {
+            traceInfoIfCI(
+                `WARNING: Interpreter ${interpreter.id} has no sysPrefix and existing item ${
+                    this.interpreter.sysPrefix
+                } will be blown away, ${new Error().stack}`
+            );
+        }
         Object.assign(this.interpreter, interpreter);
     }
     public static fromJSON(options: Record<string, unknown> | PythonKernelConnectionMetadata) {

diff --git a/src/notebooks/controllers/ipywidgets/scriptSourceProvider/baseIPyWidgetScriptManager.ts b/src/notebooks/controllers/ipywidgets/scriptSourceProvider/baseIPyWidgetScriptManager.ts
@@ -5,7 +5,7 @@ import { Uri } from 'vscode';
 import { splitLines, trimQuotes } from '../../../../platform/common/helpers';
 import { getDisplayPath } from '../../../../platform/common/platform/fs-paths';
 import { IDisposable } from '../../../../platform/common/types';
-import { traceError, traceInfoIfCI, traceWarning } from '../../../../platform/logging';
+import { traceError, traceWarning } from '../../../../platform/logging';
 import { sendTelemetryEvent, Telemetry } from '../../../../telemetry';
 import { IKernel, isLocalConnection } from '../../../../kernels/types';
 import { getTelemetrySafeHashedString } from '../../../../platform/telemetry/helpers';
@@ -205,20 +205,13 @@ export abstract class BaseIPyWidgetScriptManager implements IIPyWidgetScriptMana
         const widgetConfigs = await Promise.all(
             entryPoints.map((entry) => this.getConfigFromWidget(baseUrl, entry.uri, entry.widgetFolderName))
         );
-
         const config = widgetConfigs.reduce((prev, curr) => Object.assign(prev || {}, curr), {});
         // Exclude entries that are not required (widgets that we have already bundled with our code).
         if (config && Object.keys(config).length) {
             delete config['jupyter-js-widgets'];
             delete config['@jupyter-widgets/base'];
             delete config['@jupyter-widgets/controls'];
             delete config['@jupyter-widgets/output'];
-        } else {
-            traceInfoIfCI(
-                `No config, entryPoints = ${JSON.stringify(entryPoints)}, widgetConfigs = ${JSON.stringify(
-                    widgetConfigs
-                )}`
-            );
         }
         sendTelemetryEvent(
             Telemetry.DiscoverIPyWidgetNamesPerf,

diff --git a/src/notebooks/controllers/ipywidgets/scriptSourceProvider/cdnWidgetScriptSourceProvider.ts b/src/notebooks/controllers/ipywidgets/scriptSourceProvider/cdnWidgetScriptSourceProvider.ts
@@ -68,7 +68,7 @@ function getCDNPrefix(cdn?: WidgetCDNs): string | undefined {
  */
 @injectable()
 export class CDNWidgetScriptSourceProvider implements IWidgetScriptSourceProvider {
-    id: 'cdn';
+    id = 'cdn';
     private cache = new Map<string, Promise<WidgetScriptSource>>();
     private isOnCDNCache = new Map<string, Promise<boolean>>();
     private readonly notifiedUserAboutWidgetScriptNotFound = new Set<string>();

diff --git a/src/notebooks/controllers/ipywidgets/scriptSourceProvider/ipyWidgetScriptSourceProvider.ts b/src/notebooks/controllers/ipywidgets/scriptSourceProvider/ipyWidgetScriptSourceProvider.ts
@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-import { traceError, traceInfo } from '../../../../platform/logging';
+import { traceError, traceVerbose, traceWarning } from '../../../../platform/logging';
 import { WidgetCDNs, IConfigurationService } from '../../../../platform/common/types';
 import { sendTelemetryEvent, Telemetry } from '../../../../telemetry';
 import { getTelemetrySafeHashedString } from '../../../../platform/telemetry/helpers';
@@ -27,7 +27,7 @@ import { noop } from '../../../../platform/common/utils/misc';
  * If user has not configured antying, user will be presented with a prompt.
  */
 export class IPyWidgetScriptSourceProvider extends DisposableBase implements IWidgetScriptSourceProvider {
-    id: 'all';
+    id = 'all';
     private readonly scriptProviders: IWidgetScriptSourceProvider[];
     private get configuredScriptSources(): readonly WidgetCDNs[] {
         const settings = this.configurationSettings.getSettings(undefined);
@@ -88,17 +88,21 @@ export class IPyWidgetScriptSourceProvider extends DisposableBase implements IWi
             if (source.scriptUri) {
                 found = source;
                 break;
+            } else {
+                traceWarning(
+                    `Widget Script Source not found for ${moduleName}@${moduleVersion} from ${scriptProvider.id}`
+                );
             }
         }
         this.sendTelemetryForWidgetModule(moduleName, moduleVersion, '', found.source).catch(noop);
         if (!found.scriptUri) {
             traceError(
-                `Script source for Widget ${moduleName}@${moduleVersion} not found in ${
-                    this.scriptProviders.map((item) => item.id).join(', ') || 'None'
-                } providers & ${this.isDisposed ? 'Disposed' : 'Not Disposed'}`
+                `Script source for Widget ${moduleName}@${moduleVersion} not found in ${this.scriptProviders
+                    .map((item) => item.id)
+                    .join(', ')}`
             );
         } else {
-            traceInfo(
+            traceVerbose(
                 `Script source for Widget ${moduleName}@${moduleVersion} was found from source ${found.source} and ${found.scriptUri}`
             );
         }

diff --git a/...notebooks/controllers/ipywidgets/scriptSourceProvider/localIPyWidgetScriptManager.node.ts b/...notebooks/controllers/ipywidgets/scriptSourceProvider/localIPyWidgetScriptManager.node.ts
@@ -12,6 +12,7 @@ import { IKernel } from '../../../../kernels/types';
 import { BaseIPyWidgetScriptManager } from './baseIPyWidgetScriptManager';
 import { IIPyWidgetScriptManager, INbExtensionsPathProvider } from '../types';
 import { JupyterPaths } from '../../../../kernels/raw/finder/jupyterPaths.node';
+import { traceWarning } from '../../../../platform/logging';
 
 type KernelConnectionId = string;
 /**
@@ -69,6 +70,7 @@ export class LocalIPyWidgetScriptManager extends BaseIPyWidgetScriptManager impl
             const stopWatch = new StopWatch();
             this.sourceNbExtensionsPath = await this.nbExtensionsPathProvider.getNbExtensionsParentPath(this.kernel);
             if (!this.sourceNbExtensionsPath) {
+                traceWarning(`No nbextensions folder found for kernel ${this.kernel.kernelConnectionMetadata.id}`);
                 return;
             }
             const kernelHash = await getTelemetrySafeHashedString(this.kernel.kernelConnectionMetadata.id);

diff --git a/...books/controllers/ipywidgets/scriptSourceProvider/localWidgetScriptSourceProvider.node.ts b/...books/controllers/ipywidgets/scriptSourceProvider/localWidgetScriptSourceProvider.node.ts
@@ -17,7 +17,7 @@ import {
  * <python folder>/share/jupyter/nbextensions/bqplot/index.js
  */
 export class LocalWidgetScriptSourceProvider implements IWidgetScriptSourceProvider {
-    id: 'local';
+    id = 'local';
     constructor(
         private readonly kernel: IKernel,
         private readonly localResourceUriConverter: ILocalResourceUriConverter,

diff --git a/...notebooks/controllers/ipywidgets/scriptSourceProvider/remoteWidgetScriptSourceProvider.ts b/...notebooks/controllers/ipywidgets/scriptSourceProvider/remoteWidgetScriptSourceProvider.ts
@@ -15,7 +15,7 @@ import {
  * `<remote url>/nbextensions/moduleName/index`
  */
 export class RemoteWidgetScriptSourceProvider implements IWidgetScriptSourceProvider {
-    id: 'remote';
+    id = 'remote';
     public static validUrls = new Map<string, boolean>();
     private readonly kernelConnection: RemoteKernelConnectionMetadata;
     private readonly scriptManager: IIPyWidgetScriptManager;

diff --git a/src/notebooks/controllers/kernelSource/localPythonEnvKernelSourceSelector.node.ts b/src/notebooks/controllers/kernelSource/localPythonEnvKernelSourceSelector.node.ts
@@ -224,11 +224,7 @@ export class LocalPythonEnvNotebookKernelSourceSelector
         });
 
         const existingInterpreterInfo = this._kernels.get(e.id);
-        if (existingInterpreterInfo) {
-            existingInterpreterInfo.updateInterpreter(result.interpreter);
-            this._kernels.set(e.id, Object.assign(existingInterpreterInfo, result));
-            this._onDidChangeKernels.fire({});
-        } else {
+        if (!existingInterpreterInfo) {
             this._kernels.set(e.id, result);
             this._onDidChangeKernels.fire({});
         }

diff --git a/src/test/datascience/widgets/thirdpartyWidgets.vscode.common.test.ts b/src/test/datascience/widgets/thirdpartyWidgets.vscode.common.test.ts
@@ -118,7 +118,7 @@ import { isWeb } from '../../../platform/common/utils/misc';
             await clickWidget(comms, cell0, 'button');
             await assertOutputContainsHtml(cell0, comms, ['>Figure 1<', '<canvas', 'Download plot']);
         });
-        test.skip('Render AnyWidget (test js<-->kernel comms with binary data)', async function () {
+        test('Render AnyWidget (test js<-->kernel comms with binary data)', async function () {
             await initializeNotebookForWidgetTest(
                 disposables,
                 {