From ec001a0b1503555e685996baab8fda4f0648c454 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Thu, 12 Oct 2023 14:11:15 -0700 Subject: [PATCH 01/15] Fix for webpack warning with LSP types (#22211) --- build/webpack/webpack.extension.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/build/webpack/webpack.extension.config.js b/build/webpack/webpack.extension.config.js index f496aa32ee26..7003ffa277d2 100644 --- a/build/webpack/webpack.extension.config.js +++ b/build/webpack/webpack.extension.config.js @@ -69,6 +69,7 @@ const config = { resolve: { extensions: ['.ts', '.js'], plugins: [new tsconfig_paths_webpack_plugin.TsconfigPathsPlugin({ configFile: configFileName })], + conditionNames: ['require', 'node'], }, output: { filename: '[name].js', From 6c23e4335db10e900ea0ca2402e267322c3a2e69 Mon Sep 17 00:00:00 2001 From: Anthony Kim <62267334+anthonykim1@users.noreply.github.com> Date: Thu, 12 Oct 2023 14:44:50 -0700 Subject: [PATCH 02/15] Handle white spaces for list along with dictionary (#22209) Legacy normalization script leaves unnecessary white spaces for dictionary as well as list. While there are multiple correct usage of it such as for after a function, for more intuitive REPL experience. We want to keep previous normalization style and white space format EXCEPT for dictionary and list case. Dictionary case is handled, but this is the PR to handle the elimination of extra white spaces for list as well. Closes: #22208 --- pythonFiles/normalizeSelection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pythonFiles/normalizeSelection.py b/pythonFiles/normalizeSelection.py index 0ac47ab5dc3b..7ace42daa901 100644 --- a/pythonFiles/normalizeSelection.py +++ b/pythonFiles/normalizeSelection.py @@ -119,7 +119,7 @@ def normalize_lines(selection): # Insert a newline between each top-level statement, and append a newline to the selection. source = "\n".join(statements) + "\n" - if selection[-2] == "}": + if selection[-2] == "}" or selection[-2] == "]": source = source[:-1] except Exception: # If there's a problem when parsing statements, From bdb8efb9dd20cefd56a11687ed7f7dfc89f9f15d Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Thu, 12 Oct 2023 17:18:49 -0700 Subject: [PATCH 03/15] Try using `import` in webpack condition names (#22212) --- build/webpack/webpack.extension.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/webpack/webpack.extension.config.js b/build/webpack/webpack.extension.config.js index 7003ffa277d2..a33508e5d96a 100644 --- a/build/webpack/webpack.extension.config.js +++ b/build/webpack/webpack.extension.config.js @@ -69,7 +69,7 @@ const config = { resolve: { extensions: ['.ts', '.js'], plugins: [new tsconfig_paths_webpack_plugin.TsconfigPathsPlugin({ configFile: configFileName })], - conditionNames: ['require', 'node'], + conditionNames: ['import', 'require', 'node'], }, output: { filename: '[name].js', From 76ae73a46bea7e722bf43e4d5550b3d895066d90 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 12 Oct 2023 21:52:38 -0700 Subject: [PATCH 04/15] Skip setting `PYTHONUTF8` when activating terminals (#22213) Closes https://github.com/microsoft/vscode-python/issues/22205 --- .../activation/terminalEnvVarCollectionService.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index c11ec221d4d7..92e97c95e468 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -394,7 +394,12 @@ function shouldPS1BeSet(type: PythonEnvType | undefined, env: EnvironmentVariabl } function shouldSkip(env: string) { - return ['_', 'SHLVL'].includes(env); + return [ + '_', + 'SHLVL', + // Even though this maybe returned, setting it can result in output encoding errors in terminal. + 'PYTHONUTF8', + ].includes(env); } function getPromptForEnv(interpreter: PythonEnvironment | undefined) { From eada0f1ab940729ae335389c34899d64b56edd35 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 13 Oct 2023 10:32:09 -0700 Subject: [PATCH 05/15] Bump microvenv from 2023.2.0 to 2023.3.post1 (#22204) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [microvenv](https://github.com/brettcannon/microvenv) from 2023.2.0 to 2023.3.post1.
Release notes

Sourced from microvenv's releases.

2023.3.post1

What's Changed

⚠️ Breaking Changes

🎉 New Features

Full Changelog: https://github.com/brettcannon/microvenv/compare/v2023.2.0...v2023.3.post1

Commits
  • bf19f92 Update the docs due to Windows support
  • 0c5436d Fix docs.yml
  • 00d43b4 Update the version for release
  • 2e8d62e Add a release.yml workflow
  • 7b9ca8a Add support for Windows (except for create()) (#55)
  • 7085e42 Drop CI path requirements
  • f9dc600 Add mypy's stubtest to linting
  • 54e08f8 Clarify that _create.py is self-contained
  • 3145fcb Merge branch 'main' of github.com:brettcannon/microvenv
  • 6c0529e Drop the static HTML directory
  • Additional commits viewable in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=microvenv&package-manager=pip&previous-version=2023.2.0&new-version=2023.3.post1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements.txt b/requirements.txt index 205b9fc4804c..31765898ab59 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,9 +8,9 @@ importlib-metadata==6.7.0 \ --hash=sha256:1aaf550d4f73e5d6783e7acb77aec43d49da8017410afae93822cc9cca98c4d4 \ --hash=sha256:cb52082e659e97afc5dac71e79de97d8681de3aa07ff18578330904a9d18e5b5 # via -r requirements.in -microvenv==2023.2.0 \ - --hash=sha256:5b46296d6a65992946da504bd9e724a5becf5c256091f2f9383e5b4e9f567f23 \ - --hash=sha256:a07e88a8fb5ee90219b86dd90095cb5646462d45d30285ea3b1a3c7cf33616d3 +microvenv==2023.3.post1 \ + --hash=sha256:67f0a48511cf16d6a2a45137175d0ddc36a657b91459b598cfbe976ef2afd596 \ + --hash=sha256:6e8c80ccfe813b00b77ab9cc2e5af3fd44e2fe540df176509fda97123f8b8290 # via -r requirements.in packaging==23.2 \ --hash=sha256:048fb0e9405036518eaaf48a55953c750c11e1a1b68e0dd1a9d62ed0c092cfc5 \ From ed155afa4bf6acdbd0341d093b5a00e13237985e Mon Sep 17 00:00:00 2001 From: Eleanor Boyd Date: Fri, 13 Oct 2023 16:24:39 -0700 Subject: [PATCH 06/15] remove asserts from catchable code for testing (#22210) some asserts were inside functions / mocking and with this then the extension code catches the exception and doesn't error out as the test. Bring the asserts out of the functions into the test so the asserts work as expected. --- .../testing/common/testingAdapter.test.ts | 50 ++++++++++++------- .../testing/common/testingPayloadsEot.test.ts | 14 ++++-- .../testController/server.unit.test.ts | 7 ++- .../testExecutionAdapter.unit.test.ts | 18 +++++-- 4 files changed, 62 insertions(+), 27 deletions(-) diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 519a60e3f0f7..a9ed25194fa9 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -653,17 +653,21 @@ suite('End to End Tests: test adapters', () => { if (data.error === undefined) { // Dereference a NULL pointer const indexOfTest = JSON.stringify(data).search('Dereference a NULL pointer'); - assert.notDeepEqual(indexOfTest, -1, 'Expected test to have a null pointer'); - } else { - assert.ok(data.error, "Expected errors in 'error' field"); + if (indexOfTest === -1) { + failureOccurred = true; + failureMsg = 'Expected test to have a null pointer'; + } + } else if (data.error.length === 0) { + failureOccurred = true; + failureMsg = "Expected errors in 'error' field"; } } else { const indexOfTest = JSON.stringify(data.tests).search('error'); - assert.notDeepEqual( - indexOfTest, - -1, - 'If payload status is not error then the individual tests should be marked as errors. This should occur on windows machines.', - ); + if (indexOfTest === -1) { + failureOccurred = true; + failureMsg = + 'If payload status is not error then the individual tests should be marked as errors. This should occur on windows machines.'; + } } } catch (err) { failureMsg = err ? (err as Error).toString() : ''; @@ -705,22 +709,32 @@ suite('End to End Tests: test adapters', () => { if (data.error === undefined) { // Dereference a NULL pointer const indexOfTest = JSON.stringify(data).search('Dereference a NULL pointer'); - assert.notDeepEqual(indexOfTest, -1, 'Expected test to have a null pointer'); - } else { - assert.ok(data.error, "Expected errors in 'error' field"); + if (indexOfTest === -1) { + failureOccurred = true; + failureMsg = 'Expected test to have a null pointer'; + } + } else if (data.error.length === 0) { + failureOccurred = true; + failureMsg = "Expected errors in 'error' field"; } } else { const indexOfTest = JSON.stringify(data.result).search('error'); - assert.notDeepEqual( - indexOfTest, - -1, - 'If payload status is not error then the individual tests should be marked as errors. This should occur on windows machines.', - ); + if (indexOfTest === -1) { + failureOccurred = true; + failureMsg = + 'If payload status is not error then the individual tests should be marked as errors. This should occur on windows machines.'; + } + } + if (data.result === undefined) { + failureOccurred = true; + failureMsg = 'Expected results to be present'; } - assert.ok(data.result, 'Expected results to be present'); // make sure the testID is found in the results const indexOfTest = JSON.stringify(data).search('test_seg_fault.TestSegmentationFault.test_segfault'); - assert.notDeepEqual(indexOfTest, -1, 'Expected testId to be present'); + if (indexOfTest === -1) { + failureOccurred = true; + failureMsg = 'Expected testId to be present'; + } } catch (err) { failureMsg = err ? (err as Error).toString() : ''; failureOccurred = true; diff --git a/src/test/testing/common/testingPayloadsEot.test.ts b/src/test/testing/common/testingPayloadsEot.test.ts index a30b1efe288c..2b8b9c0667df 100644 --- a/src/test/testing/common/testingPayloadsEot.test.ts +++ b/src/test/testing/common/testingPayloadsEot.test.ts @@ -165,13 +165,20 @@ suite('EOT tests', () => { mockProc.emit('close', 0, null); client.end(); }); - + let errorBool = false; + let errorMessage = ''; resultResolver = new PythonResultResolver(testController, PYTEST_PROVIDER, workspaceUri); resultResolver._resolveExecution = async (payload, _token?) => { // the payloads that get to the _resolveExecution are all data and should be successful. actualCollectedResult = actualCollectedResult + JSON.stringify(payload.result); - assert.strictEqual(payload.status, 'success', "Expected status to be 'success'"); - assert.ok(payload.result, 'Expected results to be present'); + if (payload.status !== 'success') { + errorBool = true; + errorMessage = "Expected status to be 'success'"; + } + if (!payload.result) { + errorBool = true; + errorMessage = 'Expected results to be present'; + } return Promise.resolve(); }; @@ -208,6 +215,7 @@ suite('EOT tests', () => { actualCollectedResult, "Expected collected result to match 'data'", ); + assert.strictEqual(errorBool, false, errorMessage); }); }); }); diff --git a/src/test/testing/testController/server.unit.test.ts b/src/test/testing/testController/server.unit.test.ts index 742492b33ba8..62f5b8327219 100644 --- a/src/test/testing/testController/server.unit.test.ts +++ b/src/test/testing/testController/server.unit.test.ts @@ -117,13 +117,15 @@ suite('Python Test Server, DataWithPayloadChunks', () => { const dataWithPayloadChunks = testCaseDataObj; await server.serverReady(); - + let errorOccur = false; + let errorMessage = ''; server.onRunDataReceived(({ data }) => { try { const resultData = JSON.parse(data).result; eventData = eventData + JSON.stringify(resultData); } catch (e) { - assert(false, 'Error parsing data'); + errorOccur = true; + errorMessage = 'Error parsing data'; } deferred.resolve(); }); @@ -143,6 +145,7 @@ suite('Python Test Server, DataWithPayloadChunks', () => { await deferred.promise; const expectedResult = dataWithPayloadChunks.data; assert.deepStrictEqual(eventData, expectedResult); + assert.deepStrictEqual(errorOccur, false, errorMessage); }); }); }); diff --git a/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts b/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts index 4d4a8d0ebee4..e2903d353bbf 100644 --- a/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts +++ b/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts @@ -31,12 +31,16 @@ suite('Unittest test execution adapter', () => { test('runTests should send the run command to the test server', async () => { let options: TestCommandOptions | undefined; - + let errorBool = false; + let errorMessage = ''; const stubTestServer = ({ sendCommand(opt: TestCommandOptions, runTestIdPort?: string): Promise { delete opt.outChannel; options = opt; - assert(runTestIdPort !== undefined); + if (runTestIdPort === undefined) { + errorBool = true; + errorMessage = 'runTestIdPort is undefined'; + } return Promise.resolve(); }, onRunDataReceived: () => { @@ -60,6 +64,7 @@ suite('Unittest test execution adapter', () => { testIds, }; assert.deepStrictEqual(options, expectedOptions); + assert.equal(errorBool, false, errorMessage); }); }); test('runTests should respect settings.testing.cwd when present', async () => { @@ -69,12 +74,16 @@ suite('Unittest test execution adapter', () => { }), } as unknown) as IConfigurationService; let options: TestCommandOptions | undefined; - + let errorBool = false; + let errorMessage = ''; const stubTestServer = ({ sendCommand(opt: TestCommandOptions, runTestIdPort?: string): Promise { delete opt.outChannel; options = opt; - assert(runTestIdPort !== undefined); + if (runTestIdPort === undefined) { + errorBool = true; + errorMessage = 'runTestIdPort is undefined'; + } return Promise.resolve(); }, onRunDataReceived: () => { @@ -99,6 +108,7 @@ suite('Unittest test execution adapter', () => { testIds, }; assert.deepStrictEqual(options, expectedOptions); + assert.equal(errorBool, false, errorMessage); }); }); }); From 1310bd665d83bcd4e09903bff39ac841dafcad52 Mon Sep 17 00:00:00 2001 From: Anthony Kim <62267334+anthonykim1@users.noreply.github.com> Date: Tue, 17 Oct 2023 00:00:05 -0700 Subject: [PATCH 07/15] Enable experiments for all tests (#22194) Closes: #22193 Enables to opt into experiments for tests such as single workspace, multi workspace, debugger, venv, etc. --- src/test/initialize.ts | 4 ++ src/test/smoke/smartSend.smoke.test.ts | 83 ++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/src/test/initialize.ts b/src/test/initialize.ts index add1d8624461..487860410bf0 100644 --- a/src/test/initialize.ts +++ b/src/test/initialize.ts @@ -31,6 +31,10 @@ export async function initializePython() { export async function initialize(): Promise { await initializePython(); + + const pythonConfig = vscode.workspace.getConfiguration('python'); + await pythonConfig.update('experiments.optInto', ['All'], vscode.ConfigurationTarget.Global); + await pythonConfig.update('experiments.optOutFrom', [], vscode.ConfigurationTarget.Global); const api = await activateExtension(); if (!IS_SMOKE_TEST) { // When running smoke tests, we won't have access to these. diff --git a/src/test/smoke/smartSend.smoke.test.ts b/src/test/smoke/smartSend.smoke.test.ts index e69de29bb2d1..20ec70af9b5b 100644 --- a/src/test/smoke/smartSend.smoke.test.ts +++ b/src/test/smoke/smartSend.smoke.test.ts @@ -0,0 +1,83 @@ +import * as vscode from 'vscode'; +import * as path from 'path'; +import * as fs from 'fs-extra'; +import { assert } from 'chai'; +import { EXTENSION_ROOT_DIR_FOR_TESTS, IS_SMOKE_TEST } from '../constants'; +import { closeActiveWindows, initialize, initializeTest } from '../initialize'; +import { openFile, waitForCondition } from '../common'; + +suite('Smoke Test: Run Smart Selection and Advance Cursor', () => { + suiteSetup(async function () { + if (!IS_SMOKE_TEST) { + return this.skip(); + } + await initialize(); + return undefined; + }); + + setup(initializeTest); + suiteTeardown(closeActiveWindows); + teardown(closeActiveWindows); + + test('Smart Send', async () => { + const file = path.join( + EXTENSION_ROOT_DIR_FOR_TESTS, + 'src', + 'testMultiRootWkspc', + 'smokeTests', + 'create_delete_file.py', + ); + const outputFile = path.join( + EXTENSION_ROOT_DIR_FOR_TESTS, + 'src', + 'testMultiRootWkspc', + 'smokeTests', + 'smart_send_smoke.txt', + ); + + await fs.remove(outputFile); + + const textDocument = await openFile(file); + + if (vscode.window.activeTextEditor) { + const myPos = new vscode.Position(0, 0); + vscode.window.activeTextEditor!.selections = [new vscode.Selection(myPos, myPos)]; + } + await vscode.commands + .executeCommand('python.execSelectionInTerminal', textDocument.uri) + .then(undefined, (err) => { + assert.fail(`Something went wrong running the Python file in the terminal: ${err}`); + }); + + const checkIfFileHasBeenCreated = () => fs.pathExists(outputFile); + await waitForCondition(checkIfFileHasBeenCreated, 10_000, `"${outputFile}" file not created`); + + await vscode.commands + .executeCommand('python.execSelectionInTerminal', textDocument.uri) + .then(undefined, (err) => { + assert.fail(`Something went wrong running the Python file in the terminal: ${err}`); + }); + await vscode.commands + .executeCommand('python.execSelectionInTerminal', textDocument.uri) + .then(undefined, (err) => { + assert.fail(`Something went wrong running the Python file in the terminal: ${err}`); + }); + + async function wait() { + return new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 10000); + }); + } + + await wait(); + + const deletedFile = !(await fs.pathExists(outputFile)); + if (deletedFile) { + assert.ok(true, `"${outputFile}" file has been deleted`); + } else { + assert.fail(`"${outputFile}" file still exists`); + } + }); +}); From f43826256703a40f76e1a93e677e72e5963689bc Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 17 Oct 2023 19:38:08 +1100 Subject: [PATCH 08/15] Add support for a tensorboard experiment (#22215) --- package.json | 6 +- package.nls.json | 1 + src/client/common/experiments/groups.ts | 5 + .../nbextensionCodeLensProvider.ts | 22 +- src/client/tensorBoard/serviceRegistry.ts | 2 + .../tensorBoard/tensorBoardFileWatcher.ts | 22 +- .../tensorBoardImportCodeLensProvider.ts | 22 +- .../tensorBoard/tensorBoardSessionProvider.ts | 28 ++- .../tensorBoard/tensorBoardUsageTracker.ts | 16 +- .../tensorBoard/tensorboarExperiment.ts | 65 +++++- .../tensorBoard/tensorboardIntegration.ts | 3 - src/client/tensorBoard/terminalWatcher.ts | 12 +- .../nbextensionCodeLensProvider.unit.test.ts | 85 ++++---- ...orBoardImportCodeLensProvider.unit.test.ts | 106 +++++----- .../tensorBoardPrompt.unit.test.ts | 2 +- .../tensorBoardUsageTracker.unit.test.ts | 191 ++++++++++-------- 16 files changed, 380 insertions(+), 208 deletions(-) diff --git a/package.json b/package.json index 20401cc43762..5b13f9eae0a3 100644 --- a/package.json +++ b/package.json @@ -537,7 +537,8 @@ "pythonPromptNewToolsExt", "pythonTerminalEnvVarActivation", "pythonTestAdapter", - "pythonREPLSmartSend" + "pythonREPLSmartSend", + "pythonRecommendTensorboardExt" ], "enumDescriptions": [ "%python.experiments.All.description%", @@ -545,7 +546,8 @@ "%python.experiments.pythonPromptNewToolsExt.description%", "%python.experiments.pythonTerminalEnvVarActivation.description%", "%python.experiments.pythonTestAdapter.description%", - "%python.experiments.pythonREPLSmartSend.description%" + "%python.experiments.pythonREPLSmartSend.description%", + "%python.experiments.pythonRecommendTensorboardExt.description%" ] }, "scope": "machine", diff --git a/package.nls.json b/package.nls.json index f843399e09c5..87692fb7c1a8 100644 --- a/package.nls.json +++ b/package.nls.json @@ -42,6 +42,7 @@ "python.experiments.pythonTerminalEnvVarActivation.description": "Enables use of environment variables to activate terminals instead of sending activation commands.", "python.experiments.pythonTestAdapter.description": "Denotes the Python Test Adapter experiment.", "python.experiments.pythonREPLSmartSend.description": "Denotes the Python REPL Smart Send experiment.", + "python.experiments.pythonRecommendTensorboardExt.description": "Denotes the Tensorboard Extension recommendation experiment.", "python.globalModuleInstallation.description": "Whether to install Python modules globally when not using an environment.", "python.languageServer.description": "Defines type of the language server.", "python.languageServer.defaultDescription": "Automatically select a language server: Pylance if installed and available, otherwise fallback to Jedi.", diff --git a/src/client/common/experiments/groups.ts b/src/client/common/experiments/groups.ts index b7a598e0a08a..29035bbc57fe 100644 --- a/src/client/common/experiments/groups.ts +++ b/src/client/common/experiments/groups.ts @@ -22,3 +22,8 @@ export enum EnableTestAdapterRewrite { export enum EnableREPLSmartSend { experiment = 'pythonREPLSmartSend', } + +// Experiment to recommend installing the tensorboard extension. +export enum RecommendTensobardExtension { + experiment = 'pythonRecommendTensorboardExt', +} diff --git a/src/client/tensorBoard/nbextensionCodeLensProvider.ts b/src/client/tensorBoard/nbextensionCodeLensProvider.ts index 7b9a116ee144..afaaf116851a 100644 --- a/src/client/tensorBoard/nbextensionCodeLensProvider.ts +++ b/src/client/tensorBoard/nbextensionCodeLensProvider.ts @@ -3,21 +3,23 @@ import { inject, injectable } from 'inversify'; import { once } from 'lodash'; -import { CancellationToken, CodeLens, Command, languages, Position, Range, TextDocument } from 'vscode'; +import { CancellationToken, CodeLens, Command, Disposable, languages, Position, Range, TextDocument } from 'vscode'; import { IExtensionSingleActivationService } from '../activation/types'; import { Commands, NotebookCellScheme, PYTHON_LANGUAGE } from '../common/constants'; -import { IDisposableRegistry } from '../common/types'; +import { IDisposable, IDisposableRegistry } from '../common/types'; import { TensorBoard } from '../common/utils/localize'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; import { TensorBoardEntrypoint, TensorBoardEntrypointTrigger } from './constants'; import { containsNotebookExtension } from './helpers'; -import { useNewTensorboardExtension } from './tensorboarExperiment'; +import { TensorboardExperiment } from './tensorboarExperiment'; @injectable() export class TensorBoardNbextensionCodeLensProvider implements IExtensionSingleActivationService { public readonly supportedWorkspaceTypes = { untrustedWorkspace: false, virtualWorkspace: false }; + private readonly disposables: IDisposable[] = []; + private sendTelemetryOnce = once( sendTelemetryEvent.bind(this, EventName.TENSORBOARD_ENTRYPOINT_SHOWN, undefined, { trigger: TensorBoardEntrypointTrigger.nbextension, @@ -25,12 +27,22 @@ export class TensorBoardNbextensionCodeLensProvider implements IExtensionSingleA }), ); - constructor(@inject(IDisposableRegistry) private disposables: IDisposableRegistry) {} + constructor( + @inject(IDisposableRegistry) disposables: IDisposableRegistry, + @inject(TensorboardExperiment) private readonly experiment: TensorboardExperiment, + ) { + disposables.push(this); + } + + public dispose(): void { + Disposable.from(...this.disposables).dispose(); + } public async activate(): Promise { - if (useNewTensorboardExtension()) { + if (TensorboardExperiment.isTensorboardExtensionInstalled) { return; } + this.experiment.disposeOnInstallingTensorboard(this); this.activateInternal().ignoreErrors(); } diff --git a/src/client/tensorBoard/serviceRegistry.ts b/src/client/tensorBoard/serviceRegistry.ts index dd193f528eea..5fedb7b6abf5 100644 --- a/src/client/tensorBoard/serviceRegistry.ts +++ b/src/client/tensorBoard/serviceRegistry.ts @@ -11,6 +11,7 @@ import { TensorBoardSessionProvider } from './tensorBoardSessionProvider'; import { TensorBoardNbextensionCodeLensProvider } from './nbextensionCodeLensProvider'; import { TerminalWatcher } from './terminalWatcher'; import { TensorboardDependencyChecker } from './tensorboardDependencyChecker'; +import { TensorboardExperiment } from './tensorboarExperiment'; export function registerTypes(serviceManager: IServiceManager): void { serviceManager.addSingleton(TensorBoardSessionProvider, TensorBoardSessionProvider); @@ -34,4 +35,5 @@ export function registerTypes(serviceManager: IServiceManager): void { serviceManager.addBinding(TensorBoardNbextensionCodeLensProvider, IExtensionSingleActivationService); serviceManager.addSingleton(IExtensionSingleActivationService, TerminalWatcher); serviceManager.addSingleton(TensorboardDependencyChecker, TensorboardDependencyChecker); + serviceManager.addSingleton(TensorboardExperiment, TensorboardExperiment); } diff --git a/src/client/tensorBoard/tensorBoardFileWatcher.ts b/src/client/tensorBoard/tensorBoardFileWatcher.ts index dccdb95290ec..f2f9344d7365 100644 --- a/src/client/tensorBoard/tensorBoardFileWatcher.ts +++ b/src/client/tensorBoard/tensorBoardFileWatcher.ts @@ -2,13 +2,13 @@ // Licensed under the MIT License. import { inject, injectable } from 'inversify'; -import { FileSystemWatcher, RelativePattern, WorkspaceFolder, WorkspaceFoldersChangeEvent } from 'vscode'; +import { Disposable, FileSystemWatcher, RelativePattern, WorkspaceFolder, WorkspaceFoldersChangeEvent } from 'vscode'; import { IExtensionSingleActivationService } from '../activation/types'; import { IWorkspaceService } from '../common/application/types'; -import { IDisposableRegistry } from '../common/types'; +import { IDisposable, IDisposableRegistry } from '../common/types'; import { TensorBoardEntrypointTrigger } from './constants'; import { TensorBoardPrompt } from './tensorBoardPrompt'; -import { useNewTensorboardExtension } from './tensorboarExperiment'; +import { TensorboardExperiment } from './tensorboarExperiment'; @injectable() export class TensorBoardFileWatcher implements IExtensionSingleActivationService { @@ -18,16 +18,26 @@ export class TensorBoardFileWatcher implements IExtensionSingleActivationService private globPatterns = ['*tfevents*', '*/*tfevents*', '*/*/*tfevents*']; + private readonly disposables: IDisposable[] = []; + constructor( @inject(IWorkspaceService) private workspaceService: IWorkspaceService, @inject(TensorBoardPrompt) private tensorBoardPrompt: TensorBoardPrompt, - @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, - ) {} + @inject(IDisposableRegistry) disposables: IDisposableRegistry, + @inject(TensorboardExperiment) private readonly experiment: TensorboardExperiment, + ) { + disposables.push(this); + } + + public dispose(): void { + Disposable.from(...this.disposables).dispose(); + } public async activate(): Promise { - if (useNewTensorboardExtension()) { + if (TensorboardExperiment.isTensorboardExtensionInstalled) { return; } + this.experiment.disposeOnInstallingTensorboard(this); this.activateInternal().ignoreErrors(); } diff --git a/src/client/tensorBoard/tensorBoardImportCodeLensProvider.ts b/src/client/tensorBoard/tensorBoardImportCodeLensProvider.ts index d6dc8d7e82e5..585b9151922a 100644 --- a/src/client/tensorBoard/tensorBoardImportCodeLensProvider.ts +++ b/src/client/tensorBoard/tensorBoardImportCodeLensProvider.ts @@ -3,16 +3,16 @@ import { inject, injectable } from 'inversify'; import { once } from 'lodash'; -import { CancellationToken, CodeLens, Command, languages, Position, Range, TextDocument } from 'vscode'; +import { CancellationToken, CodeLens, Command, Disposable, languages, Position, Range, TextDocument } from 'vscode'; import { IExtensionSingleActivationService } from '../activation/types'; import { Commands, PYTHON } from '../common/constants'; -import { IDisposableRegistry } from '../common/types'; +import { IDisposable, IDisposableRegistry } from '../common/types'; import { TensorBoard } from '../common/utils/localize'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; import { TensorBoardEntrypoint, TensorBoardEntrypointTrigger } from './constants'; import { containsTensorBoardImport } from './helpers'; -import { useNewTensorboardExtension } from './tensorboarExperiment'; +import { TensorboardExperiment } from './tensorboarExperiment'; @injectable() export class TensorBoardImportCodeLensProvider implements IExtensionSingleActivationService { @@ -25,12 +25,24 @@ export class TensorBoardImportCodeLensProvider implements IExtensionSingleActiva }), ); - constructor(@inject(IDisposableRegistry) private disposables: IDisposableRegistry) {} + private readonly disposables: IDisposable[] = []; + + constructor( + @inject(IDisposableRegistry) disposables: IDisposableRegistry, + @inject(TensorboardExperiment) private readonly experiment: TensorboardExperiment, + ) { + disposables.push(this); + } + + public dispose(): void { + Disposable.from(...this.disposables).dispose(); + } public async activate(): Promise { - if (useNewTensorboardExtension()) { + if (TensorboardExperiment.isTensorboardExtensionInstalled) { return; } + this.experiment.disposeOnInstallingTensorboard(this); this.activateInternal().ignoreErrors(); } diff --git a/src/client/tensorBoard/tensorBoardSessionProvider.ts b/src/client/tensorBoard/tensorBoardSessionProvider.ts index c81059654075..ec52b9ef94dc 100644 --- a/src/client/tensorBoard/tensorBoardSessionProvider.ts +++ b/src/client/tensorBoard/tensorBoardSessionProvider.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { inject, injectable } from 'inversify'; -import { l10n, ViewColumn } from 'vscode'; +import { Disposable, l10n, ViewColumn } from 'vscode'; import { IExtensionSingleActivationService } from '../activation/types'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../common/application/types'; import { Commands } from '../common/constants'; @@ -14,6 +14,7 @@ import { IPersistentState, IPersistentStateFactory, IConfigurationService, + IDisposable, } from '../common/types'; import { IMultiStepInputFactory } from '../common/utils/multiStepInput'; import { IInterpreterService } from '../interpreter/contracts'; @@ -22,7 +23,7 @@ import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; import { TensorBoardEntrypoint, TensorBoardEntrypointTrigger } from './constants'; import { TensorBoardSession } from './tensorBoardSession'; -import { useNewTensorboardExtension } from './tensorboarExperiment'; +import { TensorboardExperiment } from './tensorboarExperiment'; export const PREFERRED_VIEWGROUP = 'PythonTensorBoardWebviewPreferredViewGroup'; @@ -36,18 +37,22 @@ export class TensorBoardSessionProvider implements IExtensionSingleActivationSer private hasActiveTensorBoardSessionContext: ContextKey; + private readonly disposables: IDisposable[] = []; + constructor( @inject(IInstaller) private readonly installer: IInstaller, @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, @inject(IApplicationShell) private readonly applicationShell: IApplicationShell, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, @inject(ICommandManager) private readonly commandManager: ICommandManager, - @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, + @inject(IDisposableRegistry) disposables: IDisposableRegistry, @inject(IPythonExecutionFactory) private readonly pythonExecFactory: IPythonExecutionFactory, @inject(IPersistentStateFactory) private stateFactory: IPersistentStateFactory, @inject(IMultiStepInputFactory) private readonly multiStepFactory: IMultiStepInputFactory, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, + @inject(TensorboardExperiment) private readonly experiment: TensorboardExperiment, ) { + disposables.push(this); this.preferredViewGroupMemento = this.stateFactory.createGlobalPersistentState( PREFERRED_VIEWGROUP, ViewColumn.Active, @@ -58,10 +63,15 @@ export class TensorBoardSessionProvider implements IExtensionSingleActivationSer ); } + public dispose(): void { + Disposable.from(...this.disposables).dispose(); + } + public async activate(): Promise { - if (useNewTensorboardExtension()) { + if (TensorboardExperiment.isTensorboardExtensionInstalled) { return; } + this.experiment.disposeOnInstallingTensorboard(this); this.disposables.push( this.commandManager.registerCommand( @@ -69,16 +79,20 @@ export class TensorBoardSessionProvider implements IExtensionSingleActivationSer ( entrypoint: TensorBoardEntrypoint = TensorBoardEntrypoint.palette, trigger: TensorBoardEntrypointTrigger = TensorBoardEntrypointTrigger.palette, - ) => { + ): void => { sendTelemetryEvent(EventName.TENSORBOARD_SESSION_LAUNCH, undefined, { trigger, entrypoint, }); - return this.createNewSession(); + if (this.experiment.recommendAndUseNewExtension() === 'continueWithPythonExtension') { + void this.createNewSession(); + } }, ), this.commandManager.registerCommand(Commands.RefreshTensorBoard, () => - this.knownSessions.map((w) => w.refresh()), + this.experiment.recommendAndUseNewExtension() === 'continueWithPythonExtension' + ? this.knownSessions.map((w) => w.refresh()) + : undefined, ), ); } diff --git a/src/client/tensorBoard/tensorBoardUsageTracker.ts b/src/client/tensorBoard/tensorBoardUsageTracker.ts index 7c8ea7b00961..b88e416a113f 100644 --- a/src/client/tensorBoard/tensorBoardUsageTracker.ts +++ b/src/client/tensorBoard/tensorBoardUsageTracker.ts @@ -3,7 +3,7 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; -import { TextEditor } from 'vscode'; +import { Disposable, TextEditor } from 'vscode'; import { IExtensionSingleActivationService } from '../activation/types'; import { IDocumentManager } from '../common/application/types'; import { isTestExecution } from '../common/constants'; @@ -12,7 +12,7 @@ import { getDocumentLines } from '../telemetry/importTracker'; import { TensorBoardEntrypointTrigger } from './constants'; import { containsTensorBoardImport } from './helpers'; import { TensorBoardPrompt } from './tensorBoardPrompt'; -import { useNewTensorboardExtension } from './tensorboarExperiment'; +import { TensorboardExperiment } from './tensorboarExperiment'; const testExecution = isTestExecution(); @@ -26,12 +26,20 @@ export class TensorBoardUsageTracker implements IExtensionSingleActivationServic @inject(IDocumentManager) private documentManager: IDocumentManager, @inject(IDisposableRegistry) private disposables: IDisposableRegistry, @inject(TensorBoardPrompt) private prompt: TensorBoardPrompt, - ) {} + @inject(TensorboardExperiment) private readonly experiment: TensorboardExperiment, + ) { + disposables.push(this); + } + + public dispose(): void { + Disposable.from(...this.disposables).dispose(); + } public async activate(): Promise { - if (useNewTensorboardExtension()) { + if (TensorboardExperiment.isTensorboardExtensionInstalled) { return; } + this.experiment.disposeOnInstallingTensorboard(this); if (testExecution) { await this.activateInternal(); } else { diff --git a/src/client/tensorBoard/tensorboarExperiment.ts b/src/client/tensorBoard/tensorboarExperiment.ts index 25eac8db71da..3cf4cb3c779a 100644 --- a/src/client/tensorBoard/tensorboarExperiment.ts +++ b/src/client/tensorBoard/tensorboarExperiment.ts @@ -1,8 +1,67 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { extensions } from 'vscode'; +import { Disposable, EventEmitter, commands, extensions, l10n, window } from 'vscode'; +import { inject, injectable } from 'inversify'; +import { IDisposable, IDisposableRegistry, IExperimentService } from '../common/types'; +import { RecommendTensobardExtension } from '../common/experiments/groups'; +import { TENSORBOARD_EXTENSION_ID } from '../common/constants'; -export function useNewTensorboardExtension(): boolean { - return !!extensions.getExtension('ms-toolsai.tensorboard'); +@injectable() +export class TensorboardExperiment { + private readonly _onDidChange = new EventEmitter(); + + public readonly onDidChange = this._onDidChange.event; + + private readonly toDisposeWhenTensobardIsInstalled: IDisposable[] = []; + + public static get isTensorboardExtensionInstalled(): boolean { + return !!extensions.getExtension(TENSORBOARD_EXTENSION_ID); + } + + private readonly isExperimentEnabled: boolean; + + constructor( + @inject(IDisposableRegistry) disposables: IDisposableRegistry, + @inject(IExperimentService) experiments: IExperimentService, + ) { + this.isExperimentEnabled = experiments.inExperimentSync(RecommendTensobardExtension.experiment); + disposables.push(this._onDidChange); + extensions.onDidChange( + () => + TensorboardExperiment.isTensorboardExtensionInstalled + ? Disposable.from(...this.toDisposeWhenTensobardIsInstalled).dispose() + : undefined, + this, + disposables, + ); + } + + public recommendAndUseNewExtension(): 'continueWithPythonExtension' | 'usingTensorboardExtension' { + if (!this.isExperimentEnabled) { + return 'continueWithPythonExtension'; + } + if (TensorboardExperiment.isTensorboardExtensionInstalled) { + return 'usingTensorboardExtension'; + } + const install = l10n.t('Install Tensorboard Extension'); + window + .showInformationMessage( + l10n.t( + 'Install the TensorBoard extension to use the this functionality. Once installed, select the command `Launch Tensorboard`.', + ), + { modal: true }, + install, + ) + .then((result): void => { + if (result === install) { + void commands.executeCommand('workbench.extensions.installExtension', TENSORBOARD_EXTENSION_ID); + } + }); + return 'usingTensorboardExtension'; + } + + public disposeOnInstallingTensorboard(disposabe: IDisposable): void { + this.toDisposeWhenTensobardIsInstalled.push(disposabe); + } } diff --git a/src/client/tensorBoard/tensorboardIntegration.ts b/src/client/tensorBoard/tensorboardIntegration.ts index 74f69afab84f..22d590d6ee65 100644 --- a/src/client/tensorBoard/tensorboardIntegration.ts +++ b/src/client/tensorBoard/tensorboardIntegration.ts @@ -69,10 +69,7 @@ export class TensorboardExtensionIntegration { public hideCommands(): void { if (this.extensions.getExtension(TENSORBOARD_EXTENSION_ID)) { - console.error('TensorBoard extension is installed'); void commands.executeCommand('setContext', 'python.tensorboardExtInstalled', true); - } else { - console.error('TensorBoard extension not installed'); } } diff --git a/src/client/tensorBoard/terminalWatcher.ts b/src/client/tensorBoard/terminalWatcher.ts index 30ccf7e1726a..5f48def54e43 100644 --- a/src/client/tensorBoard/terminalWatcher.ts +++ b/src/client/tensorBoard/terminalWatcher.ts @@ -4,7 +4,7 @@ import { IExtensionSingleActivationService } from '../activation/types'; import { IDisposable, IDisposableRegistry } from '../common/types'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; -import { useNewTensorboardExtension } from './tensorboarExperiment'; +import { TensorboardExperiment } from './tensorboarExperiment'; // Every 5 min look, through active terminals to see if any are running `tensorboard` @injectable() @@ -13,12 +13,18 @@ export class TerminalWatcher implements IExtensionSingleActivationService, IDisp private handle: NodeJS.Timeout | undefined; - constructor(@inject(IDisposableRegistry) private disposables: IDisposableRegistry) {} + constructor( + @inject(IDisposableRegistry) private disposables: IDisposableRegistry, + @inject(TensorboardExperiment) private readonly experiment: TensorboardExperiment, + ) { + disposables.push(this); + } public async activate(): Promise { - if (useNewTensorboardExtension()) { + if (TensorboardExperiment.isTensorboardExtensionInstalled) { return; } + this.experiment.disposeOnInstallingTensorboard(this); const handle = setInterval(() => { // When user runs a command in VSCode terminal, the terminal's name // becomes the program that is currently running. Since tensorboard diff --git a/src/test/tensorBoard/nbextensionCodeLensProvider.unit.test.ts b/src/test/tensorBoard/nbextensionCodeLensProvider.unit.test.ts index 9a46d92c1422..aef90d14eacf 100644 --- a/src/test/tensorBoard/nbextensionCodeLensProvider.unit.test.ts +++ b/src/test/tensorBoard/nbextensionCodeLensProvider.unit.test.ts @@ -1,49 +1,60 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import * as sinon from 'sinon'; import { assert } from 'chai'; import { CancellationTokenSource } from 'vscode'; +import { instance, mock } from 'ts-mockito'; import { TensorBoardNbextensionCodeLensProvider } from '../../client/tensorBoard/nbextensionCodeLensProvider'; import { MockDocument } from '../mocks/mockDocument'; +import { TensorboardExperiment } from '../../client/tensorBoard/tensorboarExperiment'; -suite('TensorBoard nbextension code lens provider', () => { - let codeLensProvider: TensorBoardNbextensionCodeLensProvider; - let cancelTokenSource: CancellationTokenSource; +[true, false].forEach((tbExtensionInstalled) => { + suite(`Tensorboard Extension is ${tbExtensionInstalled ? 'installed' : 'not installed'}`, () => { + suite.only('TensorBoard nbextension code lens provider', () => { + let experiment: TensorboardExperiment; + let codeLensProvider: TensorBoardNbextensionCodeLensProvider; + let cancelTokenSource: CancellationTokenSource; - setup(() => { - codeLensProvider = new TensorBoardNbextensionCodeLensProvider([]); - cancelTokenSource = new CancellationTokenSource(); - }); - teardown(() => { - cancelTokenSource.dispose(); - }); + setup(() => { + sinon.stub(TensorboardExperiment, 'isTensorboardExtensionInstalled').returns(tbExtensionInstalled); + experiment = mock(); + codeLensProvider = new TensorBoardNbextensionCodeLensProvider([], instance(experiment)); + cancelTokenSource = new CancellationTokenSource(); + }); + teardown(() => { + sinon.restore(); + cancelTokenSource.dispose(); + }); - test('Provide code lens for Python notebook loading tensorboard nbextension', async () => { - const document = new MockDocument('a=1\n%load_ext tensorboard', 'foo.ipynb', async () => true); - const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); - assert.ok(codeLens.length > 0, 'Failed to provide code lens for file loading tensorboard nbextension'); - }); - test('Provide code lens for Python notebook launching tensorboard nbextension', async () => { - const document = new MockDocument('a=1\n%tensorboard --logdir logs/fit', 'foo.ipynb', async () => true); - const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); - assert.ok(codeLens.length > 0, 'Failed to provide code lens for file loading tensorboard nbextension'); - }); - test('Fails when cancellation is signaled', () => { - const document = new MockDocument('a=1\n%tensorboard --logdir logs/fit', 'foo.ipynb', async () => true); - cancelTokenSource.cancel(); - const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); - assert.ok(codeLens.length === 0, 'Provided codelens even after cancellation was requested'); + test('Provide code lens for Python notebook loading tensorboard nbextension', async () => { + const document = new MockDocument('a=1\n%load_ext tensorboard', 'foo.ipynb', async () => true); + const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); + assert.ok(codeLens.length > 0, 'Failed to provide code lens for file loading tensorboard nbextension'); + }); + test('Provide code lens for Python notebook launching tensorboard nbextension', async () => { + const document = new MockDocument('a=1\n%tensorboard --logdir logs/fit', 'foo.ipynb', async () => true); + const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); + assert.ok(codeLens.length > 0, 'Failed to provide code lens for file loading tensorboard nbextension'); + }); + test('Fails when cancellation is signaled', () => { + const document = new MockDocument('a=1\n%tensorboard --logdir logs/fit', 'foo.ipynb', async () => true); + cancelTokenSource.cancel(); + const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); + assert.ok(codeLens.length === 0, 'Provided codelens even after cancellation was requested'); + }); + // Can't verify these cases without running in vscode as we depend on vscode to not call us + // based on the DocumentSelector we provided. See nbExtensionCodeLensProvider.test.ts for that. + // test('Does not provide code lens for Python file loading tensorboard nbextension', async () => { + // const document = new MockDocument('a=1\n%load_ext tensorboard', 'foo.py', async () => true); + // const codeLens = codeLensProvider.provideCodeLenses(document); + // assert.ok(codeLens.length === 0, 'Provided code lens for Python file loading tensorboard nbextension'); + // }); + // test('Does not provide code lens for Python file launching tensorboard nbextension', async () => { + // const document = new MockDocument('a=1\n%tensorboard --logdir logs/fit', 'foo.py', async () => true); + // const codeLens = codeLensProvider.provideCodeLenses(document); + // assert.ok(codeLens.length === 0, 'Provided code lens for Python file loading tensorboard nbextension'); + // }); + }); }); - // Can't verify these cases without running in vscode as we depend on vscode to not call us - // based on the DocumentSelector we provided. See nbExtensionCodeLensProvider.test.ts for that. - // test('Does not provide code lens for Python file loading tensorboard nbextension', async () => { - // const document = new MockDocument('a=1\n%load_ext tensorboard', 'foo.py', async () => true); - // const codeLens = codeLensProvider.provideCodeLenses(document); - // assert.ok(codeLens.length === 0, 'Provided code lens for Python file loading tensorboard nbextension'); - // }); - // test('Does not provide code lens for Python file launching tensorboard nbextension', async () => { - // const document = new MockDocument('a=1\n%tensorboard --logdir logs/fit', 'foo.py', async () => true); - // const codeLens = codeLensProvider.provideCodeLenses(document); - // assert.ok(codeLens.length === 0, 'Provided code lens for Python file loading tensorboard nbextension'); - // }); }); diff --git a/src/test/tensorBoard/tensorBoardImportCodeLensProvider.unit.test.ts b/src/test/tensorBoard/tensorBoardImportCodeLensProvider.unit.test.ts index 9b691c9af17c..07bcce035a7c 100644 --- a/src/test/tensorBoard/tensorBoardImportCodeLensProvider.unit.test.ts +++ b/src/test/tensorBoard/tensorBoardImportCodeLensProvider.unit.test.ts @@ -1,58 +1,72 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import * as sinon from 'sinon'; import { assert } from 'chai'; import { CancellationTokenSource } from 'vscode'; +import { instance, mock } from 'ts-mockito'; import { TensorBoardImportCodeLensProvider } from '../../client/tensorBoard/tensorBoardImportCodeLensProvider'; import { MockDocument } from '../mocks/mockDocument'; +import { TensorboardExperiment } from '../../client/tensorBoard/tensorboarExperiment'; -suite('TensorBoard import code lens provider', () => { - let codeLensProvider: TensorBoardImportCodeLensProvider; - let cancelTokenSource: CancellationTokenSource; +[true, false].forEach((tbExtensionInstalled) => { + suite(`Tensorboard Extension is ${tbExtensionInstalled ? 'installed' : 'not installed'}`, () => { + suite.only('TensorBoard import code lens provider', () => { + let experiment: TensorboardExperiment; + let codeLensProvider: TensorBoardImportCodeLensProvider; + let cancelTokenSource: CancellationTokenSource; - setup(() => { - codeLensProvider = new TensorBoardImportCodeLensProvider([]); - cancelTokenSource = new CancellationTokenSource(); - }); - teardown(() => { - cancelTokenSource.dispose(); - }); - [ - 'import tensorboard', - 'import foo, tensorboard', - 'import foo, tensorboard, bar', - 'import tensorboardX', - 'import tensorboardX, bar', - 'import torch.profiler', - 'import foo, torch.profiler', - 'from torch.utils import tensorboard', - 'from torch.utils import foo, tensorboard', - 'import torch.utils.tensorboard, foo', - 'from torch import profiler', - ].forEach((importStatement) => { - test(`Provides code lens for Python files containing ${importStatement}`, () => { - const document = new MockDocument(importStatement, 'foo.py', async () => true); - const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); - assert.ok(codeLens.length > 0, `Failed to provide code lens for file containing ${importStatement} import`); - }); - test(`Provides code lens for Python ipynbs containing ${importStatement}`, () => { - const document = new MockDocument(importStatement, 'foo.ipynb', async () => true); - const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); - assert.ok( - codeLens.length > 0, - `Failed to provide code lens for ipynb containing ${importStatement} import`, - ); + setup(() => { + sinon.stub(TensorboardExperiment, 'isTensorboardExtensionInstalled').returns(tbExtensionInstalled); + experiment = mock(); + codeLensProvider = new TensorBoardImportCodeLensProvider([], instance(experiment)); + cancelTokenSource = new CancellationTokenSource(); + }); + teardown(() => { + sinon.restore(); + cancelTokenSource.dispose(); + }); + [ + 'import tensorboard', + 'import foo, tensorboard', + 'import foo, tensorboard, bar', + 'import tensorboardX', + 'import tensorboardX, bar', + 'import torch.profiler', + 'import foo, torch.profiler', + 'from torch.utils import tensorboard', + 'from torch.utils import foo, tensorboard', + 'import torch.utils.tensorboard, foo', + 'from torch import profiler', + ].forEach((importStatement) => { + test(`Provides code lens for Python files containing ${importStatement}`, () => { + const document = new MockDocument(importStatement, 'foo.py', async () => true); + const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); + assert.ok( + codeLens.length > 0, + `Failed to provide code lens for file containing ${importStatement} import`, + ); + }); + test(`Provides code lens for Python ipynbs containing ${importStatement}`, () => { + const document = new MockDocument(importStatement, 'foo.ipynb', async () => true); + const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); + assert.ok( + codeLens.length > 0, + `Failed to provide code lens for ipynb containing ${importStatement} import`, + ); + }); + test('Fails when cancellation is signaled', () => { + const document = new MockDocument(importStatement, 'foo.py', async () => true); + cancelTokenSource.cancel(); + const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); + assert.ok(codeLens.length === 0, 'Provided codelens even after cancellation was requested'); + }); + }); + test('Does not provide code lens if no matching import', () => { + const document = new MockDocument('import foo', 'foo.ipynb', async () => true); + const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); + assert.ok(codeLens.length === 0, 'Provided code lens for file without tensorboard import'); + }); }); - test('Fails when cancellation is signaled', () => { - const document = new MockDocument(importStatement, 'foo.py', async () => true); - cancelTokenSource.cancel(); - const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); - assert.ok(codeLens.length === 0, 'Provided codelens even after cancellation was requested'); - }); - }); - test('Does not provide code lens if no matching import', () => { - const document = new MockDocument('import foo', 'foo.ipynb', async () => true); - const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token); - assert.ok(codeLens.length === 0, 'Provided code lens for file without tensorboard import'); }); }); diff --git a/src/test/tensorBoard/tensorBoardPrompt.unit.test.ts b/src/test/tensorBoard/tensorBoardPrompt.unit.test.ts index 6f096e560d70..d94b0d6c5f23 100644 --- a/src/test/tensorBoard/tensorBoardPrompt.unit.test.ts +++ b/src/test/tensorBoard/tensorBoardPrompt.unit.test.ts @@ -7,7 +7,7 @@ import { Common } from '../../client/common/utils/localize'; import { TensorBoardEntrypointTrigger } from '../../client/tensorBoard/constants'; import { TensorBoardPrompt } from '../../client/tensorBoard/tensorBoardPrompt'; -suite('TensorBoard prompt', () => { +suite.only('TensorBoard prompt', () => { let applicationShell: ApplicationShell; let commandManager: CommandManager; let persistentState: PersistentState; diff --git a/src/test/tensorBoard/tensorBoardUsageTracker.unit.test.ts b/src/test/tensorBoard/tensorBoardUsageTracker.unit.test.ts index ff187dd2afc1..54771ab4b6b6 100644 --- a/src/test/tensorBoard/tensorBoardUsageTracker.unit.test.ts +++ b/src/test/tensorBoard/tensorBoardUsageTracker.unit.test.ts @@ -1,98 +1,117 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; -import { anything, reset, when } from 'ts-mockito'; +import { anything, instance, mock, reset, when } from 'ts-mockito'; import { TensorBoardUsageTracker } from '../../client/tensorBoard/tensorBoardUsageTracker'; import { TensorBoardPrompt } from '../../client/tensorBoard/tensorBoardPrompt'; import { MockDocumentManager } from '../mocks/mockDocumentManager'; import { createTensorBoardPromptWithMocks } from './helpers'; import { mockedVSCodeNamespaces } from '../vscode-mock'; +import { TensorboardExperiment } from '../../client/tensorBoard/tensorboarExperiment'; -suite('TensorBoard usage tracker', () => { - let documentManager: MockDocumentManager; - let tensorBoardImportTracker: TensorBoardUsageTracker; - let prompt: TensorBoardPrompt; - let showNativeTensorBoardPrompt: sinon.SinonSpy; +[true, false].forEach((tbExtensionInstalled) => { + suite(`Tensorboard Extension is ${tbExtensionInstalled ? 'installed' : 'not installed'}`, () => { + suite.only('TensorBoard usage tracker', () => { + let experiment: TensorboardExperiment; + let documentManager: MockDocumentManager; + let tensorBoardImportTracker: TensorBoardUsageTracker; + let prompt: TensorBoardPrompt; + let showNativeTensorBoardPrompt: sinon.SinonSpy; - suiteSetup(() => { - reset(mockedVSCodeNamespaces.extensions); - when(mockedVSCodeNamespaces.extensions?.getExtension(anything())).thenReturn(undefined); - }); - suiteTeardown(() => reset(mockedVSCodeNamespaces.extensions)); - setup(() => { - documentManager = new MockDocumentManager(); - prompt = createTensorBoardPromptWithMocks(); - showNativeTensorBoardPrompt = sinon.spy(prompt, 'showNativeTensorBoardPrompt'); - tensorBoardImportTracker = new TensorBoardUsageTracker(documentManager, [], prompt); - }); + suiteSetup(() => { + reset(mockedVSCodeNamespaces.extensions); + when(mockedVSCodeNamespaces.extensions?.getExtension(anything())).thenReturn(undefined); + }); + suiteTeardown(() => reset(mockedVSCodeNamespaces.extensions)); + setup(() => { + sinon.stub(TensorboardExperiment, 'isTensorboardExtensionInstalled').returns(tbExtensionInstalled); + experiment = mock(); + documentManager = new MockDocumentManager(); + prompt = createTensorBoardPromptWithMocks(); + showNativeTensorBoardPrompt = sinon.spy(prompt, 'showNativeTensorBoardPrompt'); + tensorBoardImportTracker = new TensorBoardUsageTracker( + documentManager, + [], + prompt, + instance(experiment), + ); + }); - test('Simple tensorboard import in Python file', async () => { - const document = documentManager.addDocument('import tensorboard', 'foo.py'); - await documentManager.showTextDocument(document); - await tensorBoardImportTracker.activate(); - assert.ok(showNativeTensorBoardPrompt.calledOnce); - }); - test('Simple tensorboardX import in Python file', async () => { - const document = documentManager.addDocument('import tensorboardX', 'foo.py'); - await documentManager.showTextDocument(document); - await tensorBoardImportTracker.activate(); - assert.ok(showNativeTensorBoardPrompt.calledOnce); - }); - test('Simple tensorboard import in Python ipynb', async () => { - const document = documentManager.addDocument('import tensorboard', 'foo.ipynb'); - await documentManager.showTextDocument(document); - await tensorBoardImportTracker.activate(); - assert.ok(showNativeTensorBoardPrompt.calledOnce); - }); - test('`from x.y.tensorboard import z` import', async () => { - const document = documentManager.addDocument('from torch.utils.tensorboard import SummaryWriter', 'foo.py'); - await documentManager.showTextDocument(document); - await tensorBoardImportTracker.activate(); - assert.ok(showNativeTensorBoardPrompt.calledOnce); - }); - test('`from x.y import tensorboard` import', async () => { - const document = documentManager.addDocument('from torch.utils import tensorboard', 'foo.py'); - await documentManager.showTextDocument(document); - await tensorBoardImportTracker.activate(); - assert.ok(showNativeTensorBoardPrompt.calledOnce); - }); - test('`from tensorboardX import x` import', async () => { - const document = documentManager.addDocument('from tensorboardX import SummaryWriter', 'foo.py'); - await documentManager.showTextDocument(document); - await tensorBoardImportTracker.activate(); - assert.ok(showNativeTensorBoardPrompt.calledOnce); - }); - test('`import x, y` import', async () => { - const document = documentManager.addDocument('import tensorboard, tensorflow', 'foo.py'); - await documentManager.showTextDocument(document); - await tensorBoardImportTracker.activate(); - assert.ok(showNativeTensorBoardPrompt.calledOnce); - }); - test('`import pkg as _` import', async () => { - const document = documentManager.addDocument('import tensorboard as tb', 'foo.py'); - await documentManager.showTextDocument(document); - await tensorBoardImportTracker.activate(); - assert.ok(showNativeTensorBoardPrompt.calledOnce); - }); - test('Show prompt on changed text editor', async () => { - await tensorBoardImportTracker.activate(); - const document = documentManager.addDocument('import tensorboard as tb', 'foo.py'); - await documentManager.showTextDocument(document); - assert.ok(showNativeTensorBoardPrompt.calledOnce); - }); - test('Do not show prompt if no tensorboard import', async () => { - const document = documentManager.addDocument('import tensorflow as tf\nfrom torch.utils import foo', 'foo.py'); - await documentManager.showTextDocument(document); - await tensorBoardImportTracker.activate(); - assert.ok(showNativeTensorBoardPrompt.notCalled); - }); - test('Do not show prompt if language is not Python', async () => { - const document = documentManager.addDocument( - 'import tensorflow as tf\nfrom torch.utils import foo', - 'foo.cpp', - 'cpp', - ); - await documentManager.showTextDocument(document); - await tensorBoardImportTracker.activate(); - assert.ok(showNativeTensorBoardPrompt.notCalled); + test('Simple tensorboard import in Python file', async () => { + const document = documentManager.addDocument('import tensorboard', 'foo.py'); + await documentManager.showTextDocument(document); + await tensorBoardImportTracker.activate(); + assert.ok(showNativeTensorBoardPrompt.calledOnce); + }); + test('Simple tensorboardX import in Python file', async () => { + const document = documentManager.addDocument('import tensorboardX', 'foo.py'); + await documentManager.showTextDocument(document); + await tensorBoardImportTracker.activate(); + assert.ok(showNativeTensorBoardPrompt.calledOnce); + }); + test('Simple tensorboard import in Python ipynb', async () => { + const document = documentManager.addDocument('import tensorboard', 'foo.ipynb'); + await documentManager.showTextDocument(document); + await tensorBoardImportTracker.activate(); + assert.ok(showNativeTensorBoardPrompt.calledOnce); + }); + test('`from x.y.tensorboard import z` import', async () => { + const document = documentManager.addDocument( + 'from torch.utils.tensorboard import SummaryWriter', + 'foo.py', + ); + await documentManager.showTextDocument(document); + await tensorBoardImportTracker.activate(); + assert.ok(showNativeTensorBoardPrompt.calledOnce); + }); + test('`from x.y import tensorboard` import', async () => { + const document = documentManager.addDocument('from torch.utils import tensorboard', 'foo.py'); + await documentManager.showTextDocument(document); + await tensorBoardImportTracker.activate(); + assert.ok(showNativeTensorBoardPrompt.calledOnce); + }); + test('`from tensorboardX import x` import', async () => { + const document = documentManager.addDocument('from tensorboardX import SummaryWriter', 'foo.py'); + await documentManager.showTextDocument(document); + await tensorBoardImportTracker.activate(); + assert.ok(showNativeTensorBoardPrompt.calledOnce); + }); + test('`import x, y` import', async () => { + const document = documentManager.addDocument('import tensorboard, tensorflow', 'foo.py'); + await documentManager.showTextDocument(document); + await tensorBoardImportTracker.activate(); + assert.ok(showNativeTensorBoardPrompt.calledOnce); + }); + test('`import pkg as _` import', async () => { + const document = documentManager.addDocument('import tensorboard as tb', 'foo.py'); + await documentManager.showTextDocument(document); + await tensorBoardImportTracker.activate(); + assert.ok(showNativeTensorBoardPrompt.calledOnce); + }); + test('Show prompt on changed text editor', async () => { + await tensorBoardImportTracker.activate(); + const document = documentManager.addDocument('import tensorboard as tb', 'foo.py'); + await documentManager.showTextDocument(document); + assert.ok(showNativeTensorBoardPrompt.calledOnce); + }); + test('Do not show prompt if no tensorboard import', async () => { + const document = documentManager.addDocument( + 'import tensorflow as tf\nfrom torch.utils import foo', + 'foo.py', + ); + await documentManager.showTextDocument(document); + await tensorBoardImportTracker.activate(); + assert.ok(showNativeTensorBoardPrompt.notCalled); + }); + test('Do not show prompt if language is not Python', async () => { + const document = documentManager.addDocument( + 'import tensorflow as tf\nfrom torch.utils import foo', + 'foo.cpp', + 'cpp', + ); + await documentManager.showTextDocument(document); + await tensorBoardImportTracker.activate(); + assert.ok(showNativeTensorBoardPrompt.notCalled); + }); + }); }); }); From 10b98d34b51b501531ac21027fcab59b12c8e182 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 17 Oct 2023 20:29:54 +1100 Subject: [PATCH 09/15] Deprecate the log directory setting (#22236) --- package.json | 4 +++- package.nls.json | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 5b13f9eae0a3..c89ccf624a27 100644 --- a/package.json +++ b/package.json @@ -1100,7 +1100,9 @@ "default": "", "description": "%python.tensorBoard.logDirectory.description%", "scope": "resource", - "type": "string" + "type": "string", + "markdownDeprecationMessage": "%python.tensorBoard.logDirectory.markdownDeprecationMessage%", + "deprecationMessage": "%python.tensorBoard.logDirectory.deprecationMessage%" }, "python.terminal.activateEnvInCurrentTerminal": { "default": false, diff --git a/package.nls.json b/package.nls.json index 87692fb7c1a8..c738b3692daf 100644 --- a/package.nls.json +++ b/package.nls.json @@ -182,6 +182,8 @@ "python.pipenvPath.description": "Path to the pipenv executable to use for activation.", "python.poetryPath.description": "Path to the poetry executable.", "python.tensorBoard.logDirectory.description": "Set this setting to your preferred TensorBoard log directory to skip log directory prompt when starting TensorBoard.", + "python.tensorBoard.logDirectory.markdownDeprecationMessage": "Tensorboard support has been moved to the extension [Tensorboard extension](https://marketplace.visualstudio.com/items?itemName=ms-toolsai.tensorboard). Instead use the setting `tensorBoard.logDirectory`.", + "python.tensorBoard.logDirectory.deprecationMessage": "Tensorboard support has been moved to the extension Tensorboard extension. Instead use the setting `tensorBoard.logDirectory`.", "python.terminal.activateEnvInCurrentTerminal.description": "Activate Python Environment in the current Terminal on load of the Extension.", "python.terminal.activateEnvironment.description": "Activate Python Environment in all Terminals created.", "python.terminal.executeInFileDir.description": "When executing a file in the terminal, whether to use execute in the file's directory, instead of the current open folder.", From ebaf8fe0d587cfbc190bd89ad4d584c35ff57bd1 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 17 Oct 2023 13:29:27 -0700 Subject: [PATCH 10/15] Fix experiment telemetry related to optInto/optOutFrom settings (#22241) cc/ @luabud --- src/client/common/experiments/service.ts | 6 ++++-- src/client/telemetry/index.ts | 8 ++++---- src/test/common/experiments/service.unit.test.ts | 16 +++++++++++----- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/client/common/experiments/service.ts b/src/client/common/experiments/service.ts index 270f91512809..3d85b99a26ff 100644 --- a/src/client/common/experiments/service.ts +++ b/src/client/common/experiments/service.ts @@ -257,8 +257,10 @@ function sendOptInOptOutTelemetry(optedIn: string[], optedOut: string[], package const sanitizedOptedIn = optedIn.filter((exp) => optedInEnumValues.includes(exp)); const sanitizedOptedOut = optedOut.filter((exp) => optedOutEnumValues.includes(exp)); + JSON.stringify(sanitizedOptedIn.sort()); + sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_OPT_IN_OPT_OUT_SETTINGS, undefined, { - optedInto: sanitizedOptedIn, - optedOutFrom: sanitizedOptedOut, + optedInto: JSON.stringify(sanitizedOptedIn.sort()), + optedOutFrom: JSON.stringify(sanitizedOptedOut.sort()), }); } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index f69da6046254..ba65c4d1913f 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1405,14 +1405,14 @@ export interface IEventNamePropertyMapping { [EventName.PYTHON_EXPERIMENTS_OPT_IN_OPT_OUT_SETTINGS]: { /** * List of valid experiments in the python.experiments.optInto setting - * @type {string[]} + * @type {string} */ - optedInto: string[]; + optedInto: string; /** * List of valid experiments in the python.experiments.optOutFrom setting - * @type {string[]} + * @type {string} */ - optedOutFrom: string[]; + optedOutFrom: string; }; /** * Telemetry event sent when LS is started for workspace (workspace folder in case of multi-root) diff --git a/src/test/common/experiments/service.unit.test.ts b/src/test/common/experiments/service.unit.test.ts index 1d96f2e0bd70..ab05db6da5a1 100644 --- a/src/test/common/experiments/service.unit.test.ts +++ b/src/test/common/experiments/service.unit.test.ts @@ -491,7 +491,10 @@ suite('Experimentation service', () => { await experimentService.activate(); const { properties } = telemetryEvents[1]; - assert.deepStrictEqual(properties, { optedInto: ['foo'], optedOutFrom: ['bar'] }); + assert.deepStrictEqual(properties, { + optedInto: JSON.stringify(['foo']), + optedOutFrom: JSON.stringify(['bar']), + }); }); test('Set telemetry properties to empty arrays if no experiments have been opted into or out from', async () => { @@ -523,7 +526,7 @@ suite('Experimentation service', () => { await experimentService.activate(); const { properties } = telemetryEvents[1]; - assert.deepStrictEqual(properties, { optedInto: [], optedOutFrom: [] }); + assert.deepStrictEqual(properties, { optedInto: '[]', optedOutFrom: '[]' }); }); test('If the entered value for a setting contains "All", do not expand it to be a list of all experiments, and pass it as-is', async () => { @@ -555,7 +558,10 @@ suite('Experimentation service', () => { await experimentService.activate(); const { properties } = telemetryEvents[0]; - assert.deepStrictEqual(properties, { optedInto: ['All'], optedOutFrom: ['All'] }); + assert.deepStrictEqual(properties, { + optedInto: JSON.stringify(['All']), + optedOutFrom: JSON.stringify(['All']), + }); }); // This is an unlikely scenario. @@ -577,7 +583,7 @@ suite('Experimentation service', () => { await experimentService.activate(); const { properties } = telemetryEvents[1]; - assert.deepStrictEqual(properties, { optedInto: [], optedOutFrom: [] }); + assert.deepStrictEqual(properties, { optedInto: '[]', optedOutFrom: '[]' }); }); // This is also an unlikely scenario. @@ -608,7 +614,7 @@ suite('Experimentation service', () => { await experimentService.activate(); const { properties } = telemetryEvents[1]; - assert.deepStrictEqual(properties, { optedInto: [], optedOutFrom: [] }); + assert.deepStrictEqual(properties, { optedInto: '[]', optedOutFrom: '[]' }); }); }); }); From 754f8effa482d2e37a8dfba588da4d51374e2a63 Mon Sep 17 00:00:00 2001 From: Eleanor Boyd Date: Tue, 17 Oct 2023 13:50:28 -0700 Subject: [PATCH 11/15] remove node deletion for error tolerant discovery (#22207) helps with a part of https://github.com/microsoft/vscode-python/issues/21757 --- pythonFiles/vscode_pytest/__init__.py | 12 ++-- .../testController/common/resultResolver.ts | 8 --- .../pytest/pytestDiscoveryAdapter.ts | 5 +- .../resultResolver.unit.test.ts | 63 +++++++++++++++++++ 4 files changed, 72 insertions(+), 16 deletions(-) diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index 8349e1aa893d..300f145b6f75 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -302,12 +302,12 @@ def pytest_sessionfinish(session, exitstatus): session -- the pytest session object. exitstatus -- the status code of the session. - 0: All tests passed successfully. - 1: One or more tests failed. - 2: Pytest was unable to start or run any tests due to issues with test discovery or test collection. - 3: Pytest was interrupted by the user, for example by pressing Ctrl+C during test execution. - 4: Pytest encountered an internal error or exception during test execution. - 5: Pytest was unable to find any tests to run. + Exit code 0: All tests were collected and passed successfully + Exit code 1: Tests were collected and run but some of the tests failed + Exit code 2: Test execution was interrupted by the user + Exit code 3: Internal error happened while executing tests + Exit code 4: pytest command line usage error + Exit code 5: No tests were collected """ cwd = pathlib.Path.cwd() if IS_DISCOVERY: diff --git a/src/client/testing/testController/common/resultResolver.ts b/src/client/testing/testController/common/resultResolver.ts index aaf82b143823..22a13090e1b1 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -103,14 +103,6 @@ export class PythonResultResolver implements ITestResultResolver { // If the test root for this folder exists: Workspace refresh, update its children. // Otherwise, it is a freshly discovered workspace, and we need to create a new test root and populate the test tree. populateTestTree(this.testController, rawTestData.tests, undefined, this, token); - } else { - // Delete everything from the test controller. - const errorNode = this.testController.items.get(`DiscoveryError:${workspacePath}`); - this.testController.items.replace([]); - // Add back the error node if it exists. - if (errorNode !== undefined) { - this.testController.items.add(errorNode); - } } sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { diff --git a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts index 09ca36849000..daaaec04ee1c 100644 --- a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts +++ b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts @@ -120,9 +120,10 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { } }); result?.proc?.on('close', (code, signal) => { - if (code !== 0) { + // pytest exits with code of 5 when 0 tests are found- this is not a failure for discovery. + if (code !== 0 && code !== 5) { traceError( - `Subprocess exited unsuccessfully with exit code ${code} and signal ${signal}. Creating and sending error discovery payload`, + `Subprocess exited unsuccessfully with exit code ${code} and signal ${signal} on workspace ${uri.fsPath}. Creating and sending error discovery payload`, ); // if the child process exited with a non-zero exit code, then we need to send the error payload. this.testServer.triggerDiscoveryDataReceivedEvent({ diff --git a/src/test/testing/testController/resultResolver.unit.test.ts b/src/test/testing/testController/resultResolver.unit.test.ts index 2078c72e8cf6..5ecf75987b3c 100644 --- a/src/test/testing/testController/resultResolver.unit.test.ts +++ b/src/test/testing/testController/resultResolver.unit.test.ts @@ -195,6 +195,69 @@ suite('Result Resolver tests', () => { cancelationToken, // token ); }); + test('resolveDiscovery should create error and not clear test items to allow for error tolerant discovery', async () => { + // test specific constants used expected values + testProvider = 'pytest'; + workspaceUri = Uri.file('/foo/bar'); + resultResolver = new ResultResolver.PythonResultResolver(testController, testProvider, workspaceUri); + const errorMessage = 'error msg A'; + const expectedErrorMessage = `${defaultErrorMessage}\r\n ${errorMessage}`; + + // create test result node + const tests: DiscoveredTestNode = { + path: 'path', + name: 'name', + type_: 'folder', + id_: 'id', + children: [], + }; + // stub out return values of functions called in resolveDiscovery + const errorPayload: DiscoveredTestPayload = { + cwd: workspaceUri.fsPath, + status: 'error', + error: [errorMessage], + }; + const regPayload: DiscoveredTestPayload = { + cwd: workspaceUri.fsPath, + status: 'success', + error: [errorMessage], + tests, + }; + const errorTestItemOptions: testItemUtilities.ErrorTestItemOptions = { + id: 'id', + label: 'label', + error: 'error', + }; + + // stub out functionality of buildErrorNodeOptions and createErrorTestItem which are called in resolveDiscovery + const buildErrorNodeOptionsStub = sinon.stub(util, 'buildErrorNodeOptions').returns(errorTestItemOptions); + const createErrorTestItemStub = sinon.stub(testItemUtilities, 'createErrorTestItem').returns(blankTestItem); + + // stub out functionality of populateTestTreeStub which is called in resolveDiscovery + sinon.stub(util, 'populateTestTree').returns(); + // add spies to insure these aren't called + const deleteSpy = sinon.spy(testController.items, 'delete'); + const replaceSpy = sinon.spy(testController.items, 'replace'); + // call resolve discovery + let deferredTillEOT: Deferred = createDeferred(); + resultResolver.resolveDiscovery(regPayload, deferredTillEOT, cancelationToken); + deferredTillEOT = createDeferred(); + resultResolver.resolveDiscovery(errorPayload, deferredTillEOT, cancelationToken); + + // assert the stub functions were called with the correct parameters + + // builds an error node root + sinon.assert.calledWithMatch(buildErrorNodeOptionsStub, workspaceUri, expectedErrorMessage, testProvider); + // builds an error item + sinon.assert.calledWithMatch(createErrorTestItemStub, sinon.match.any, sinon.match.any); + + if (!deleteSpy.calledOnce) { + throw new Error("The delete method was called, but it shouldn't have been."); + } + if (replaceSpy.called) { + throw new Error("The replace method was called, but it shouldn't have been."); + } + }); }); suite('Test execution result resolver', () => { let resultResolver: ResultResolver.PythonResultResolver; From 44053a22aafaa4ae1d661f867b4735b237308a14 Mon Sep 17 00:00:00 2001 From: Eleanor Boyd Date: Tue, 17 Oct 2023 13:50:59 -0700 Subject: [PATCH 12/15] add hookwrappers to pytest plugin to ensure run (#22240) fixes https://github.com/microsoft/vscode-python/issues/22232. From [this discussion](https://github.com/pytest-dev/pytest/discussions/11509), learned that some pytest hooks are meant to be unique and only one will be called per run. If multiple plugins are at play then another plugin the user has might override our plugin. Added the hookwrapper so our is always run. --- pythonFiles/vscode_pytest/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index 300f145b6f75..1718d435bb23 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -183,6 +183,7 @@ class testRunResultDict(Dict[str, Dict[str, TestOutcome]]): tests: Dict[str, TestOutcome] +@pytest.hookimpl(hookwrapper=True, trylast=True) def pytest_report_teststatus(report, config): """ A pytest hook that is called when a test is called. It is called 3 times per test, @@ -223,6 +224,7 @@ def pytest_report_teststatus(report, config): "success", collected_test if collected_test else None, ) + yield ERROR_MESSAGE_CONST = { From 4caa20735b4fa7832e3d62e884cbc04b482f2ad8 Mon Sep 17 00:00:00 2001 From: Eleanor Boyd Date: Tue, 17 Oct 2023 15:55:24 -0700 Subject: [PATCH 13/15] add wrapper hook for pytest_runtest_protocol (#22243) fixes https://github.com/microsoft/vscode-python/issues/22232. From https://github.com/pytest-dev/pytest/discussions/11509, learned that some pytest hooks are meant to be unique and only one will be called per run. If multiple plugins are at play then another plugin the user has might override our plugin. Added the hookwrapper so our is always run. Same as https://github.com/microsoft/vscode-python/pull/22240 --- pythonFiles/vscode_pytest/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index 1718d435bb23..0767b85c5249 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -235,6 +235,7 @@ def pytest_report_teststatus(report, config): } +@pytest.hookimpl(hookwrapper=True, trylast=True) def pytest_runtest_protocol(item, nextitem): map_id_to_path[item.nodeid] = get_node_path(item) skipped = check_skipped_wrapper(item) @@ -257,6 +258,7 @@ def pytest_runtest_protocol(item, nextitem): "success", collected_test if collected_test else None, ) + yield def check_skipped_wrapper(item): From 01c7665e37f4674a6a574d38f4f7af9344ec0485 Mon Sep 17 00:00:00 2001 From: Eleanor Boyd Date: Tue, 17 Oct 2023 16:45:44 -0700 Subject: [PATCH 14/15] add correct retrieval of workspace adapter for test discovery in multiroot context (#22246) fixes: https://github.com/microsoft/vscode-python/issues/22218 --- .../testing/testController/controller.ts | 43 +++++++++++++------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/src/client/testing/testController/controller.ts b/src/client/testing/testController/controller.ts index a87017a26a51..329326d84af9 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -269,13 +269,20 @@ export class PythonTestController implements ITestController, IExtensionSingleAc if (settings.testing.pytestEnabled) { if (pythonTestAdapterRewriteEnabled(this.serviceContainer)) { traceInfo(`Running discovery for pytest using the new test adapter.`); - const testAdapter = - this.testAdapters.get(uri) || (this.testAdapters.values().next().value as WorkspaceTestAdapter); - testAdapter.discoverTests( - this.testController, - this.refreshCancellation.token, - this.pythonExecFactory, - ); + if (workspace && workspace.uri) { + const testAdapter = this.testAdapters.get(workspace.uri); + if (testAdapter) { + testAdapter.discoverTests( + this.testController, + this.refreshCancellation.token, + this.pythonExecFactory, + ); + } else { + traceError('Unable to find test adapter for workspace.'); + } + } else { + traceError('Unable to find workspace for given file'); + } } else { // else use OLD test discovery mechanism await this.pytest.refreshTestData(this.testController, uri, this.refreshCancellation.token); @@ -283,13 +290,21 @@ export class PythonTestController implements ITestController, IExtensionSingleAc } else if (settings.testing.unittestEnabled) { if (pythonTestAdapterRewriteEnabled(this.serviceContainer)) { traceInfo(`Running discovery for unittest using the new test adapter.`); - const testAdapter = - this.testAdapters.get(uri) || (this.testAdapters.values().next().value as WorkspaceTestAdapter); - testAdapter.discoverTests( - this.testController, - this.refreshCancellation.token, - this.pythonExecFactory, - ); + traceInfo(`Running discovery for pytest using the new test adapter.`); + if (workspace && workspace.uri) { + const testAdapter = this.testAdapters.get(workspace.uri); + if (testAdapter) { + testAdapter.discoverTests( + this.testController, + this.refreshCancellation.token, + this.pythonExecFactory, + ); + } else { + traceError('Unable to find test adapter for workspace.'); + } + } else { + traceError('Unable to find workspace for given file'); + } } else { // else use OLD test discovery mechanism await this.unittest.refreshTestData(this.testController, uri, this.refreshCancellation.token); From 7cb3593c1f998d109721f783a0b80ae878dd0164 Mon Sep 17 00:00:00 2001 From: Bolton Bailey Date: Wed, 18 Oct 2023 13:29:57 -0500 Subject: [PATCH 15/15] Remove unmatched parenthesis from error message (#22254) Remove unmatched parenthesis from error message closes: https://github.com/microsoft/vscode-python/issues/22253 --- src/client/terminals/codeExecution/helper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/terminals/codeExecution/helper.ts b/src/client/terminals/codeExecution/helper.ts index c560de9c17b7..058c78e332a3 100644 --- a/src/client/terminals/codeExecution/helper.ts +++ b/src/client/terminals/codeExecution/helper.ts @@ -152,7 +152,7 @@ export class CodeExecutionHelper implements ICodeExecutionHelper { return undefined; } if (activeEditor.document.languageId !== PYTHON_LANGUAGE) { - this.applicationShell.showErrorMessage(l10n.t('The active file is not a Python source file)')); + this.applicationShell.showErrorMessage(l10n.t('The active file is not a Python source file')); return undefined; } if (activeEditor.document.isDirty) {