fix(playwright): start no browser in _beforeSuite with restart: browser (#5784) (#5785)

With the browser restart strategy, _before() starts a browser for each
test. The browser that _beforeSuite() started was replaced by the one of
the first test and never closed, so each test file left one browser
running.

Author: danielrentz

lib/helper/Playwright.js

@@ -554,9 +554,10 @@ class Playwright extends Helper {
       return
     }
 
-    // Start browser if not manually started and not already running
-    // Browser should start in singleton mode (restart: false) or when restart strategy is enabled
-    if (!this.options.manualStart && !this.isRunning) {
+    // Start browser if not manually started and not already running.
+    // With the browser restart strategy, _before() starts a browser for each test,
+    // and a browser started here would never be closed (#5784)
+    if (!this.options.manualStart && !this.isRunning && !restartsBrowser()) {
       this.debugSection('Session', 'Starting singleton browser session')
       return this._startBrowser()
     }

test/unit/helper/Playwright_restart_test.js

@@ -0,0 +1,29 @@
+import { expect } from 'chai'
+import Playwright from '../../../lib/helper/Playwright.js'
+
+describe('Playwright restart strategy', () => {
+  function countBrowserStarts(restart) {
+    const helper = new Playwright({ browser: 'chromium', url: 'http://localhost', restart })
+    let starts = 0
+    helper._startBrowser = async () => {
+      starts++
+    }
+    return { helper, starts: () => starts }
+  }
+
+  it('should not start a browser before a suite when each test starts its own browser (#5784)', async () => {
+    for (const restart of [true, 'browser']) {
+      const { helper, starts } = countBrowserStarts(restart)
+      await helper._beforeSuite()
+      expect(starts(), `restart: ${restart}`).to.equal(0)
+    }
+  })
+
+  it('should start the browser before a suite when tests share it', async () => {
+    for (const restart of [false, 'context', 'session']) {
+      const { helper, starts } = countBrowserStarts(restart)
+      await helper._beforeSuite()
+      expect(starts(), `restart: ${restart}`).to.equal(1)
+    }
+  })
+})