Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests - shutdown gracefully to enable webview tests #90508

Merged
merged 7 commits into from
Feb 12, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ const webviewId = 'myWebview';

const testDocument = join(vscode.workspace.rootPath || '', './bower.json');

// TODO: Re-enable after https://github.com/microsoft/vscode/issues/88415
suite.skip('Webview tests', () => {
suite('Webview tests', () => {
const disposables: vscode.Disposable[] = [];

function _register<T extends vscode.Disposable>(disposable: T) {
Expand Down
37 changes: 31 additions & 6 deletions src/vs/platform/lifecycle/electron-main/lifecycleMainService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { ipcMain as ipc, app } from 'electron';
import { ipcMain as ipc, app, BrowserWindow } from 'electron';
import { ILogService } from 'vs/platform/log/common/log';
import { IStateService } from 'vs/platform/state/node/state';
import { Event, Emitter } from 'vs/base/common/event';
Expand All @@ -12,8 +12,8 @@ import { ICodeWindow } from 'vs/platform/windows/electron-main/windows';
import { handleVetos } from 'vs/platform/lifecycle/common/lifecycle';
import { isMacintosh, isWindows } from 'vs/base/common/platform';
import { Disposable } from 'vs/base/common/lifecycle';
import { Barrier } from 'vs/base/common/async';
import { ParsedArgs } from 'vs/platform/environment/common/environment';
import { Barrier, timeout } from 'vs/base/common/async';
import { ParsedArgs, IEnvironmentService } from 'vs/platform/environment/common/environment';

export const ILifecycleMainService = createDecorator<ILifecycleMainService>('lifecycleMainService');

Expand Down Expand Up @@ -106,7 +106,7 @@ export interface ILifecycleMainService {
/**
* Forcefully shutdown the application. No livecycle event handlers are triggered.
*/
kill(code?: number): void;
kill(code?: number): Promise<void>;

/**
* Returns a promise that resolves when a certain lifecycle phase
Expand Down Expand Up @@ -175,7 +175,8 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe

constructor(
@ILogService private readonly logService: ILogService,
@IStateService private readonly stateService: IStateService
@IStateService private readonly stateService: IStateService,
@IEnvironmentService private readonly environmentService: IEnvironmentService
) {
super();

Expand Down Expand Up @@ -550,9 +551,33 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
this.quit().then(veto => quitVetoed = veto);
}

kill(code?: number): void {
async kill(code?: number): Promise<void> {
this.logService.trace('Lifecycle#kill()');

// API tests: we have seen issues (native crashes) when calling app.exit()
// without closing the test window properly. app.exit() is not recommended
// when having a window open, so for tests running we make sure to close the
// window. Since we do not want to block the tests from completing longer
// than needed, we set a race of 1s to exit anyway.
//
// Note: Electron implements a similar logic here:
// https://github.com/electron/electron/blob/fe5318d753637c3903e23fc1ed1b263025887b6a/spec-main/window-helpers.ts#L5
//
if (this.environmentService.isExtensionDevelopment && !!this.environmentService.extensionTestsLocationURI && this.windowCounter === 1) {
await Promise.race([
timeout(1000),
new Promise(c => {
const testWindow = BrowserWindow.getAllWindows()[0];
if (testWindow && !testWindow.isDestroyed()) {
testWindow.on('closed', () => c());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would also want to check the validity of testWindow.webContents and listen to the event accordingly for the reasons mentioned here https://github.com/electron/electron/blob/master/spec-main/window-helpers.ts#L7-L20

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deepak1556 ok see my latest commit

testWindow.destroy();
} else {
c();
}
})
]);
}

app.exit(code);
}
}