chore: revert #728 and #733 (#771)

Author: dgozman

CLAUDE.md

@@ -5,27 +5,30 @@ Semantic commit messages: `label(scope): description`
 Labels: `fix`, `feat`, `chore`, `docs`, `test`, `devops`
 
 ```bash
-git checkout -b fix-39562
+git checkout -b fix-12345
 # ... make changes ...
 git add <changed-files>
 git commit -m "$(cat <<'EOF'
-fix(proxy): handle SOCKS proxy authentication
+fix: ask for 127.0.0.1 host when launching test server
 
-Fixes: https://github.com/microsoft/playwright/issues/39562
+Fixes: https://github.com/microsoft/playwright-vscode/issues/12345
 EOF
 )"
-git push origin fix-39562
-gh pr create --repo microsoft/playwright --head username:fix-39562 \
-  --title "fix(proxy): handle SOCKS proxy authentication" \
+git push origin fix-12345
+gh pr create --repo microsoft/playwright-vscdoe --head username:fix-12345 \
+  --title "fix: ask for 127.0.0.1 host when launching test server" \
   --body "$(cat <<'EOF'
 ## Summary
 - <describe the change very! briefly>
 
-Fixes https://github.com/microsoft/playwright/issues/39562
+Fixes https://github.com/microsoft/playwright/issues/12345
 EOF
 )"
 ```
 
 Never add Co-Authored-By agents in commit message.
+Never add "Generated with" in commit message.
+Never add test plan to PR description. Keep PR description short — a few bullet points at most.
 Branch naming for issue fixes: `fix-<issue-number>`
 
+**Never `git push` without an explicit instruction to push.** Applies even when a PR is already open for the branch — additional commits are immediately visible to reviewers. Commit locally, report what was committed, and wait. Only push when the user's message contains "push", "upload", "create PR", "ship it", or equivalent.

src/extension.ts

@@ -505,7 +505,7 @@ export class Extension implements RunHooks {
   }
 
   private async _runGlobalHooks(type: 'setup' | 'teardown') {
-    if (!this._models.selectedModel()?.canRunGlobalHooks(type))
+    if (!this._models.selectedModel()?.needsGlobalHooks(type))
       return 'passed';
     const request = new this._vscode.TestRunRequest();
     const testRun = this._testController.createTestRun(request);

src/playwrightTestServer.ts

@@ -61,7 +61,6 @@ export class PlaywrightTestServer {
   private _model: TestModel;
   private _testServerPromise: Promise<TestServerConnectionWrapper> | undefined;
   private _config?: JsonConfig;
-  private _globalSetupEnv: { [key: string]: string | undefined } = {};
 
   constructor(vscode: vscodeTypes.VSCode, model: TestModel, options: PlaywrightTestOptions) {
     this._vscode = vscode;
@@ -152,10 +151,10 @@ export class PlaywrightTestServer {
     const { connection } = await this._testServer();
     if (!connection)
       return 'failed';
-    return await this._runGlobalHooksInServer(connection, type, testListener, token, true);
+    return await this._runGlobalHooksInServer(connection, type, testListener, token);
   }
 
-  private async _runGlobalHooksInServer(testServer: TestServerConnection, type: 'setup' | 'teardown', testListener: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken, saveEnv: boolean): Promise<'passed' | 'failed' | 'interrupted' | 'timedout'> {
+  private async _runGlobalHooksInServer(testServer: TestServerConnection, type: 'setup' | 'teardown', testListener: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<'passed' | 'failed' | 'interrupted' | 'timedout'> {
     const teleReceiver = new TeleReporterReceiver(testListener, {
       mergeProjects: true,
       mergeTestCases: true,
@@ -166,17 +165,13 @@ export class PlaywrightTestServer {
     try {
       if (type === 'setup' && (!this._config || this._config.globalSetup))
         testListener.onStdOut?.('\x1b[2mRunning global setup if any\u2026\x1b[0m\n');
-      const result = await Promise.race([
-        type === 'setup' ? testServer.runGlobalSetup({}) : testServer.runGlobalTeardown({}).then(result => ({ ...result, env: [] })),
+      const { report, status } = await Promise.race([
+        type === 'setup' ? testServer.runGlobalSetup({}) : testServer.runGlobalTeardown({}),
         new Promise<{ status: 'interrupted', report: [] }>(f => token.onCancellationRequested(() => f({ status: 'interrupted', report: [] }))),
       ]);
-      for (const message of result.report)
+      for (const message of report)
         void teleReceiver.dispatch(message);
-      if (type === 'setup' && saveEnv && 'env' in result)
-        this._globalSetupEnv = Object.fromEntries(result.env.map(([key, value]) => [key, value ?? undefined]));
-      if (type === 'teardown' && saveEnv)
-        this._globalSetupEnv = {};
-      return result.status;
+      return status;
     } finally {
       disposable.dispose();
     }
@@ -263,7 +258,7 @@ export class PlaywrightTestServer {
     };
   }
 
-  async debugTests(request: vscodeTypes.TestRunRequest, runOptions: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken, debugGlobalSetup: boolean): Promise<void> {
+  async debugTests(request: vscodeTypes.TestRunRequest, runOptions: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<void> {
     const addressPromise = new Promise<string>(f => {
       const disposable = this._options.onStdOut(output => {
         const match = output.match(/Listening on (.*)/);
@@ -308,7 +303,6 @@ export class PlaywrightTestServer {
           ...process.env,
           CI: this._options.isUnderTest ? undefined : process.env.CI,
           ...this._options.envProvider(this._model.config.configFile),
-          ...(debugGlobalSetup ? {} : this._globalSetupEnv),
           // Reset VSCode's options that affect nested Electron.
           ELECTRON_RUN_AS_NODE: undefined,
           FORCE_COLOR: '1',
@@ -335,11 +329,9 @@ export class PlaywrightTestServer {
       if (!locations && !testIds)
         return;
 
-      if (debugGlobalSetup) {
-        const result = await this._runGlobalHooksInServer(debugTestServer, 'setup', reporter, token, false);
-        if (result !== 'passed')
-          return;
-      }
+      const result = await this._runGlobalHooksInServer(debugTestServer, 'setup', reporter, token);
+      if (result !== 'passed')
+        return;
 
       // Locations are regular expressions.
       const locationPatterns = locations ? locations.map(escapeRegex) : [];
@@ -364,8 +356,8 @@ export class PlaywrightTestServer {
       }
     } finally {
       disposables.forEach(disposable => disposable.dispose());
-      if (debugGlobalSetup && !token.isCancellationRequested && debugTestServer && !debugTestServer.isClosed())
-        await this._runGlobalHooksInServer(debugTestServer, 'teardown', reporter, token, false);
+      if (!token.isCancellationRequested && debugTestServer && !debugTestServer.isClosed())
+        await this._runGlobalHooksInServer(debugTestServer, 'teardown', reporter, token);
       if (debugTestServer) {
         // Should exit upon close automatically.
         debugTestServer.close();

src/testModel.ts

@@ -109,7 +109,7 @@ export class TestModel extends DisposableBase {
     if (!this.isEnabled)
       return;
     await this._listFiles();
-    if (configSettings && configSettings.projects.length) { // there's a race where we save config settings before projects are loaded, ignore them
+    if (configSettings) {
       let firstProject = true;
       for (const project of this.projects()) {
         const projectSettings = configSettings.projects.find(p => p.name === project.name);
@@ -120,22 +120,8 @@ export class TestModel extends DisposableBase {
         firstProject = false;
       }
     } else {
-      if (this.projects().length === 0)
-        return;
-
-      let foundFirstBrowserProject = false;
-      for (const p of this.projects()) {
-        if (isBogStandardBrowserProject(p.name)) {
-          if (foundFirstBrowserProject) {
-            p[kIsEnabled] = false;
-          } else {
-            p[kIsEnabled] = true;
-            foundFirstBrowserProject = true;
-          }
-        } else {
-          p[kIsEnabled] = true;
-        }
-      }
+      if (this.projects().length)
+        this.projects()[0][kIsEnabled] = true;
     }
   }
 
@@ -479,6 +465,14 @@ export class TestModel extends DisposableBase {
     return !this._ranGlobalSetup;
   }
 
+  needsGlobalHooks(type: 'setup' | 'teardown'): boolean {
+    if (type === 'setup' && !this._ranGlobalSetup)
+      return true;
+    if (type === 'teardown' && this._ranGlobalSetup)
+      return true;
+    return false;
+  }
+
   async runGlobalHooks(type: 'setup' | 'teardown', testListener: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<reporterTypes.FullResult['status']> {
     if (type === 'setup') {
       if (this._ranGlobalSetup && !this._embedder.settingsModel.runGlobalSetupOnEachRun.get())
@@ -533,8 +527,6 @@ export class TestModel extends DisposableBase {
     const globalSetupResult = await this.runGlobalHooks('setup', reporter, token);
     if (globalSetupResult !== 'passed')
       return;
-    if (token?.isCancellationRequested)
-      return;
 
     const externalOptions = await this._embedder.runHooks.onWillRunTests(this.config, false);
     const showBrowser = this._embedder.settingsModel.showBrowser.get() && !!externalOptions.connectWsEndpoint;
@@ -579,17 +571,8 @@ export class TestModel extends DisposableBase {
 
     this._collection._saveSettings();
 
-    // Toggling "run global setup on each run" allows user to debug global setup/teardown code.
-    const debugShouldRunGlobalSetup = !!this._embedder.settingsModel.runGlobalSetupOnEachRun.get();
-    if (debugShouldRunGlobalSetup) {
-      const globalTeardownResult = await this.runGlobalHooks('teardown', reporter, token);
-      if (globalTeardownResult !== 'passed')
-        return;
-    } else {
-      const globalSetupResult = await this.runGlobalHooks('setup', reporter, token);
-      if (globalSetupResult !== 'passed')
-        return;
-    }
+    // Underlying debugTest implementation will run the global setup.
+    await this.runGlobalHooks('teardown', reporter, token);
     if (token?.isCancellationRequested)
       return;
 
@@ -614,7 +597,7 @@ export class TestModel extends DisposableBase {
     try {
       if (token?.isCancellationRequested)
         return;
-      await this._playwrightTest.debugTests(request, options, reporter, token, debugShouldRunGlobalSetup);
+      await this._playwrightTest.debugTests(request, options, reporter, token);
     } finally {
       await this._embedder.runHooks.onDidRunTests();
     }
@@ -930,11 +913,3 @@ function isAncestorOf(root: vscodeTypes.TestItem, descendent: vscodeTypes.TestIt
 function noOverrideToUndefined<T>(value: T | 'no-override'): T | undefined {
   return value === 'no-override' ? undefined : value;
 }
-
-function isBogStandardBrowserProject(name: string) {
-  name = name.toLowerCase();
-
-  return ['chromium', 'firefox', 'webkit', 'google chrome', 'chrome', 'microsoft edge', 'edge', ].some(prefix => {
-    return name.startsWith(prefix) || name.startsWith(`mobile ${prefix}`) || name.startsWith(`desktop ${prefix}`);
-  });
-}

src/upstream/testServerInterface.ts

@@ -47,7 +47,6 @@ export interface TestServerInterface {
 
   runGlobalSetup(params: {}): Promise<{
     report: ReportEntry[],
-    env: [string, string | null][],
     status: reporterTypes.FullResult['status']
   }>;
 

tests/codegen.spec.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { connectToSharedBrowser, enableProjects, expect, test, waitForPage } from './utils';
+import { connectToSharedBrowser, expect, test, waitForPage } from './utils';
 import fs from 'node:fs';
 
 test('should generate code', async ({ activate }) => {
@@ -113,8 +113,6 @@ test('Record at Cursor should respect custom testId', async ({ activate, showBro
     `,
   });
 
-  await enableProjects(vscode, ['main']);
-
   await testController.expandTestItems(/test.spec/);
   await expect(await testController.run()).toHaveOutput('1 passed');
 

tests/debug-tests.spec.ts

@@ -52,6 +52,7 @@ test('should debug all tests', async ({ activate }) => {
         testIds: undefined
       })
     },
+    { method: 'runGlobalTeardown', params: {} },
   ]);
 });
 
@@ -94,6 +95,7 @@ test('should debug one test', async ({ activate }) => {
         testIds: [expect.any(String)]
       })
     },
+    { method: 'runGlobalTeardown', params: {} },
   ]);
 });
 
@@ -245,75 +247,32 @@ test('should run global setup before debugging', async ({ activate }, testInfo)
     }`,
     'globalSetup.ts': `
       async function globalSetup(config) {
-        console.log('RUN GLOBAL SETUP UNDER DEBUG: ' + !!process.env.VSCODE_MOCK_DEBUGGING);
+        console.log('RUN GLOBAL SETUP');
         process.env.MAGIC_NUMBER = '42';
       }
       export default globalSetup;
     `,
     'tests/test.spec.ts': `
       import { test, expect } from '@playwright/test';
       test('should pass', async () => {
-        console.log('TEST UNDER DEBUG: ' + !!process.env.VSCODE_MOCK_DEBUGGING);
         console.log('MAGIC NUMBER: ' + process.env.MAGIC_NUMBER);
         expect(process.env.MAGIC_NUMBER).toBe('42');
       });
     `
   });
 
-  const testRunPromise1 = new Promise<TestRun>(f => testController.onDidCreateTestRun(f));
   await testController.expandTestItems(/test.spec/);
-  const runFinishedPromise1 = testController.debugProfile().run(testController.findTestItems(/pass/));
-  const testRun1 = await testRunPromise1;
-  await expect(testRun1).toHaveOutput(`RUN GLOBAL SETUP UNDER DEBUG: false`);
-  await expect.poll(() => stripAnsi(vscode.debug.output)).toContain(`TEST UNDER DEBUG: true`);
-  await expect.poll(() => stripAnsi(vscode.debug.output)).toContain(`MAGIC NUMBER: 42`);
-  expect(testRun1.renderLog()).toBe(`
+  const testItems = testController.findTestItems(/pass/);
+  const testRun = await testController.debug(testItems);
+
+  expect(stripAnsi(vscode.debug.output)).toContain(`RUN GLOBAL SETUP`);
+  expect(stripAnsi(vscode.debug.output)).toContain(`MAGIC NUMBER: 42`);
+
+  expect(testRun.renderLog({ messages: true })).toBe(`
     tests > test.spec.ts > should pass [2:0]
       enqueued
       enqueued
       started
       passed
   `);
-  await runFinishedPromise1;
-
-  // Second time it should reuse the global setup and not run it again.
-  const testRunPromise2 = new Promise<TestRun>(f => testController.onDidCreateTestRun(f));
-  await testController.expandTestItems(/test.spec/);
-  const runFinishedPromise2 = testController.debugProfile().run(testController.findTestItems(/pass/));
-  const testRun2 = await testRunPromise2;
-  await expect.poll(() => stripAnsi(vscode.debug.output)).toContain(`TEST UNDER DEBUG: true`);
-  await expect.poll(() => stripAnsi(vscode.debug.output)).toContain(`MAGIC NUMBER: 42`);
-  expect(testRun2.renderOutput()).not.toContain(`RUN GLOBAL SETUP`);
-  await runFinishedPromise2;
-});
-
-test('should debug global setup when toggle is enabled', async ({ activate }, testInfo) => {
-  const { vscode, testController } = await activate({
-    'playwright.config.js': `module.exports = {
-      testDir: 'tests',
-      globalSetup: 'globalSetup.ts',
-    }`,
-    'globalSetup.ts': `
-      async function globalSetup(config) {
-        console.log('RUN GLOBAL SETUP UNDER DEBUG: ' + !!process.env.VSCODE_MOCK_DEBUGGING);
-        process.env.MAGIC_NUMBER = '42';
-      }
-      export default globalSetup;
-    `,
-    'tests/test.spec.ts': `
-      import { test, expect } from '@playwright/test';
-      test('should pass', async () => {
-        console.log('TEST UNDER DEBUG: ' + !!process.env.VSCODE_MOCK_DEBUGGING);
-        console.log('MAGIC NUMBER: ' + process.env.MAGIC_NUMBER);
-        expect(process.env.MAGIC_NUMBER).toBe('42');
-      });
-    `
-  }, { runGlobalSetupOnEachRun: true });
-
-  await testController.expandTestItems(/test.spec/);
-  const runFinishedPromise = testController.debugProfile().run(testController.findTestItems(/pass/));
-  await expect.poll(() => stripAnsi(vscode.debug.output)).toContain(`RUN GLOBAL SETUP UNDER DEBUG: true`);
-  await expect.poll(() => stripAnsi(vscode.debug.output)).toContain(`TEST UNDER DEBUG: true`);
-  await expect.poll(() => stripAnsi(vscode.debug.output)).toContain(`MAGIC NUMBER: 42`);
-  await runFinishedPromise;
 });

tests/list-files.spec.ts

@@ -252,6 +252,7 @@ test('should support multiple projects', async ({ activate }) => {
     -   tests
       -   test1.spec.ts
       -   test2.spec.ts
+    -    [playwright.config.js [project 2] — disabled]
   `);
 
   await expect(vscode).toHaveConnectionLog([
@@ -281,7 +282,6 @@ test('should switch between multiple projects with filter', async ({ activate })
       test('three', async () => {});
     `,
   });
-  await enableProjects(vscode, ['project 1']);
   await expect(testController).toHaveTestTree(`
     -   tests
       -   test1.spec.ts

tests/mock/vscode.ts

@@ -248,9 +248,7 @@ export class TestItem {
   }
 
   private _innerDelete(id: string) {
-    const item = this.map.get(id);
     this.map.delete(id);
-    item?.forEach(child => this.testController.allTestItems.delete(child.id));
     this.testController.allTestItems.delete(id);
   }
 
@@ -782,7 +780,7 @@ class Debug {
     this._debuggerProcess = spawn(node, [configuration.program, ...configuration.args], {
       cwd: configuration.cwd,
       stdio: 'pipe',
-      env: { ...configuration.env, VSCODE_MOCK_DEBUGGING: '1' },
+      env: configuration.env,
     });
 
     this._debuggerProcess.stdout.on('data', data => {