Skip to content

Commit 72c25f0

Browse files
committed
Fix Playwright tunnel restart race
1 parent bf1381b commit 72c25f0

9 files changed

Lines changed: 378 additions & 15 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "playwright-local-browser-server",
5+
"comment": "Fix a race where restarting the Playwright tunnel could create a replacement before the previous tunnel finished stopping.",
6+
"type": "patch"
7+
}
8+
],
9+
"packageName": "playwright-local-browser-server",
10+
"email": "9123665+justinTM@users.noreply.github.com"
11+
}

common/config/subspaces/default/pnpm-lock.yaml

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"extends": "@rushstack/heft-vscode-extension-rig/profiles/default/config/jest.config.json",
3+
"testPathIgnorePatterns": ["/node_modules/", "/lib/vscodeTest/"]
4+
}

vscode-extensions/playwright-local-browser-server-vscode-extension/package.json

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@
3434
"build": "heft build --clean",
3535
"build:watch": "heft build-watch",
3636
"start": "heft start",
37+
"test:vscode": "node ./lib/vscodeTest/runTest.js",
3738
"_phase:build": "heft run --only build -- --clean",
38-
"_phase:test": ""
39+
"_phase:test": "heft run --only test -- --clean"
3940
},
4041
"contributes": {
4142
"commands": [
@@ -106,9 +107,14 @@
106107
"@rushstack/heft-vscode-extension-rig": "workspace:*",
107108
"@rushstack/heft-node-rig": "workspace:*",
108109
"@rushstack/heft": "workspace:*",
110+
"@types/glob": "7.1.1",
111+
"@types/mocha": "10.0.6",
109112
"@types/node": "20.17.19",
110113
"@types/vscode": "1.103.0",
111-
"@types/webpack-env": "1.18.8"
114+
"@types/webpack-env": "1.18.8",
115+
"@vscode/test-electron": "^1.6.2",
116+
"glob": "~7.0.5",
117+
"mocha": "^10.1.0"
112118
},
113119
"sideEffects": false
114120
}

vscode-extensions/playwright-local-browser-server-vscode-extension/src/extension.ts

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
157157

158158
// Tunnel instance
159159
let tunnel: PlaywrightTunnel | undefined;
160+
let stopTunnelPromise: Promise<void> | undefined;
160161

161162
function getTmpPath(): string {
162163
return path.join(os.tmpdir(), 'playwright-browser-tunnel');
@@ -290,6 +291,12 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
290291
}
291292

292293
async function handleStartTunnelAsync(isAutoStart: boolean = false): Promise<void> {
294+
const pendingStopPromise: Promise<void> | undefined = stopTunnelPromise;
295+
if (pendingStopPromise) {
296+
outputChannel.appendLine('Waiting for Playwright tunnel to stop before starting again...');
297+
await pendingStopPromise;
298+
}
299+
293300
// Store current tunnel reference to avoid race conditions
294301
const existingTunnel: PlaywrightTunnel | undefined = tunnel;
295302
if (existingTunnel && currentStatus !== 'stopped' && currentStatus !== 'error') {
@@ -439,27 +446,39 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
439446
}
440447

441448
async function handleStopTunnelAsync(): Promise<void> {
449+
const pendingStopPromise: Promise<void> | undefined = stopTunnelPromise;
450+
if (pendingStopPromise) {
451+
outputChannel.appendLine('Tunnel stop already in progress.');
452+
await pendingStopPromise;
453+
return;
454+
}
455+
442456
const currentTunnel: PlaywrightTunnel | undefined = tunnel;
443457
if (!currentTunnel) {
444458
outputChannel.appendLine('No tunnel instance to stop.');
445459
void vscode.window.showInformationMessage('Playwright tunnel is not running.');
446460
return;
447461
}
448462

449-
// Clear the reference before awaiting to avoid race condition
450-
tunnel = undefined;
451-
452-
try {
463+
const currentStopPromise: Promise<void> = (async () => {
453464
outputChannel.appendLine('Stopping Playwright tunnel...');
454-
await currentTunnel.stopAsync();
455-
updateStatusBar('stopped');
456-
outputChannel.appendLine('Tunnel stopped.');
457-
void vscode.window.showInformationMessage('Playwright tunnel stopped.');
458-
} catch (error) {
459-
const errorMessage: string = getNormalizedErrorString(error);
460-
outputChannel.appendLine(`Failed to stop tunnel: ${errorMessage}`);
461-
void vscode.window.showErrorMessage(`Failed to stop Playwright tunnel: ${errorMessage}`);
462-
}
465+
try {
466+
await currentTunnel.stopAsync();
467+
tunnel = undefined;
468+
updateStatusBar('stopped');
469+
outputChannel.appendLine('Tunnel stopped.');
470+
void vscode.window.showInformationMessage('Playwright tunnel stopped.');
471+
} catch (error) {
472+
const errorMessage: string = getNormalizedErrorString(error);
473+
outputChannel.appendLine(`Failed to stop tunnel: ${errorMessage}`);
474+
void vscode.window.showErrorMessage(`Failed to stop Playwright tunnel: ${errorMessage}`);
475+
} finally {
476+
stopTunnelPromise = undefined;
477+
}
478+
})();
479+
480+
stopTunnelPromise = currentStopPromise;
481+
await currentStopPromise;
463482
}
464483

465484
async function handleShowMenu(): Promise<void> {
Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
2+
// See LICENSE in the project root for license information.
3+
4+
import type * as vscode from 'vscode';
5+
6+
type CommandHandler = (...args: unknown[]) => unknown;
7+
8+
const commandHandlers: Map<string, CommandHandler> = new Map();
9+
10+
let autoStart: boolean = false;
11+
12+
const createOutputChannelMock = jest.fn(() => ({
13+
appendLine: jest.fn(),
14+
show: jest.fn(),
15+
dispose: jest.fn()
16+
}));
17+
18+
jest.mock(
19+
'vscode',
20+
() => ({
21+
StatusBarAlignment: {
22+
Right: 2
23+
},
24+
ThemeColor: jest.fn(),
25+
Uri: {
26+
file: jest.fn((filePath: string) => ({ fsPath: filePath }))
27+
},
28+
commands: {
29+
executeCommand: jest.fn(),
30+
registerCommand: jest.fn((command: string, handler: CommandHandler) => {
31+
commandHandlers.set(command, handler);
32+
return { dispose: jest.fn() };
33+
})
34+
},
35+
env: {
36+
remoteName: undefined
37+
},
38+
window: {
39+
createOutputChannel: createOutputChannelMock,
40+
createStatusBarItem: jest.fn(() => ({
41+
show: jest.fn(),
42+
dispose: jest.fn()
43+
})),
44+
showErrorMessage: jest.fn(),
45+
showInformationMessage: jest.fn(async () => 'Just This Once'),
46+
showQuickPick: jest.fn(),
47+
showTextDocument: jest.fn(),
48+
showWarningMessage: jest.fn()
49+
},
50+
workspace: {
51+
fs: {
52+
writeFile: jest.fn()
53+
},
54+
getConfiguration: jest.fn(() => ({
55+
get: jest.fn((name: string, defaultValue: unknown) => {
56+
switch (name) {
57+
case 'autoStart':
58+
return autoStart;
59+
case 'tunnelPort':
60+
return 56767;
61+
default:
62+
return defaultValue;
63+
}
64+
}),
65+
update: jest.fn()
66+
})),
67+
workspaceFolders: undefined
68+
},
69+
ConfigurationTarget: {
70+
Global: 1
71+
}
72+
}),
73+
{ virtual: true }
74+
);
75+
76+
jest.mock('@rushstack/terminal', () => ({
77+
Terminal: jest.fn()
78+
}));
79+
80+
jest.mock('@rushstack/vscode-shared/lib/VScodeOutputChannelTerminalProvider', () => ({
81+
VScodeOutputChannelTerminalProvider: jest.fn()
82+
}));
83+
84+
jest.mock('@rushstack/vscode-shared/lib/runWorkspaceCommandAsync', () => ({
85+
runWorkspaceCommandAsync: jest.fn()
86+
}));
87+
88+
jest.mock('@rushstack/playwright-browser-tunnel/lib/LaunchOptionsValidator', () => ({
89+
LaunchOptionsValidator: {
90+
validateLaunchOptionsAsync: jest.fn(),
91+
readAllowlistAsync: jest.fn(),
92+
getAllowlistFilePath: jest.fn(),
93+
addToAllowlistAsync: jest.fn(),
94+
removeFromAllowlistAsync: jest.fn(),
95+
clearAllowlistAsync: jest.fn()
96+
}
97+
}));
98+
99+
class MockPlaywrightTunnel {
100+
public static readonly instances: MockPlaywrightTunnel[] = [];
101+
102+
private _resolveStop!: () => void;
103+
private _stopPromise: Promise<void> = new Promise((resolve) => {
104+
this._resolveStop = resolve;
105+
});
106+
107+
public readonly startAsync: jest.Mock<Promise<void>, []> = jest.fn(async () => {
108+
await new Promise(() => {
109+
// The real tunnel runs continuously until it is stopped.
110+
});
111+
});
112+
113+
public readonly stopAsync: jest.Mock<Promise<void>, []> = jest.fn(async () => {
114+
await this._stopPromise;
115+
});
116+
117+
public constructor() {
118+
MockPlaywrightTunnel.instances.push(this);
119+
}
120+
121+
public resolveStop(): void {
122+
this._resolveStop();
123+
}
124+
}
125+
126+
jest.mock('@rushstack/playwright-browser-tunnel/lib/PlaywrightBrowserTunnel', () => ({
127+
PlaywrightTunnel: MockPlaywrightTunnel
128+
}));
129+
130+
describe('playwright-local-browser-server extension lifecycle', () => {
131+
beforeEach(() => {
132+
jest.clearAllMocks();
133+
commandHandlers.clear();
134+
MockPlaywrightTunnel.instances.length = 0;
135+
autoStart = false;
136+
});
137+
138+
it('waits for an in-progress stop before starting a replacement tunnel', async () => {
139+
const { activate } = await import('../extension');
140+
141+
await activate({ subscriptions: [] } as unknown as vscode.ExtensionContext);
142+
143+
const startTunnelAsync: CommandHandler = commandHandlers.get('playwright-local-browser-server.start')!;
144+
const stopTunnelAsync: CommandHandler = commandHandlers.get('playwright-local-browser-server.stop')!;
145+
146+
await startTunnelAsync();
147+
expect(MockPlaywrightTunnel.instances).toHaveLength(1);
148+
149+
const firstTunnel: MockPlaywrightTunnel = MockPlaywrightTunnel.instances[0];
150+
const stopPromise: Promise<unknown> = Promise.resolve(stopTunnelAsync());
151+
expect(firstTunnel.stopAsync).toHaveBeenCalledTimes(1);
152+
153+
const restartPromise: Promise<unknown> = Promise.resolve(startTunnelAsync());
154+
await Promise.resolve();
155+
156+
expect(MockPlaywrightTunnel.instances).toHaveLength(1);
157+
158+
firstTunnel.resolveStop();
159+
await stopPromise;
160+
await restartPromise;
161+
162+
expect(MockPlaywrightTunnel.instances).toHaveLength(2);
163+
});
164+
});
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
2+
// See LICENSE in the project root for license information.
3+
4+
import * as os from 'node:os';
5+
import * as path from 'node:path';
6+
7+
import { runTests } from '@vscode/test-electron';
8+
9+
async function main(): Promise<void> {
10+
try {
11+
const extensionDevelopmentPath: string = path.resolve(__dirname, '../../dist/vsix/unpacked');
12+
const extensionTestsPath: string = path.resolve(__dirname, './suite/index');
13+
const testDataPath: string = path.join(os.tmpdir(), 'playwright-local-browser-server-vscode-test');
14+
15+
await runTests({
16+
vscodeExecutablePath: process.env.VSCODE_EXECUTABLE_PATH,
17+
extensionDevelopmentPath,
18+
extensionTestsPath,
19+
launchArgs: [
20+
path.join(testDataPath, 'workspace'),
21+
'--user-data-dir',
22+
path.join(testDataPath, 'user-data'),
23+
'--extensions-dir',
24+
path.join(testDataPath, 'extensions'),
25+
'--disable-workspace-trust',
26+
'--skip-welcome',
27+
'--skip-release-notes'
28+
]
29+
});
30+
} catch (error) {
31+
// eslint-disable-next-line no-console
32+
console.error('Failed to run VS Code extension tests', error);
33+
process.exit(1);
34+
}
35+
}
36+
37+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
38+
main();

0 commit comments

Comments
 (0)