From 2aac1502eac537b5076f154f04d1e55caa448e03 Mon Sep 17 00:00:00 2001 From: rchiodo Date: Wed, 7 Oct 2020 15:50:16 -0700 Subject: [PATCH 01/10] Splitting test log --- .github/workflows/pr_datascience.yml | 3 - .vscode/settings.json | 2 +- ThirdPartyNotices-Repository.txt | 32 ++++++- build/.mocha-multi-reporters.config | 2 +- build/ci/scripts/spec_with_pid.js | 96 +++++++++++++++++++ .../tests/logParser.py | 81 ++++++++++++++++ src/client/common/process/baseDaemon.ts | 14 ++- src/client/logging/formatters.ts | 4 +- .../uiTests/ipywidget.ui.functional.test.ts | 2 +- 9 files changed, 223 insertions(+), 13 deletions(-) create mode 100644 build/ci/scripts/spec_with_pid.js create mode 100644 pythonFiles/vscode_datascience_helpers/tests/logParser.py diff --git a/.github/workflows/pr_datascience.yml b/.github/workflows/pr_datascience.yml index e33c72c7932a..481340e18add 100644 --- a/.github/workflows/pr_datascience.yml +++ b/.github/workflows/pr_datascience.yml @@ -1,9 +1,6 @@ name: Pull Request DataScience on: - push: - branches: - - main pull_request: branches: - main diff --git a/.vscode/settings.json b/.vscode/settings.json index b2dc13efe26e..9f729bd8127e 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -45,7 +45,7 @@ "source.fixAll.eslint": true, "source.fixAll.tslint": true }, - "python.languageServer": "Microsoft", + "python.languageServer": "Pylance", "python.analysis.logLevel": "Trace", "python.analysis.downloadChannel": "beta", "python.linting.pylintEnabled": false, diff --git a/ThirdPartyNotices-Repository.txt b/ThirdPartyNotices-Repository.txt index 98c64072ca51..87d22406897b 100644 --- a/ThirdPartyNotices-Repository.txt +++ b/ThirdPartyNotices-Repository.txt @@ -20,6 +20,7 @@ Microsoft Python extension for Visual Studio Code incorporates third party mater 16. ipywidgets (https://github.com/jupyter-widgets) 17. vscode-cpptools (https://github.com/microsoft/vscode-cpptools) 18. font-awesome (https://github.com/FortAwesome/Font-Awesome) +19. mocha (https://github.com/mochajs/mocha) %% Go for Visual Studio Code NOTICES, INFORMATION, AND LICENSE BEGIN HERE @@ -1195,4 +1196,33 @@ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. ========================================= -END OF jedi-language-server NOTICES, INFORMATION, AND LICENSE \ No newline at end of file +END OF jedi-language-server NOTICES, INFORMATION, AND LICENSE + +%% mocha NOTICES, INFORMATION, AND LICENSE BEGIN HERE +========================================= + +(The MIT License) + +Copyright (c) 2011-2020 OpenJS Foundation and contributors, https://openjsf.org + +Permission is hereby granted, free of charge, to any person obtaining +a copy of this software and associated documentation files (the +'Software'), to deal in the Software without restriction, including +without limitation the rights to use, copy, modify, merge, publish, +distribute, sublicense, and/or sell copies of the Software, and to +permit persons to whom the Software is furnished to do so, subject to +the following conditions: + +The above copyright notice and this permission notice shall be +included in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED 'AS IS', WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, +TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE +SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +========================================= +END OF mocha NOTICES, INFORMATION, AND LICENSE diff --git a/build/.mocha-multi-reporters.config b/build/.mocha-multi-reporters.config index 539aa1c15b60..abe46f117f5b 100644 --- a/build/.mocha-multi-reporters.config +++ b/build/.mocha-multi-reporters.config @@ -1,3 +1,3 @@ { - "reporterEnabled": "spec,mocha-junit-reporter" + "reporterEnabled": "./build/ci/scripts/spec_with_pid,mocha-junit-reporter" } diff --git a/build/ci/scripts/spec_with_pid.js b/build/ci/scripts/spec_with_pid.js new file mode 100644 index 000000000000..52cb064446cc --- /dev/null +++ b/build/ci/scripts/spec_with_pid.js @@ -0,0 +1,96 @@ +'use strict'; +/** + * @module Spec + */ +/** + * Module dependencies. + */ + +var Base = require('mocha/lib/reporters/base'); +var constants = require('mocha/lib/runner').constants; +var EVENT_RUN_BEGIN = constants.EVENT_RUN_BEGIN; +var EVENT_RUN_END = constants.EVENT_RUN_END; +var EVENT_SUITE_BEGIN = constants.EVENT_SUITE_BEGIN; +var EVENT_SUITE_END = constants.EVENT_SUITE_END; +var EVENT_TEST_FAIL = constants.EVENT_TEST_FAIL; +var EVENT_TEST_PASS = constants.EVENT_TEST_PASS; +var EVENT_TEST_PENDING = constants.EVENT_TEST_PENDING; +var inherits = require('mocha/lib/utils').inherits; +var color = Base.color; + +/** + * Expose `Spec`. + */ + +exports = module.exports = Spec; + +/** + * Constructs a new `Spec` reporter instance. + * + * @public + * @class + * @memberof Mocha.reporters + * @extends Mocha.reporters.Base + * @param {Runner} runner - Instance triggers reporter actions. + * @param {Object} [options] - runner options + */ +function Spec(runner, options) { + Base.call(this, runner, options); + + var self = this; + var indents = 0; + var n = 0; + + function indent() { + return Array(indents).join(' '); + } + + runner.on(EVENT_RUN_BEGIN, function () { + Base.consoleLog(); + }); + + runner.on(EVENT_SUITE_BEGIN, function (suite) { + ++indents; + Base.consoleLog(color('suite', `${process.pid} %s%s`), indent(), suite.title); + }); + + runner.on(EVENT_SUITE_END, function () { + --indents; + if (indents === 1) { + Base.consoleLog(); + } + }); + + runner.on(EVENT_TEST_PENDING, function (test) { + var fmt = indent() + color('pending', `${process.pid} - %s`); + Base.consoleLog(fmt, test.title); + }); + + runner.on(EVENT_TEST_PASS, function (test) { + var fmt; + if (test.speed === 'fast') { + fmt = indent() + color('checkmark', `${process.pid} ` + Base.symbols.ok) + color('pass', ' %s'); + Base.consoleLog(fmt, test.title); + } else { + fmt = + indent() + + color('checkmark', `${process.pid} ` + Base.symbols.ok) + + color('pass', ' %s') + + color(test.speed, ' (%dms)'); + Base.consoleLog(fmt, test.title, test.duration); + } + }); + + runner.on(EVENT_TEST_FAIL, function (test) { + Base.consoleLog(indent() + color('fail', `${process.pid} %d) %s`), ++n, test.title); + }); + + runner.once(EVENT_RUN_END, self.epilogue.bind(self)); +} + +/** + * Inherit from `Base.prototype`. + */ +inherits(Spec, Base); + +Spec.description = 'hierarchical & verbose [default]'; diff --git a/pythonFiles/vscode_datascience_helpers/tests/logParser.py b/pythonFiles/vscode_datascience_helpers/tests/logParser.py new file mode 100644 index 000000000000..75b442e253d2 --- /dev/null +++ b/pythonFiles/vscode_datascience_helpers/tests/logParser.py @@ -0,0 +1,81 @@ +import sys +import argparse +import os +os.system("color") +from pathlib import Path +import re + +parser = argparse.ArgumentParser(description='Parse a test log into its parts') +parser.add_argument('testlog', type=str, nargs=1, help='Log to parse') +parser.add_argument('--testoutput', action='store_true', help='Show all failures and passes') +parser.add_argument('--split', action='store_true', help='Split into per process files. Each file will have the pid appended') +ansi_escape = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])') +pid_regex = re.compile(r'(\d+).*') + +def isUnwantedOutput(s: str): + return s.startswith("{\"json") or s.startswith("Content-type") or s.startswith("Content-Type") or s.startswith("Output") or s.startswith("Starting") or s.startswith("Kernel") + +def printTestOutput(testlog): + # Find all the lines that don't have a PID in them. These are the test output + p = Path(testlog[0]) + with p.open() as f: + for line in f.readlines(): + stripped = line.strip() + if (not isUnwantedOutput(stripped) and len(stripped) > 0): + if (stripped[0] < '0'): + print(line.rstrip()) # Should be a test line + elif (stripped[0] > '9'): + print(line.rstrip()) # Not starting with a number + +def splitByPid(testlog): + # Split testlog into prefixed logs based on pid + baseFile = os.path.splitext(testlog[0])[0] + p = Path(testlog[0]) + pids = set() + logs = {} + pid = None + with p.open() as f: + for line in f.readlines(): + stripped = ansi_escape.sub('', line.strip()) + # See if starts with a pid + if (len(stripped) > 0 and stripped[0] <= '9' and stripped[0] >= '0'): + # Pull out the pid + match = pid_regex.match(stripped) + + # Pids are at least two digits + if (match != None and len(match.group(1)) > 2): + # Pid is found + pid = int(match.group(1)) + + # See if we've created a log for this pid or not + if (not pid in pids): + pids.add(pid) + logFile = '{}_{}.log'.format(baseFile, pid) + print('Writing to new log: ' + logFile) + logs[pid] = Path(logFile).open(mode="w") + + # Add this line to the log + if (pid != None): + logs[pid].write(line) + # Close all of the open logs + for key in logs: + logs[key].close() + + + + +def doWork(args): + if (not args.testlog): + print('Test log should be passed') + elif (args.testoutput): + printTestOutput(args.testlog) + elif (args.split): + splitByPid(args.testlog) + else: + parser.print_usage() + +def main(): + doWork(parser.parse_args()) + +if __name__ == "__main__": + main() \ No newline at end of file diff --git a/src/client/common/process/baseDaemon.ts b/src/client/common/process/baseDaemon.ts index abd93668115d..0628e96e9006 100644 --- a/src/client/common/process/baseDaemon.ts +++ b/src/client/common/process/baseDaemon.ts @@ -177,14 +177,20 @@ export abstract class BasePythonDaemon { return Object.keys(options).every((item) => daemonSupportedSpawnOptions.indexOf(item as any) >= 0); } protected sendRequestWithoutArgs(type: RequestType0): Thenable { - return Promise.race([this.connection.sendRequest(type), this.connectionClosedDeferred.promise]); + if (this.proc && this.proc.exitCode === null) { + return Promise.race([this.connection.sendRequest(type), this.connectionClosedDeferred.promise]); + } + return this.connectionClosedDeferred.promise; } protected sendRequest(type: RequestType, params?: P): Thenable { - if (!this.isAlive) { + if (!this.isAlive || this.proc.exitCode !== null) { traceError('Daemon is handling a request after death.'); } - // Throw an error if the connection has been closed. - return Promise.race([this.connection.sendRequest(type, params), this.connectionClosedDeferred.promise]); + if (this.proc && this.proc.exitCode === null) { + // Throw an error if the connection has been closed. + return Promise.race([this.connection.sendRequest(type, params), this.connectionClosedDeferred.promise]); + } + return this.connectionClosedDeferred.promise; } protected throwIfRPCConnectionIsDead() { if (!this.isAlive) { diff --git a/src/client/logging/formatters.ts b/src/client/logging/formatters.ts index b3dd4e52761f..734746a246c1 100644 --- a/src/client/logging/formatters.ts +++ b/src/client/logging/formatters.ts @@ -37,13 +37,13 @@ function normalizeLevel(name: LogLevelName): string { // Return a log entry that can be emitted as-is. function formatMessage(level: LogLevelName, timestamp: string, message: string): string { const levelFormatted = normalizeLevel(level); - return `${levelFormatted} ${timestamp}: ${message}`; + return `${process.pid} ${levelFormatted} ${timestamp}: ${message}`; } // Return a log entry that can be emitted as-is. function formatLabeledMessage(level: LogLevelName, timestamp: string, label: string, message: string): string { const levelFormatted = normalizeLevel(level); - return `${levelFormatted} ${label} ${timestamp}: ${message}`; + return `${process.pid} ${levelFormatted} ${label} ${timestamp}: ${message}`; } // Return a minimal format object that can be used with a "winston" diff --git a/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts b/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts index 2c5dbc8b7825..d42afe219a20 100644 --- a/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts +++ b/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts @@ -41,7 +41,7 @@ use(chaiAsPromised); suiteSetup(function () { // Skip all tests until flakiness can be resolved. // See issue: https://github.com/microsoft/vscode-python/issues/13936 - if (IS_CI_SERVER) { + if (IS_CI_SERVER || process.env.VSC_PYTHON_FORCE_LOGGING) { this.skip(); } From bcd65e9ff3e8c54398dfece8484acb323135d00b Mon Sep 17 00:00:00 2001 From: rchiodo Date: Thu, 8 Oct 2020 11:23:04 -0700 Subject: [PATCH 02/10] Fix problem with kernels ports being reused --- .../tests/logParser.py | 10 +-- .../kernel-launcher/kernelLauncher.ts | 83 +++++++++++++++++-- .../datascience/dataScienceIocContainer.ts | 4 + .../dataviewer.functional.test.tsx | 5 +- .../datascience/debugger.functional.test.tsx | 5 +- .../gotocell.functional.test.ts | 3 + .../errorHandler.functional.test.tsx | 3 + .../intellisense.functional.test.tsx | 8 +- .../nativeEditorProvider.functional.test.ts | 3 + .../interactiveWindow.functional.test.tsx | 6 +- ...UriProviderRegistration.functional.test.ts | 3 + .../kernelLauncher.functional.test.ts | 1 + .../datascience/liveshare.functional.test.tsx | 2 +- .../nativeEditor.functional.test.tsx | 16 +--- .../datascience/notebook.functional.test.ts | 14 +--- .../plotViewer.functional.test.tsx | 7 +- .../raw-kernel/rawKernel.functional.test.ts | 3 + .../trustedNotebooks.functional.test.tsx | 4 + .../uiTests/ipywidget.ui.functional.test.ts | 10 +-- .../variableexplorer.functional.test.tsx | 12 +-- .../startPage/startPage.functional.test.tsx | 3 + 21 files changed, 126 insertions(+), 79 deletions(-) diff --git a/pythonFiles/vscode_datascience_helpers/tests/logParser.py b/pythonFiles/vscode_datascience_helpers/tests/logParser.py index 75b442e253d2..16e6437823b2 100644 --- a/pythonFiles/vscode_datascience_helpers/tests/logParser.py +++ b/pythonFiles/vscode_datascience_helpers/tests/logParser.py @@ -12,20 +12,14 @@ ansi_escape = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])') pid_regex = re.compile(r'(\d+).*') -def isUnwantedOutput(s: str): - return s.startswith("{\"json") or s.startswith("Content-type") or s.startswith("Content-Type") or s.startswith("Output") or s.startswith("Starting") or s.startswith("Kernel") - def printTestOutput(testlog): # Find all the lines that don't have a PID in them. These are the test output p = Path(testlog[0]) with p.open() as f: for line in f.readlines(): stripped = line.strip() - if (not isUnwantedOutput(stripped) and len(stripped) > 0): - if (stripped[0] < '0'): - print(line.rstrip()) # Should be a test line - elif (stripped[0] > '9'): - print(line.rstrip()) # Not starting with a number + if (len(stripped) > 2 and stripped[0] == '\x1B' and stripped[1] == '['): + print(line.rstrip()) # Should be a test line as it has color encoding def splitByPid(testlog): # Split testlog into prefixed logs based on pid diff --git a/src/client/datascience/kernel-launcher/kernelLauncher.ts b/src/client/datascience/kernel-launcher/kernelLauncher.ts index 50894ab93a10..7944b3b21c6d 100644 --- a/src/client/datascience/kernel-launcher/kernelLauncher.ts +++ b/src/client/datascience/kernel-launcher/kernelLauncher.ts @@ -2,10 +2,14 @@ // Licensed under the MIT License. 'use strict'; +import * as fsextra from 'fs-extra'; import { inject, injectable } from 'inversify'; +import * as os from 'os'; +import * as path from 'path'; import * as portfinder from 'portfinder'; import { promisify } from 'util'; import * as uuid from 'uuid/v4'; +import { traceInfo } from '../../common/logger'; import { IProcessServiceFactory } from '../../common/process/types'; import { Resource } from '../../common/types'; import { captureTelemetry } from '../../telemetry'; @@ -16,20 +20,73 @@ import { KernelDaemonPool } from './kernelDaemonPool'; import { KernelProcess } from './kernelProcess'; import { IKernelConnection, IKernelLauncher, IKernelProcess } from './types'; -const PortToStartFrom = 9_000; +const PortFormatString = `kernelLauncherPortStart_{0}.tmp`; // Launches and returns a kernel process given a resource or python interpreter. // If the given interpreter is undefined, it will try to use the selected interpreter. // If the selected interpreter doesn't have a kernel, it will find a kernel on disk and use that. @injectable() export class KernelLauncher implements IKernelLauncher { - private static nextFreePortToTryAndUse = PortToStartFrom; + private static startPortFS: number = 0; + private static startPortPromise = KernelLauncher.computeStartPort(); + private static nextFreePortToTryAndUsePromise = KernelLauncher.startPortPromise; constructor( @inject(IProcessServiceFactory) private processExecutionFactory: IProcessServiceFactory, @inject(IDataScienceFileSystem) private readonly fs: IDataScienceFileSystem, @inject(KernelDaemonPool) private readonly daemonPool: KernelDaemonPool ) {} + // This function is public so it can be called when a test shuts down + public static cleanupStartPort() { + // Cleanup our start port file + fsextra.close(KernelLauncher.startPortFS).ignoreErrors(); + + // Destroy the file + KernelLauncher.startPortPromise + .then((p) => { + traceInfo(`Cleaning up port start file : ${p}`); + + const filePath = path.join(os.tmpdir(), PortFormatString.format(p.toString())); + return fsextra.pathExists(filePath).then((e) => { + if (e) { + return fsextra.remove(filePath); + } + }); + }) + .ignoreErrors(); + } + + private static async computeStartPort(): Promise { + // Since multiple instances of VS code may be running, write our best guess to a shared file + let portStart = 9_000; + let result = 0; + while (result === 0 && portStart < 65_000) { + try { + // Try creating a file (not worrying about two instances starting at exactly the same time. That's much less likely) + const filePath = path.join(os.tmpdir(), PortFormatString.format(portStart.toString())); + + // Create a file stream object that should fail if the file exists + KernelLauncher.startPortFS = await fsextra.open(filePath, 'wx'); + + // If that works, we have our port + result = portStart; + } catch { + // If that fails, it should mean the file already exists + portStart += 1_000; + } + } + + traceInfo(`Computed port start for KernelLauncher is : ${result}`); + + // Before finishing setup an on exit handler for the current process. + // Note we have to do this as a proc exit handler instead of IDisposable because + // during a test run the container is destroyed over and over again. What we need + // is for the same process to always use the same start port. + process.on('exit', KernelLauncher.cleanupStartPort); + + return result; + } + @captureTelemetry(Telemetry.KernelLauncherPerf) public async launch( kernelConnectionMetadata: KernelSpecConnectionMetadata | PythonKernelConnectionMetadata, @@ -49,18 +106,28 @@ export class KernelLauncher implements IKernelLauncher { return kernelProcess; } - private async getKernelConnection(): Promise { + private async getPorts(): Promise { const getPorts = promisify(portfinder.getPorts); + + // Have to wait for static port lookup (it handles case where two VS code instances are running) + const nextFreePort = await KernelLauncher.nextFreePortToTryAndUsePromise; + const startPort = await KernelLauncher.startPortPromise; + // Ports may have been freed, hence start from begining. - const port = - KernelLauncher.nextFreePortToTryAndUse > PortToStartFrom + 1_000 - ? PortToStartFrom - : KernelLauncher.nextFreePortToTryAndUse; + const port = nextFreePort > startPort + 1_000 ? startPort : nextFreePort; + + // Then get the next set starting at that point const ports = await getPorts(5, { host: '127.0.0.1', port }); + // We launch restart kernels in the background, its possible other session hasn't started. // Ensure we do not use same ports. - KernelLauncher.nextFreePortToTryAndUse = Math.max(...ports) + 1; + KernelLauncher.nextFreePortToTryAndUsePromise = Promise.resolve(Math.max(...ports) + 1); + + return ports; + } + private async getKernelConnection(): Promise { + const ports = await this.getPorts(); return { version: 1, key: uuid(), diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index cf84425ee398..710c952ad409 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -491,6 +491,10 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.asyncRegistry = new AsyncDisposableRegistry(); } + // This function should be called when an entire suite is shutdown + public static async suiteDispose(): Promise { + KernelLauncher.cleanupStartPort(); + } public async dispose(): Promise { // Make sure to disable all command handling during dispose. Don't want // anything to startup again. diff --git a/src/test/datascience/dataviewer.functional.test.tsx b/src/test/datascience/dataviewer.functional.test.tsx index d10c18e81f6c..d205046f24ea 100644 --- a/src/test/datascience/dataviewer.functional.test.tsx +++ b/src/test/datascience/dataviewer.functional.test.tsx @@ -56,6 +56,7 @@ suite('DataScience DataViewer tests', () => { suiteTeardown(() => { writeDiffSnapshot(snapshot, 'DataViewer'); + return DataScienceIocContainer.suiteDispose(); }); setup(async () => { @@ -95,10 +96,6 @@ suite('DataScience DataViewer tests', () => { delete (global as any).ascquireVsCodeApi; }); - suiteTeardown(() => { - // asyncDump(); - }); - function createJupyterVariable(variable: string, type: string): IJupyterVariable { return { name: variable, diff --git a/src/test/datascience/debugger.functional.test.tsx b/src/test/datascience/debugger.functional.test.tsx index 435fa3e01701..acd40cd8cda1 100644 --- a/src/test/datascience/debugger.functional.test.tsx +++ b/src/test/datascience/debugger.functional.test.tsx @@ -61,6 +61,7 @@ suite('DataScience Debugger tests', () => { suiteTeardown(() => { writeDiffSnapshot(snapshot, 'Debugger'); + return DataScienceIocContainer.suiteDispose(); }); setup(async () => { @@ -129,10 +130,6 @@ suite('DataScience Debugger tests', () => { } }); - suiteTeardown(() => { - // asyncDump(); - }); - async function debugCell( type: 'notebook' | 'interactive', code: string, diff --git a/src/test/datascience/editor-integration/gotocell.functional.test.ts b/src/test/datascience/editor-integration/gotocell.functional.test.ts index 13867648355b..3193de102a3e 100644 --- a/src/test/datascience/editor-integration/gotocell.functional.test.ts +++ b/src/test/datascience/editor-integration/gotocell.functional.test.ts @@ -47,6 +47,9 @@ suite('DataScience gotocell tests', () => { codeLensFactory = ioc.serviceManager.get(ICodeLensFactory); await ioc.activate(); }); + suiteTeardown(async () => { + return DataScienceIocContainer.suiteDispose(); + }); teardown(async () => { try { diff --git a/src/test/datascience/errorHandler.functional.test.tsx b/src/test/datascience/errorHandler.functional.test.tsx index ce686aaa5914..3d235b6d99b5 100644 --- a/src/test/datascience/errorHandler.functional.test.tsx +++ b/src/test/datascience/errorHandler.functional.test.tsx @@ -25,6 +25,9 @@ suite('DataScience Error Handler Functional Tests', () => { ioc = modifyContainer(); return ioc.activate(); }); + suiteTeardown(async () => { + return DataScienceIocContainer.suiteDispose(); + }); teardown(async () => { sinon.restore(); diff --git a/src/test/datascience/intellisense.functional.test.tsx b/src/test/datascience/intellisense.functional.test.tsx index daa822cb5adf..079311fd260b 100644 --- a/src/test/datascience/intellisense.functional.test.tsx +++ b/src/test/datascience/intellisense.functional.test.tsx @@ -31,15 +31,15 @@ import { ITestNativeEditorProvider } from './testNativeEditorProvider'; suiteSetup(() => { snapshot = takeSnapshot(); }); - setup(async () => { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(false, languageServerType); return ioc.activate(); }); - suiteTeardown(() => { + suiteTeardown(async () => { writeDiffSnapshot(snapshot, 'Intellisense'); + return DataScienceIocContainer.suiteDispose(); }); teardown(async () => { @@ -56,10 +56,6 @@ import { ITestNativeEditorProvider } from './testNativeEditorProvider'; await ioc.dispose(); }); - // suiteTeardown(() => { - // asyncDump(); - // }); - function getIntellisenseTextLines(wrapper: ReactWrapper, React.Component>): string[] { assert.ok(wrapper); const editor = getInteractiveEditor(wrapper); diff --git a/src/test/datascience/interactive-ipynb/nativeEditorProvider.functional.test.ts b/src/test/datascience/interactive-ipynb/nativeEditorProvider.functional.test.ts index c5e088449700..ce2c608f1e01 100644 --- a/src/test/datascience/interactive-ipynb/nativeEditorProvider.functional.test.ts +++ b/src/test/datascience/interactive-ipynb/nativeEditorProvider.functional.test.ts @@ -22,6 +22,9 @@ suite('DataScience - Native Editor Provider', () => { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(); }); + suiteTeardown(async () => { + return DataScienceIocContainer.suiteDispose(); + }); function createNotebookProvider() { return ioc.get(INotebookEditorProvider); diff --git a/src/test/datascience/interactiveWindow.functional.test.tsx b/src/test/datascience/interactiveWindow.functional.test.tsx index 3b1d3e19ef16..87b8083be89e 100644 --- a/src/test/datascience/interactiveWindow.functional.test.tsx +++ b/src/test/datascience/interactiveWindow.functional.test.tsx @@ -86,6 +86,7 @@ suite('DataScience Interactive Window output tests', () => { suiteTeardown(() => { writeDiffSnapshot(snapshot, 'Interactive Window'); + return DataScienceIocContainer.suiteDispose(); }); teardown(async () => { @@ -122,11 +123,6 @@ suite('DataScience Interactive Window output tests', () => { verifyHtmlOnCell(iw, 'InteractiveCell', html, cellIndex); } - // Uncomment this to debug hangs on exit - // suiteTeardown(() => { - // asyncDump(); - // }); - runTest( 'Simple text', async () => { diff --git a/src/test/datascience/jupyterUriProviderRegistration.functional.test.ts b/src/test/datascience/jupyterUriProviderRegistration.functional.test.ts index cf5ef1b7e3e8..95f921d2cb26 100644 --- a/src/test/datascience/jupyterUriProviderRegistration.functional.test.ts +++ b/src/test/datascience/jupyterUriProviderRegistration.functional.test.ts @@ -94,6 +94,9 @@ suite(`DataScience JupyterServerUriProvider tests`, () => { ioc.serviceManager.rebindInstance(IExtensions, new UriMockExtensions(ioc)); return ioc.activate(); }); + suiteTeardown(async () => { + return DataScienceIocContainer.suiteDispose(); + }); teardown(async () => { await ioc.dispose(); diff --git a/src/test/datascience/kernelLauncher.functional.test.ts b/src/test/datascience/kernelLauncher.functional.test.ts index d74d66c425fd..8cd97a2946f2 100644 --- a/src/test/datascience/kernelLauncher.functional.test.ts +++ b/src/test/datascience/kernelLauncher.functional.test.ts @@ -62,6 +62,7 @@ suite('DataScience - Kernel Launcher', () => { suiteTeardown(() => { writeDiffSnapshot(snapshot, 'KernelLauncher'); + return DataScienceIocContainer.suiteDispose(); }); test('Launch from kernelspec', async function () { diff --git a/src/test/datascience/liveshare.functional.test.tsx b/src/test/datascience/liveshare.functional.test.tsx index 55d45c04742c..db24d5330212 100644 --- a/src/test/datascience/liveshare.functional.test.tsx +++ b/src/test/datascience/liveshare.functional.test.tsx @@ -69,7 +69,7 @@ suite('DataScience LiveShare tests', () => { }); suiteTeardown(() => { - //asyncDump(); + return DataScienceIocContainer.suiteDispose(); }); function createContainer(role: vsls.Role): DataScienceIocContainer { diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index 095c5b7c8d5b..12bb6363d715 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -53,7 +53,6 @@ import { IMonacoEditorState, MonacoEditor } from '../../datascience-ui/react-com import { waitForCondition } from '../common'; import { createTemporaryFile } from '../utils/fs'; import { DataScienceIocContainer } from './dataScienceIocContainer'; -import { takeSnapshot, writeDiffSnapshot } from './helpers'; import { MockCustomEditorService } from './mockCustomEditorService'; import { MockDocumentManager } from './mockDocumentManager'; import { IMountedWebView, WaitForMessageOptions } from './mountedWebView'; @@ -110,10 +109,12 @@ suite('DataScience Native Editor', () => { }; })(originalPlatform) ); + suiteTeardown(async () => { + return DataScienceIocContainer.suiteDispose(); + }); [false, true].forEach((useCustomEditorApi) => { //import { asyncDump } from '../common/asyncDump'; - let snapshot: any; suite(`${useCustomEditorApi ? 'With' : 'Without'} Custom Editor API`, () => { function createFileCell(cell: any, data: any): ICell { const newCell = { @@ -135,12 +136,6 @@ suite('DataScience Native Editor', () => { return newCell; } - suiteSetup(() => { - snapshot = takeSnapshot(); - }); - suiteTeardown(() => { - writeDiffSnapshot(snapshot, `Native ${useCustomEditorApi}`); - }); suite('Editor tests', () => { const disposables: Disposable[] = []; let ioc: DataScienceIocContainer; @@ -212,11 +207,6 @@ suite('DataScience Native Editor', () => { } }); - // Uncomment this to debug hangs on exit - // suiteTeardown(() => { - // asyncDump(); - // }); - runMountedTest('Simple text', async () => { // Create an editor so something is listening to messages const { mount } = await createNewEditor(ioc); diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts index b163bcdb8f87..129cebb673b0 100644 --- a/src/test/datascience/notebook.functional.test.ts +++ b/src/test/datascience/notebook.functional.test.ts @@ -50,13 +50,16 @@ import { concatMultilineString } from '../../datascience-ui/common'; import { generateTestState, ICellViewModel } from '../../datascience-ui/interactive-common/mainState'; import { sleep } from '../core'; import { DataScienceIocContainer } from './dataScienceIocContainer'; -import { takeSnapshot, writeDiffSnapshot } from './helpers'; import { SupportedCommands } from './mockJupyterManager'; import { MockPythonService } from './mockPythonService'; import { createPythonService, startRemoteServer } from './remoteTestHelpers'; // tslint:disable:no-any no-multiline-string max-func-body-length no-console max-classes-per-file trailing-comma suite('DataScience notebook tests', () => { + suiteTeardown(async () => { + return DataScienceIocContainer.suiteDispose(); + }); + [false, true].forEach((useRawKernel) => { suite(`${useRawKernel ? 'With Direct Kernel' : 'With Jupyter Server'}`, () => { const disposables: Disposable[] = []; @@ -65,7 +68,6 @@ suite('DataScience notebook tests', () => { let ioc: DataScienceIocContainer; let modifiedConfig = false; const baseUri = Uri.file('foo.py'); - let snapshot: any; // tslint:disable-next-line: no-function-expression setup(async function () { @@ -82,14 +84,6 @@ suite('DataScience notebook tests', () => { notebookProvider = ioc.get(INotebookProvider); }); - suiteSetup(() => { - snapshot = takeSnapshot(); - }); - - suiteTeardown(() => { - writeDiffSnapshot(snapshot, `Notebook ${useRawKernel}`); - }); - teardown(async () => { try { if (modifiedConfig) { diff --git a/src/test/datascience/plotViewer.functional.test.tsx b/src/test/datascience/plotViewer.functional.test.tsx index 970d3b15d357..ee195ea8226d 100644 --- a/src/test/datascience/plotViewer.functional.test.tsx +++ b/src/test/datascience/plotViewer.functional.test.tsx @@ -25,6 +25,9 @@ suite('DataScience PlotViewer tests', () => { ioc.registerDataScienceTypes(); await ioc.activate(); }); + suiteTeardown(async () => { + return DataScienceIocContainer.suiteDispose(); + }); function mountWebView(): ReactWrapper, React.Component> { // Setup our webview panel @@ -137,10 +140,6 @@ suite('DataScience PlotViewer tests', () => { delete (global as any).ascquireVsCodeApi; }); - suiteTeardown(() => { - // asyncDump(); - }); - async function waitForPlot(wrapper: ReactWrapper, React.Component>, svg: string): Promise { // Get a render promise with the expected number of renders const renderPromise = waitForUpdate(wrapper, MainPanel, 1); diff --git a/src/test/datascience/raw-kernel/rawKernel.functional.test.ts b/src/test/datascience/raw-kernel/rawKernel.functional.test.ts index dc23ab8bf5cb..719248e7a5f5 100644 --- a/src/test/datascience/raw-kernel/rawKernel.functional.test.ts +++ b/src/test/datascience/raw-kernel/rawKernel.functional.test.ts @@ -45,6 +45,9 @@ suite('DataScience raw kernel tests', () => { rawKernel = await connectToKernel(port); } }); + suiteTeardown(async () => { + return DataScienceIocContainer.suiteDispose(); + }); teardown(async () => { await disconnectFromKernel(); diff --git a/src/test/datascience/trustedNotebooks.functional.test.tsx b/src/test/datascience/trustedNotebooks.functional.test.tsx index 85f0e11485bb..977a436f5d39 100644 --- a/src/test/datascience/trustedNotebooks.functional.test.tsx +++ b/src/test/datascience/trustedNotebooks.functional.test.tsx @@ -145,6 +145,10 @@ suite('DataScience Notebook trust', () => { "nbformat": 4, "nbformat_minor": 2 }`; + suiteTeardown(async () => { + return DataScienceIocContainer.suiteDispose(); + }); + const addedJSON = JSON.parse(baseFile); addedJSON.cells.splice(3, 0, { cell_type: 'code', diff --git a/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts b/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts index d42afe219a20..5520e58fcede 100644 --- a/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts +++ b/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts @@ -16,7 +16,6 @@ import { Disposable } from 'vscode'; import { LocalZMQKernel } from '../../../client/common/experiments/groups'; import { sleep } from '../../../client/common/utils/async'; import { EXTENSION_ROOT_DIR } from '../../../client/constants'; -import { IS_CI_SERVER } from '../../ciConstants'; import { retryIfFail as retryIfFailOriginal } from '../../common'; import { mockedVSCodeNamespaces } from '../../vscode-mock'; import { DataScienceIocContainer } from '../dataScienceIocContainer'; @@ -39,12 +38,6 @@ use(chaiAsPromised); let ioc: DataScienceIocContainer; suiteSetup(function () { - // Skip all tests until flakiness can be resolved. - // See issue: https://github.com/microsoft/vscode-python/issues/13936 - if (IS_CI_SERVER || process.env.VSC_PYTHON_FORCE_LOGGING) { - this.skip(); - } - // These are UI tests, hence nothing to do with platforms. this.timeout(30_000); // UI Tests, need time to start jupyter. this.retries(3); // UI tests can be flaky. @@ -53,6 +46,9 @@ use(chaiAsPromised); this.skip(); } }); + suiteTeardown(async () => { + return DataScienceIocContainer.suiteDispose(); + }); setup(async function () { ioc = new DataScienceIocContainer(true); ioc.setExtensionRootPath(EXTENSION_ROOT_DIR); diff --git a/src/test/datascience/variableexplorer.functional.test.tsx b/src/test/datascience/variableexplorer.functional.test.tsx index 59830bcda74c..db8f04bda389 100644 --- a/src/test/datascience/variableexplorer.functional.test.tsx +++ b/src/test/datascience/variableexplorer.functional.test.tsx @@ -10,7 +10,6 @@ import { RunByLine } from '../../client/common/experiments/groups'; import { InteractiveWindowMessages } from '../../client/datascience/interactive-common/interactiveWindowTypes'; import { IJupyterVariable } from '../../client/datascience/types'; import { DataScienceIocContainer } from './dataScienceIocContainer'; -import { takeSnapshot, writeDiffSnapshot } from './helpers'; import { addCode, getOrCreateInteractiveWindow } from './interactiveWindowTestHelpers'; import { addCell, createNewEditor } from './nativeEditorTestHelpers'; import { openVariableExplorer, runDoubleTest, runInteractiveTest, waitForVariablesUpdated } from './testHelpers'; @@ -25,10 +24,8 @@ const rangeInclusive = require('range-inclusive'); const disposables: Disposable[] = []; let ioc: DataScienceIocContainer; let createdNotebook = false; - let snapshot: any; suiteSetup(function () { - snapshot = takeSnapshot(); // These test require python, so only run with a non-mocked jupyter const isRollingBuild = process.env ? process.env.VSCODE_PYTHON_ROLLING !== undefined : false; if (!isRollingBuild) { @@ -38,6 +35,9 @@ const rangeInclusive = require('range-inclusive'); this.skip(); } }); + suiteTeardown(async () => { + return DataScienceIocContainer.suiteDispose(); + }); setup(async () => { ioc = new DataScienceIocContainer(); @@ -61,12 +61,6 @@ const rangeInclusive = require('range-inclusive'); await ioc.dispose(); }); - // Uncomment this to debug hangs on exit - suiteTeardown(() => { - // asyncDump(); - writeDiffSnapshot(snapshot, `Variable Explorer ${runByLine}`); - }); - async function addCodeImpartial( wrapper: ReactWrapper, React.Component>, code: string, diff --git a/src/test/startPage/startPage.functional.test.tsx b/src/test/startPage/startPage.functional.test.tsx index 803a43688f81..1cf0a15f9b8c 100644 --- a/src/test/startPage/startPage.functional.test.tsx +++ b/src/test/startPage/startPage.functional.test.tsx @@ -19,6 +19,9 @@ suite('StartPage tests', () => { ioc.registerDataScienceTypes(); await ioc.activate(); }); + suiteTeardown(async () => { + return DataScienceIocContainer.suiteDispose(); + }); teardown(async () => { await ioc.dispose(); From a908fe1b0c439f1a70f95caabba608f41e6ce41d Mon Sep 17 00:00:00 2001 From: rchiodo Date: Thu, 8 Oct 2020 14:40:07 -0700 Subject: [PATCH 03/10] Make kernel launcher port round robin only for testing --- .../kernel-launcher/kernelLauncher.ts | 78 ++++++++----------- .../datascience/dataScienceIocContainer.ts | 4 - .../dataviewer.functional.test.tsx | 1 - .../datascience/debugger.functional.test.tsx | 1 - .../gotocell.functional.test.ts | 3 - .../errorHandler.functional.test.tsx | 3 - .../intellisense.functional.test.tsx | 1 - .../nativeEditorProvider.functional.test.ts | 3 - .../interactiveWindow.functional.test.tsx | 1 - ...UriProviderRegistration.functional.test.ts | 3 - .../kernelLauncher.functional.test.ts | 1 - .../datascience/liveshare.functional.test.tsx | 4 - .../nativeEditor.functional.test.tsx | 3 - .../datascience/notebook.functional.test.ts | 4 - .../plotViewer.functional.test.tsx | 3 - .../raw-kernel/rawKernel.functional.test.ts | 3 - .../trustedNotebooks.functional.test.tsx | 4 - .../uiTests/ipywidget.ui.functional.test.ts | 3 - .../variableexplorer.functional.test.tsx | 4 - .../startPage/startPage.functional.test.tsx | 4 - src/test/unittests.ts | 9 +++ 21 files changed, 43 insertions(+), 97 deletions(-) diff --git a/src/client/datascience/kernel-launcher/kernelLauncher.ts b/src/client/datascience/kernel-launcher/kernelLauncher.ts index 7944b3b21c6d..39d35c9c4c11 100644 --- a/src/client/datascience/kernel-launcher/kernelLauncher.ts +++ b/src/client/datascience/kernel-launcher/kernelLauncher.ts @@ -9,6 +9,7 @@ import * as path from 'path'; import * as portfinder from 'portfinder'; import { promisify } from 'util'; import * as uuid from 'uuid/v4'; +import { isTestExecution } from '../../common/constants'; import { traceInfo } from '../../common/logger'; import { IProcessServiceFactory } from '../../common/process/types'; import { Resource } from '../../common/types'; @@ -27,7 +28,6 @@ const PortFormatString = `kernelLauncherPortStart_{0}.tmp`; // If the selected interpreter doesn't have a kernel, it will find a kernel on disk and use that. @injectable() export class KernelLauncher implements IKernelLauncher { - private static startPortFS: number = 0; private static startPortPromise = KernelLauncher.computeStartPort(); private static nextFreePortToTryAndUsePromise = KernelLauncher.startPortPromise; constructor( @@ -37,54 +37,44 @@ export class KernelLauncher implements IKernelLauncher { ) {} // This function is public so it can be called when a test shuts down - public static cleanupStartPort() { - // Cleanup our start port file - fsextra.close(KernelLauncher.startPortFS).ignoreErrors(); - - // Destroy the file - KernelLauncher.startPortPromise - .then((p) => { - traceInfo(`Cleaning up port start file : ${p}`); - - const filePath = path.join(os.tmpdir(), PortFormatString.format(p.toString())); - return fsextra.pathExists(filePath).then((e) => { - if (e) { - return fsextra.remove(filePath); - } - }); - }) - .ignoreErrors(); + public static async cleanupStartPort() { + try { + // Destroy the file + const port = await KernelLauncher.startPortPromise; + traceInfo(`Cleaning up port start file : ${port}`); + + const filePath = path.join(os.tmpdir(), PortFormatString.format(port.toString())); + await fsextra.remove(filePath); + } catch (exc) { + // If it fails it doesn't really matter. Just a temp file + traceInfo(`Kernel port mutex failed to cleanup: `, exc); + } } private static async computeStartPort(): Promise { - // Since multiple instances of VS code may be running, write our best guess to a shared file - let portStart = 9_000; - let result = 0; - while (result === 0 && portStart < 65_000) { - try { - // Try creating a file (not worrying about two instances starting at exactly the same time. That's much less likely) - const filePath = path.join(os.tmpdir(), PortFormatString.format(portStart.toString())); - - // Create a file stream object that should fail if the file exists - KernelLauncher.startPortFS = await fsextra.open(filePath, 'wx'); - - // If that works, we have our port - result = portStart; - } catch { - // If that fails, it should mean the file already exists - portStart += 1_000; + if (isTestExecution()) { + // Since multiple instances of a test may be running, write our best guess to a shared file + let portStart = 9_000; + let result = 0; + while (result === 0 && portStart < 65_000) { + try { + // Try creating a file with the port in the name + const filePath = path.join(os.tmpdir(), PortFormatString.format(portStart.toString())); + await fsextra.open(filePath, 'wx'); + + // If that works, we have our port + result = portStart; + } catch { + // If that fails, it should mean the file already exists + portStart += 1_000; + } } - } - - traceInfo(`Computed port start for KernelLauncher is : ${result}`); + traceInfo(`Computed port start for KernelLauncher is : ${result}`); - // Before finishing setup an on exit handler for the current process. - // Note we have to do this as a proc exit handler instead of IDisposable because - // during a test run the container is destroyed over and over again. What we need - // is for the same process to always use the same start port. - process.on('exit', KernelLauncher.cleanupStartPort); - - return result; + return result; + } else { + return 9_000; + } } @captureTelemetry(Telemetry.KernelLauncherPerf) diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index 710c952ad409..cf84425ee398 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -491,10 +491,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.asyncRegistry = new AsyncDisposableRegistry(); } - // This function should be called when an entire suite is shutdown - public static async suiteDispose(): Promise { - KernelLauncher.cleanupStartPort(); - } public async dispose(): Promise { // Make sure to disable all command handling during dispose. Don't want // anything to startup again. diff --git a/src/test/datascience/dataviewer.functional.test.tsx b/src/test/datascience/dataviewer.functional.test.tsx index d205046f24ea..5bacad6dff5f 100644 --- a/src/test/datascience/dataviewer.functional.test.tsx +++ b/src/test/datascience/dataviewer.functional.test.tsx @@ -56,7 +56,6 @@ suite('DataScience DataViewer tests', () => { suiteTeardown(() => { writeDiffSnapshot(snapshot, 'DataViewer'); - return DataScienceIocContainer.suiteDispose(); }); setup(async () => { diff --git a/src/test/datascience/debugger.functional.test.tsx b/src/test/datascience/debugger.functional.test.tsx index acd40cd8cda1..f9655dcf36b1 100644 --- a/src/test/datascience/debugger.functional.test.tsx +++ b/src/test/datascience/debugger.functional.test.tsx @@ -61,7 +61,6 @@ suite('DataScience Debugger tests', () => { suiteTeardown(() => { writeDiffSnapshot(snapshot, 'Debugger'); - return DataScienceIocContainer.suiteDispose(); }); setup(async () => { diff --git a/src/test/datascience/editor-integration/gotocell.functional.test.ts b/src/test/datascience/editor-integration/gotocell.functional.test.ts index 3193de102a3e..13867648355b 100644 --- a/src/test/datascience/editor-integration/gotocell.functional.test.ts +++ b/src/test/datascience/editor-integration/gotocell.functional.test.ts @@ -47,9 +47,6 @@ suite('DataScience gotocell tests', () => { codeLensFactory = ioc.serviceManager.get(ICodeLensFactory); await ioc.activate(); }); - suiteTeardown(async () => { - return DataScienceIocContainer.suiteDispose(); - }); teardown(async () => { try { diff --git a/src/test/datascience/errorHandler.functional.test.tsx b/src/test/datascience/errorHandler.functional.test.tsx index 3d235b6d99b5..ce686aaa5914 100644 --- a/src/test/datascience/errorHandler.functional.test.tsx +++ b/src/test/datascience/errorHandler.functional.test.tsx @@ -25,9 +25,6 @@ suite('DataScience Error Handler Functional Tests', () => { ioc = modifyContainer(); return ioc.activate(); }); - suiteTeardown(async () => { - return DataScienceIocContainer.suiteDispose(); - }); teardown(async () => { sinon.restore(); diff --git a/src/test/datascience/intellisense.functional.test.tsx b/src/test/datascience/intellisense.functional.test.tsx index 079311fd260b..1ded9989b75e 100644 --- a/src/test/datascience/intellisense.functional.test.tsx +++ b/src/test/datascience/intellisense.functional.test.tsx @@ -39,7 +39,6 @@ import { ITestNativeEditorProvider } from './testNativeEditorProvider'; suiteTeardown(async () => { writeDiffSnapshot(snapshot, 'Intellisense'); - return DataScienceIocContainer.suiteDispose(); }); teardown(async () => { diff --git a/src/test/datascience/interactive-ipynb/nativeEditorProvider.functional.test.ts b/src/test/datascience/interactive-ipynb/nativeEditorProvider.functional.test.ts index ce2c608f1e01..c5e088449700 100644 --- a/src/test/datascience/interactive-ipynb/nativeEditorProvider.functional.test.ts +++ b/src/test/datascience/interactive-ipynb/nativeEditorProvider.functional.test.ts @@ -22,9 +22,6 @@ suite('DataScience - Native Editor Provider', () => { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(); }); - suiteTeardown(async () => { - return DataScienceIocContainer.suiteDispose(); - }); function createNotebookProvider() { return ioc.get(INotebookEditorProvider); diff --git a/src/test/datascience/interactiveWindow.functional.test.tsx b/src/test/datascience/interactiveWindow.functional.test.tsx index 87b8083be89e..a0dc8b706575 100644 --- a/src/test/datascience/interactiveWindow.functional.test.tsx +++ b/src/test/datascience/interactiveWindow.functional.test.tsx @@ -86,7 +86,6 @@ suite('DataScience Interactive Window output tests', () => { suiteTeardown(() => { writeDiffSnapshot(snapshot, 'Interactive Window'); - return DataScienceIocContainer.suiteDispose(); }); teardown(async () => { diff --git a/src/test/datascience/jupyterUriProviderRegistration.functional.test.ts b/src/test/datascience/jupyterUriProviderRegistration.functional.test.ts index 95f921d2cb26..cf5ef1b7e3e8 100644 --- a/src/test/datascience/jupyterUriProviderRegistration.functional.test.ts +++ b/src/test/datascience/jupyterUriProviderRegistration.functional.test.ts @@ -94,9 +94,6 @@ suite(`DataScience JupyterServerUriProvider tests`, () => { ioc.serviceManager.rebindInstance(IExtensions, new UriMockExtensions(ioc)); return ioc.activate(); }); - suiteTeardown(async () => { - return DataScienceIocContainer.suiteDispose(); - }); teardown(async () => { await ioc.dispose(); diff --git a/src/test/datascience/kernelLauncher.functional.test.ts b/src/test/datascience/kernelLauncher.functional.test.ts index 8cd97a2946f2..d74d66c425fd 100644 --- a/src/test/datascience/kernelLauncher.functional.test.ts +++ b/src/test/datascience/kernelLauncher.functional.test.ts @@ -62,7 +62,6 @@ suite('DataScience - Kernel Launcher', () => { suiteTeardown(() => { writeDiffSnapshot(snapshot, 'KernelLauncher'); - return DataScienceIocContainer.suiteDispose(); }); test('Launch from kernelspec', async function () { diff --git a/src/test/datascience/liveshare.functional.test.tsx b/src/test/datascience/liveshare.functional.test.tsx index db24d5330212..c3cf4e2170ba 100644 --- a/src/test/datascience/liveshare.functional.test.tsx +++ b/src/test/datascience/liveshare.functional.test.tsx @@ -68,10 +68,6 @@ suite('DataScience LiveShare tests', () => { lastErrorMessage = undefined; }); - suiteTeardown(() => { - return DataScienceIocContainer.suiteDispose(); - }); - function createContainer(role: vsls.Role): DataScienceIocContainer { const result = new DataScienceIocContainer(); result.registerDataScienceTypes(); diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index 12bb6363d715..5f5388716772 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -109,9 +109,6 @@ suite('DataScience Native Editor', () => { }; })(originalPlatform) ); - suiteTeardown(async () => { - return DataScienceIocContainer.suiteDispose(); - }); [false, true].forEach((useCustomEditorApi) => { //import { asyncDump } from '../common/asyncDump'; diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts index 129cebb673b0..f2e7a8349b2b 100644 --- a/src/test/datascience/notebook.functional.test.ts +++ b/src/test/datascience/notebook.functional.test.ts @@ -56,10 +56,6 @@ import { createPythonService, startRemoteServer } from './remoteTestHelpers'; // tslint:disable:no-any no-multiline-string max-func-body-length no-console max-classes-per-file trailing-comma suite('DataScience notebook tests', () => { - suiteTeardown(async () => { - return DataScienceIocContainer.suiteDispose(); - }); - [false, true].forEach((useRawKernel) => { suite(`${useRawKernel ? 'With Direct Kernel' : 'With Jupyter Server'}`, () => { const disposables: Disposable[] = []; diff --git a/src/test/datascience/plotViewer.functional.test.tsx b/src/test/datascience/plotViewer.functional.test.tsx index ee195ea8226d..4ce147a5437b 100644 --- a/src/test/datascience/plotViewer.functional.test.tsx +++ b/src/test/datascience/plotViewer.functional.test.tsx @@ -25,9 +25,6 @@ suite('DataScience PlotViewer tests', () => { ioc.registerDataScienceTypes(); await ioc.activate(); }); - suiteTeardown(async () => { - return DataScienceIocContainer.suiteDispose(); - }); function mountWebView(): ReactWrapper, React.Component> { // Setup our webview panel diff --git a/src/test/datascience/raw-kernel/rawKernel.functional.test.ts b/src/test/datascience/raw-kernel/rawKernel.functional.test.ts index 719248e7a5f5..dc23ab8bf5cb 100644 --- a/src/test/datascience/raw-kernel/rawKernel.functional.test.ts +++ b/src/test/datascience/raw-kernel/rawKernel.functional.test.ts @@ -45,9 +45,6 @@ suite('DataScience raw kernel tests', () => { rawKernel = await connectToKernel(port); } }); - suiteTeardown(async () => { - return DataScienceIocContainer.suiteDispose(); - }); teardown(async () => { await disconnectFromKernel(); diff --git a/src/test/datascience/trustedNotebooks.functional.test.tsx b/src/test/datascience/trustedNotebooks.functional.test.tsx index 977a436f5d39..85f0e11485bb 100644 --- a/src/test/datascience/trustedNotebooks.functional.test.tsx +++ b/src/test/datascience/trustedNotebooks.functional.test.tsx @@ -145,10 +145,6 @@ suite('DataScience Notebook trust', () => { "nbformat": 4, "nbformat_minor": 2 }`; - suiteTeardown(async () => { - return DataScienceIocContainer.suiteDispose(); - }); - const addedJSON = JSON.parse(baseFile); addedJSON.cells.splice(3, 0, { cell_type: 'code', diff --git a/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts b/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts index 5520e58fcede..991a70b16b6b 100644 --- a/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts +++ b/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts @@ -46,9 +46,6 @@ use(chaiAsPromised); this.skip(); } }); - suiteTeardown(async () => { - return DataScienceIocContainer.suiteDispose(); - }); setup(async function () { ioc = new DataScienceIocContainer(true); ioc.setExtensionRootPath(EXTENSION_ROOT_DIR); diff --git a/src/test/datascience/variableexplorer.functional.test.tsx b/src/test/datascience/variableexplorer.functional.test.tsx index db8f04bda389..88d81e797ff4 100644 --- a/src/test/datascience/variableexplorer.functional.test.tsx +++ b/src/test/datascience/variableexplorer.functional.test.tsx @@ -35,10 +35,6 @@ const rangeInclusive = require('range-inclusive'); this.skip(); } }); - suiteTeardown(async () => { - return DataScienceIocContainer.suiteDispose(); - }); - setup(async () => { ioc = new DataScienceIocContainer(); ioc.setExperimentState(RunByLine.experiment, runByLine); diff --git a/src/test/startPage/startPage.functional.test.tsx b/src/test/startPage/startPage.functional.test.tsx index 1cf0a15f9b8c..a83fd7fc0a4e 100644 --- a/src/test/startPage/startPage.functional.test.tsx +++ b/src/test/startPage/startPage.functional.test.tsx @@ -19,10 +19,6 @@ suite('StartPage tests', () => { ioc.registerDataScienceTypes(); await ioc.activate(); }); - suiteTeardown(async () => { - return DataScienceIocContainer.suiteDispose(); - }); - teardown(async () => { await ioc.dispose(); }); diff --git a/src/test/unittests.ts b/src/test/unittests.ts index 066d4b628012..e3a3d0046133 100644 --- a/src/test/unittests.ts +++ b/src/test/unittests.ts @@ -82,4 +82,13 @@ if (process.argv.indexOf('--fast') === -1) { setupTranspile(); } +exports.mochaHooks = { + afterAll() { + const kernelLauncherMod = require('../client/datascience/kernel-launcher/kernelLauncher'); + + // After all tests run, clean up the kernel launcher mutex files + return kernelLauncherMod.KernelLauncher.cleanupStartPort(); + } +}; + initialize(); From 629e881417c35e2ed9d6fa4394503be2a9225024 Mon Sep 17 00:00:00 2001 From: rchiodo Date: Thu, 8 Oct 2020 14:47:36 -0700 Subject: [PATCH 04/10] Make formatters change only apply during testing --- src/client/logging/formatters.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/client/logging/formatters.ts b/src/client/logging/formatters.ts index 734746a246c1..488c4c91ccaf 100644 --- a/src/client/logging/formatters.ts +++ b/src/client/logging/formatters.ts @@ -3,6 +3,7 @@ 'use strict'; import { format } from 'winston'; +import { isTestExecution } from '../common/constants'; import { getLevel, LogLevel, LogLevelName } from './levels'; const TIMESTAMP = 'YYYY-MM-DD HH:mm:ss'; @@ -37,13 +38,17 @@ function normalizeLevel(name: LogLevelName): string { // Return a log entry that can be emitted as-is. function formatMessage(level: LogLevelName, timestamp: string, message: string): string { const levelFormatted = normalizeLevel(level); - return `${process.pid} ${levelFormatted} ${timestamp}: ${message}`; + return isTestExecution() + ? `${process.pid} ${levelFormatted} ${timestamp}: ${message}` + : `${levelFormatted} ${timestamp}: ${message}`; } // Return a log entry that can be emitted as-is. function formatLabeledMessage(level: LogLevelName, timestamp: string, label: string, message: string): string { const levelFormatted = normalizeLevel(level); - return `${process.pid} ${levelFormatted} ${label} ${timestamp}: ${message}`; + return isTestExecution() + ? `${process.pid} ${levelFormatted} ${label} ${timestamp}: ${message}` + : `${levelFormatted} ${label} ${timestamp}: ${message}`; } // Return a minimal format object that can be used with a "winston" From e64ff0b99b3fdd7ff418407fd32911768ec57598 Mon Sep 17 00:00:00 2001 From: rchiodo Date: Thu, 8 Oct 2020 14:49:15 -0700 Subject: [PATCH 05/10] Add news entry --- news/3 Code Health/14290.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/3 Code Health/14290.md diff --git a/news/3 Code Health/14290.md b/news/3 Code Health/14290.md new file mode 100644 index 000000000000..8e7fac32f023 --- /dev/null +++ b/news/3 Code Health/14290.md @@ -0,0 +1 @@ +Functional test failures related to kernel ports overlapping. \ No newline at end of file From 30aaacd8d6c6d64e4bd641f533bcbb75ae44de57 Mon Sep 17 00:00:00 2001 From: rchiodo Date: Thu, 8 Oct 2020 15:04:21 -0700 Subject: [PATCH 06/10] Apply black formatting --- .../tests/logParser.py | 55 +++++++++++-------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/pythonFiles/vscode_datascience_helpers/tests/logParser.py b/pythonFiles/vscode_datascience_helpers/tests/logParser.py index 16e6437823b2..1192ef97b235 100644 --- a/pythonFiles/vscode_datascience_helpers/tests/logParser.py +++ b/pythonFiles/vscode_datascience_helpers/tests/logParser.py @@ -1,16 +1,24 @@ import sys import argparse import os + os.system("color") from pathlib import Path import re -parser = argparse.ArgumentParser(description='Parse a test log into its parts') -parser.add_argument('testlog', type=str, nargs=1, help='Log to parse') -parser.add_argument('--testoutput', action='store_true', help='Show all failures and passes') -parser.add_argument('--split', action='store_true', help='Split into per process files. Each file will have the pid appended') -ansi_escape = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])') -pid_regex = re.compile(r'(\d+).*') +parser = argparse.ArgumentParser(description="Parse a test log into its parts") +parser.add_argument("testlog", type=str, nargs=1, help="Log to parse") +parser.add_argument( + "--testoutput", action="store_true", help="Show all failures and passes" +) +parser.add_argument( + "--split", + action="store_true", + help="Split into per process files. Each file will have the pid appended", +) +ansi_escape = re.compile(r"\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])") +pid_regex = re.compile(r"(\d+).*") + def printTestOutput(testlog): # Find all the lines that don't have a PID in them. These are the test output @@ -18,8 +26,9 @@ def printTestOutput(testlog): with p.open() as f: for line in f.readlines(): stripped = line.strip() - if (len(stripped) > 2 and stripped[0] == '\x1B' and stripped[1] == '['): - print(line.rstrip()) # Should be a test line as it has color encoding + if len(stripped) > 2 and stripped[0] == "\x1B" and stripped[1] == "[": + print(line.rstrip()) # Should be a test line as it has color encoding + def splitByPid(testlog): # Split testlog into prefixed logs based on pid @@ -30,46 +39,46 @@ def splitByPid(testlog): pid = None with p.open() as f: for line in f.readlines(): - stripped = ansi_escape.sub('', line.strip()) + stripped = ansi_escape.sub("", line.strip()) # See if starts with a pid - if (len(stripped) > 0 and stripped[0] <= '9' and stripped[0] >= '0'): + if len(stripped) > 0 and stripped[0] <= "9" and stripped[0] >= "0": # Pull out the pid match = pid_regex.match(stripped) # Pids are at least two digits - if (match != None and len(match.group(1)) > 2): + if match != None and len(match.group(1)) > 2: # Pid is found pid = int(match.group(1)) # See if we've created a log for this pid or not - if (not pid in pids): + if not pid in pids: pids.add(pid) - logFile = '{}_{}.log'.format(baseFile, pid) - print('Writing to new log: ' + logFile) + logFile = "{}_{}.log".format(baseFile, pid) + print("Writing to new log: " + logFile) logs[pid] = Path(logFile).open(mode="w") - + # Add this line to the log - if (pid != None): + if pid != None: logs[pid].write(line) # Close all of the open logs for key in logs: logs[key].close() - - def doWork(args): - if (not args.testlog): - print('Test log should be passed') - elif (args.testoutput): + if not args.testlog: + print("Test log should be passed") + elif args.testoutput: printTestOutput(args.testlog) - elif (args.split): + elif args.split: splitByPid(args.testlog) else: parser.print_usage() + def main(): doWork(parser.parse_args()) + if __name__ == "__main__": - main() \ No newline at end of file + main() From cd0fe6226c76df44719c6b7da0a760f8499c58fb Mon Sep 17 00:00:00 2001 From: rchiodo Date: Thu, 8 Oct 2020 15:37:26 -0700 Subject: [PATCH 07/10] Code review feedback and skip flakey remote password test --- .vscode/settings.json | 2 +- src/client/datascience/kernel-launcher/kernelLauncher.ts | 4 ++-- src/test/datascience/notebook.functional.test.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 9f729bd8127e..b2dc13efe26e 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -45,7 +45,7 @@ "source.fixAll.eslint": true, "source.fixAll.tslint": true }, - "python.languageServer": "Pylance", + "python.languageServer": "Microsoft", "python.analysis.logLevel": "Trace", "python.analysis.downloadChannel": "beta", "python.linting.pylintEnabled": false, diff --git a/src/client/datascience/kernel-launcher/kernelLauncher.ts b/src/client/datascience/kernel-launcher/kernelLauncher.ts index 39d35c9c4c11..34db9842f653 100644 --- a/src/client/datascience/kernel-launcher/kernelLauncher.ts +++ b/src/client/datascience/kernel-launcher/kernelLauncher.ts @@ -96,7 +96,7 @@ export class KernelLauncher implements IKernelLauncher { return kernelProcess; } - private async getPorts(): Promise { + private async getConnectionPorts(): Promise { const getPorts = promisify(portfinder.getPorts); // Have to wait for static port lookup (it handles case where two VS code instances are running) @@ -117,7 +117,7 @@ export class KernelLauncher implements IKernelLauncher { } private async getKernelConnection(): Promise { - const ports = await this.getPorts(); + const ports = await this.getConnectionPorts(); return { version: 1, key: uuid(), diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts index f2e7a8349b2b..308c74597f29 100644 --- a/src/test/datascience/notebook.functional.test.ts +++ b/src/test/datascience/notebook.functional.test.ts @@ -468,7 +468,7 @@ suite('DataScience notebook tests', () => { runTest('Remote Password', async () => { const pythonService = await createPythonService(ioc); - if (pythonService && !useRawKernel && os.platform() !== 'darwin') { + if (pythonService && !useRawKernel && os.platform() !== 'darwin' && os.platform() !== 'linux') { const configFile = path.join( EXTENSION_ROOT_DIR, 'src', From d4e19f7f96a943bdd95fddf8e87b29bb3795e897 Mon Sep 17 00:00:00 2001 From: rchiodo Date: Thu, 8 Oct 2020 16:03:33 -0700 Subject: [PATCH 08/10] Another flakey test --- src/test/datascience/notebook.functional.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts index 308c74597f29..e25a4b46f59b 100644 --- a/src/test/datascience/notebook.functional.test.ts +++ b/src/test/datascience/notebook.functional.test.ts @@ -670,8 +670,12 @@ suite('DataScience notebook tests', () => { // Make sure we have a cell in our results assert.ok(/#\s*%%/.test(results), 'No cells in returned import'); } finally { - importer.dispose(); - temp.dispose(); + try { + importer.dispose(); + temp.dispose(); + } catch { + // Don't care if they don't delete + } } }); From 56076da56e1aeff1a63195f7de7f6a22b08bd6aa Mon Sep 17 00:00:00 2001 From: rchiodo Date: Thu, 8 Oct 2020 16:05:15 -0700 Subject: [PATCH 09/10] More CR feedback --- src/client/common/process/baseDaemon.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/common/process/baseDaemon.ts b/src/client/common/process/baseDaemon.ts index 0628e96e9006..1c4a13b5157b 100644 --- a/src/client/common/process/baseDaemon.ts +++ b/src/client/common/process/baseDaemon.ts @@ -177,7 +177,7 @@ export abstract class BasePythonDaemon { return Object.keys(options).every((item) => daemonSupportedSpawnOptions.indexOf(item as any) >= 0); } protected sendRequestWithoutArgs(type: RequestType0): Thenable { - if (this.proc && this.proc.exitCode === null) { + if (this.proc && typeof this.proc.exitCode !== 'number') { return Promise.race([this.connection.sendRequest(type), this.connectionClosedDeferred.promise]); } return this.connectionClosedDeferred.promise; @@ -186,7 +186,7 @@ export abstract class BasePythonDaemon { if (!this.isAlive || this.proc.exitCode !== null) { traceError('Daemon is handling a request after death.'); } - if (this.proc && this.proc.exitCode === null) { + if (this.proc && typeof this.proc.exitCode !== 'number') { // Throw an error if the connection has been closed. return Promise.race([this.connection.sendRequest(type, params), this.connectionClosedDeferred.promise]); } From 688fb5820001a4da3974ebf10dee4f606d757bc0 Mon Sep 17 00:00:00 2001 From: rchiodo Date: Thu, 8 Oct 2020 16:05:52 -0700 Subject: [PATCH 10/10] Missed a spot --- src/client/common/process/baseDaemon.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/common/process/baseDaemon.ts b/src/client/common/process/baseDaemon.ts index 1c4a13b5157b..b95991952fe4 100644 --- a/src/client/common/process/baseDaemon.ts +++ b/src/client/common/process/baseDaemon.ts @@ -183,7 +183,7 @@ export abstract class BasePythonDaemon { return this.connectionClosedDeferred.promise; } protected sendRequest(type: RequestType, params?: P): Thenable { - if (!this.isAlive || this.proc.exitCode !== null) { + if (!this.isAlive || typeof this.proc.exitCode === 'number') { traceError('Daemon is handling a request after death.'); } if (this.proc && typeof this.proc.exitCode !== 'number') {