debt - more polish in global test reporter (#192774)

* debt - more polish in global test reporter

* cleanup

* cleanup

* fixes

src/vs/base/test/common/lifecycle.test.ts

@@ -231,7 +231,7 @@ suite('No Leakage Utilities', () => {
 					eventEmitter.event(() => {
 						// noop
 					});
-				});
+				}, false);
 			}, e => e.message.indexOf('undisposed disposables') !== -1);
 		});
 
@@ -239,7 +239,7 @@ suite('No Leakage Utilities', () => {
 			assertThrows(() => {
 				throwIfDisposablesAreLeaked(() => {
 					new DisposableStore();
-				});
+				}, false);
 			}, e => e.message.indexOf('undisposed disposables') !== -1);
 		});
 

src/vs/base/test/common/utils.ts

@@ -109,7 +109,7 @@ export class DisposableTracker implements IDisposableTracker {
 		return leaking;
 	}
 
-	ensureNoLeakingDisposables() {
+	ensureNoLeakingDisposables(logToConsole = true) {
 		const rootParentCache = new Map<DisposableInfo, DisposableInfo>();
 
 		const leakingObjects = [...this.livingDisposables.values()]
@@ -184,7 +184,9 @@ export class DisposableTracker implements IDisposableTracker {
 			message += `\n\n\n... and ${uncoveredLeakingObjs.length - maxReported} more leaking disposables\n\n`;
 		}
 
-		console.error(message);
+		if (logToConsole) {
+			console.error(message);
+		}
 
 		throw new Error(`There are ${uncoveredLeakingObjs.length} undisposed disposables!${message}`);
 	}
@@ -225,12 +227,12 @@ export function ensureNoDisposablesAreLeakedInTestSuite(): Pick<DisposableStore,
 	return testContext;
 }
 
-export function throwIfDisposablesAreLeaked(body: () => void): void {
+export function throwIfDisposablesAreLeaked(body: () => void, logToConsole = true): void {
 	const tracker = new DisposableTracker();
 	setDisposableTracker(tracker);
 	body();
 	setDisposableTracker(null);
-	tracker.ensureNoLeakingDisposables();
+	tracker.ensureNoLeakingDisposables(logToConsole);
 }
 
 export async function throwIfDisposablesAreLeakedAsync(body: () => Promise<void>): Promise<void> {

src/vs/workbench/api/test/browser/extHostLanguageFeatures.test.ts

@@ -69,7 +69,7 @@ suite('ExtHostLanguageFeatures', function () {
 	let originalErrorHandler: (e: any) => any;
 	let instantiationService: TestInstantiationService;
 
-	suiteSetup(() => {
+	setup(() => {
 
 		model = createTextModel(
 			[
@@ -137,15 +137,14 @@ suite('ExtHostLanguageFeatures', function () {
 		mainThread = rpcProtocol.set(MainContext.MainThreadLanguageFeatures, disposables.add(inst.createInstance(MainThreadLanguageFeatures, rpcProtocol)));
 	});
 
-	suiteTeardown(() => {
+	teardown(() => {
+		disposables.clear();
+
 		setUnexpectedErrorHandler(originalErrorHandler);
 		model.dispose();
 		mainThread.dispose();
 		instantiationService.dispose();
-	});
 
-	teardown(() => {
-		disposables.clear();
 		return rpcProtocol.sync();
 	});
 

src/vs/workbench/services/editor/test/browser/editorService.test.ts

@@ -1579,7 +1579,7 @@ suite('EditorService', () => {
 	test('openEditors() extracts proper resources from untyped editors for workspace trust', async () => {
 		const [, service, accessor] = await createEditorService();
 
-		const input = { resource: URI.parse('my://resource-openEditors') };
+		const input = { resource: URI.file('resource-openEditors') };
 		const otherInput: IResourceDiffEditorInput = {
 			original: { resource: URI.parse('my://resource2-openEditors') },
 			modified: { resource: URI.parse('my://resource3-openEditors') }

src/vs/workbench/services/extensions/test/browser/extensionService.test.ts

@@ -222,6 +222,7 @@ suite('ExtensionService', () => {
 
 	setup(() => {
 		disposables = new DisposableStore();
+		const testProductService = { _serviceBrand: undefined, ...product };
 		disposables.add(instantiationService = createServices(disposables, [
 			// custom
 			[IExtensionService, MyTestExtensionService],
@@ -235,7 +236,7 @@ suite('ExtensionService', () => {
 			[IExtensionManifestPropertiesService, ExtensionManifestPropertiesService],
 			[IConfigurationService, TestConfigurationService],
 			[IWorkspaceContextService, TestContextService],
-			[IProductService, { _serviceBrand: undefined, ...product }],
+			[IProductService, testProductService],
 			[IFileService, TestFileService],
 			[IWorkbenchExtensionEnablementService, TestWorkbenchExtensionEnablementService],
 			[ITelemetryService, NullTelemetryService],
@@ -245,7 +246,7 @@ suite('ExtensionService', () => {
 			[IUserDataProfileService, TestUserDataProfileService],
 			[IUriIdentityService, UriIdentityService],
 			[IRemoteExtensionsScannerService, TestRemoteExtensionsScannerService],
-			[IRemoteAuthorityResolverService, RemoteAuthorityResolverService]
+			[IRemoteAuthorityResolverService, new RemoteAuthorityResolverService(false, undefined, undefined, testProductService, new NullLogService())]
 		]));
 		extService = <MyTestExtensionService>instantiationService.get(IExtensionService);
 	});

src/vs/workbench/test/common/utils.ts

@@ -18,5 +18,5 @@ export function assertCleanState(): void {
 	// If this test fails, it is a clear indication that
 	// your test or suite is leaking services (e.g. via leaking text models)
 	// assert.strictEqual(LanguageService.instanceCount, 0, 'No leaking ILanguageService');
-	assert.strictEqual(LanguagesRegistry.instanceCount, 0, 'No leaking LanguagesRegistry');
+	assert.strictEqual(LanguagesRegistry.instanceCount, 0, 'Error: Test run should not leak in LanguagesRegistry.');
 }

test/unit/electron/renderer.html

@@ -23,16 +23,6 @@
 		window.alert = function () { throw new Error('window.alert() is not supported in tests!'); }
 		window.confirm = function () { throw new Error('window.confirm() is not supported in tests!'); }
 
-		// Ignore uncaught cancelled promise errors
-		window.addEventListener('unhandledrejection', e => {
-			const name = e && e.reason && e.reason.name;
-
-			if (name === 'Canceled') {
-				e.preventDefault();
-				e.stopPropagation();
-			}
-		});
-
 		mocha.setup({
 			ui: 'tdd',
 			timeout: typeof process.env['BUILD_ARTIFACTSTAGINGDIRECTORY'] === 'string' ? 30000 : 5000,

test/unit/electron/renderer.js

@@ -166,49 +166,57 @@ function loadTestModules(opts) {
 	}).then(loadModules);
 }
 
-let currentSuiteTitle;
 let currentTestTitle;
 
-const allowedTestOutput = new Set([
-	'The vm module of Node.js is deprecated in the renderer process and will be removed.',
-]);
-
-const allowedTestsWithOutput = new Set([
-	'throws if an event subscription is not cleaned up',
-	'throws if a disposable is not disposed',
-	'creates a snapshot',
-	'validates a snapshot',
-	'cleans up old snapshots',
-	'issue #149412: VS Code hangs when bad semantic token data is received',
-	'issue #134973: invalid semantic tokens should be handled better',
-	'issue #148651: VSCode UI process can hang if a semantic token with negative values is returned by language service',
-	'issue #149130: vscode freezes because of Bracket Pair Colorization',
-	'property limits',
-	'Error events',
-	'Ensure output channel is logged to',
-	'guards calls after runs are ended'
-]);
-
-const allowedSuitesWithOutput = new Set([
-	'ExtHostLanguageFeatures'
-]);
-
 function loadTests(opts) {
 
+	//#region Unexpected Output
+
+	const _allowedTestOutput = new Set([
+		'The vm module of Node.js is deprecated in the renderer process and will be removed.',
+	]);
+
+	const _allowedTestsWithOutput = new Set([
+		'creates a snapshot', // https://github.com/microsoft/vscode/issues/192439
+		'validates a snapshot', // https://github.com/microsoft/vscode/issues/192439
+		'cleans up old snapshots', // https://github.com/microsoft/vscode/issues/192439
+		'issue #149412: VS Code hangs when bad semantic token data is received', // https://github.com/microsoft/vscode/issues/192440
+		'issue #134973: invalid semantic tokens should be handled better', // https://github.com/microsoft/vscode/issues/192440
+		'issue #148651: VSCode UI process can hang if a semantic token with negative values is returned by language service', // https://github.com/microsoft/vscode/issues/192440
+		'issue #149130: vscode freezes because of Bracket Pair Colorization', // https://github.com/microsoft/vscode/issues/192440
+		'property limits', // https://github.com/microsoft/vscode/issues/192443
+		'Error events', // https://github.com/microsoft/vscode/issues/192443
+		'Ensure output channel is logged to', // https://github.com/microsoft/vscode/issues/192443
+		'guards calls after runs are ended' // https://github.com/microsoft/vscode/issues/192468
+	]);
+
+	let _testsWithUnexpectedOutput = false;
+
+	for (const consoleFn of [console.log, console.error, console.info, console.warn, console.trace, console.debug]) {
+		console[consoleFn.name] = function (msg) {
+			if (!_allowedTestOutput.has(msg) && !_allowedTestsWithOutput.has(currentTestTitle)) {
+				_testsWithUnexpectedOutput = true;
+				consoleFn.apply(console, arguments);
+			}
+		};
+	}
+
+	//#endregion
+
+	//#region Unexpected / Loader Errors
+
 	const _unexpectedErrors = [];
 	const _loaderErrors = [];
 
-	const testsWithUnexpectedOutput = new Set();
-	for (const consoleFn of [console.log, console.error, console.info, console.warn]) {
-		console[consoleFn.name] = function (msg) {
-			if (!allowedTestOutput.has(msg) && !allowedTestsWithOutput.has(currentTestTitle) && !allowedSuitesWithOutput.has(currentSuiteTitle)) {
-				testsWithUnexpectedOutput.add(`${currentSuiteTitle} - ${currentTestTitle}`);
-			}
-			consoleFn.apply(console, arguments);
-		};
-	}
+	const _allowedTestsWithUnhandledRejections = new Set([
+		// Lifecycle tests
+		'onWillShutdown - join with error is handled',
+		'onBeforeShutdown - veto with error is treated as veto',
+		'onBeforeShutdown - final veto with error is treated as veto',
+		// Search tests
+		'Search Model: Search reports timed telemetry on search when error is called'
+	]);
 
-	// collect loader errors
 	loader.require.config({
 		onError(err) {
 			_loaderErrors.push(err);
@@ -216,8 +224,22 @@ function loadTests(opts) {
 		}
 	});
 
-	// collect unexpected errors
 	loader.require(['vs/base/common/errors'], function (errors) {
+
+		process.on('uncaughtException', error => errors.onUnexpectedError(error));
+		process.on('unhandledRejection', (reason, promise) => {
+			errors.onUnexpectedError(reason);
+			promise.catch(() => {});
+		});
+		window.addEventListener('unhandledrejection', event => {
+			event.preventDefault(); // Do not log to test output, we show an error later when test ends
+			event.stopPropagation();
+
+			if (!_allowedTestsWithUnhandledRejections.has(currentTestTitle)) {
+				errors.onUnexpectedError(event.reason);
+			}
+		});
+
 		errors.setUnexpectedErrorHandler(function (err) {
 			let stack = (err ? err.stack : null);
 			if (!stack) {
@@ -228,6 +250,8 @@ function loadTests(opts) {
 		});
 	});
 
+	//#endregion
+
 	return loadWorkbenchTestingUtilsModule().then((workbenchTestingModule) => {
 		const assertCleanState = workbenchTestingModule.assertCleanState;
 
@@ -237,32 +261,30 @@ function loadTests(opts) {
 			});
 		});
 
-		return loadTestModules(opts).then(() => {
-			suite('Unexpected Errors & Loader Errors', function () {
-				test('should not have unexpected errors', function () {
-					const errors = _unexpectedErrors.concat(_loaderErrors);
-					if (errors.length) {
-						errors.forEach(function (stack) {
-							console.error('');
-							console.error(stack);
-						});
-						assert.ok(false, errors);
-					}
-				});
+		teardown(() => {
 
-				test('assertCleanState - check that registries are clean and objects are disposed at the end of test running', () => {
-					assertCleanState();
-				});
-			});
+			// should not have unexpected output
+			if (_testsWithUnexpectedOutput) {
+				assert.ok(false, 'Error: Unexpected console output in test run. Please ensure no console.[log|error|info|warn] usage in tests or runtime errors.');
+			}
 
-			suite('Unexpected Output', function () {
-				test('should not have unexpected output', function () {
-					if (testsWithUnexpectedOutput.size > 0) {
-						assert.ok(false, `Tests with unexpected output:\n${Array.from(testsWithUnexpectedOutput).join('\n')}`);
-					}
-				});
-			});
+			// should not have unexpected errors
+			const errors = _unexpectedErrors.concat(_loaderErrors);
+			if (errors.length) {
+				for (const error of errors) {
+					console.error(`Error: Test run should not have unexpected errors:\n${error}`);
+				}
+				assert.ok(false, 'Error: Test run should not have unexpected errors.');
+			}
 		});
+
+		suiteTeardown(() => { // intentionally not in teardown because some tests only cleanup in suiteTeardown
+
+			// should have cleaned up in registries
+			assertCleanState();
+		});
+
+		return loadTestModules(opts);
 	});
 }
 
@@ -374,7 +396,6 @@ function runTests(opts) {
 			});
 		});
 
-		runner.on('suite', suite => currentSuiteTitle = suite.title);
 		runner.on('test', test => currentTestTitle = test.title);
 
 		if (opts.dev) {