From 17f88a7d9aae6e6b44626afcadb2422922f010c2 Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Fri, 25 Sep 2026 09:48:07 +0200 Subject: [PATCH] fix(runner): give each run its own browser instead of a machine-wide one The keepAlive browser was found again through a socket file at one fixed path per machine ($TMPDIR/prestaflow-.browser), shared by every PrestaFlow process. A run reconnected to whatever Chrome that file named, and ExecuteSuite ended every run by closing that browser and deleting the file. So any run that finished while another was in flight, even a browser-free one such as a smoke suite, closed the other run's Chrome under it. The victim failed with "The page was closed and is not available anymore", typically on its first navigation, or silently lost its session when it relaunched a browser between two steps (an emptied cart, a checkout that no longer matches). Whether it happened depended on what else was running on the machine, not on the PrestaShop version under test. ExecuteSuite now scopes the socket and options files to the run (pid plus a random token) before it starts and releases only that browser at the end. Callers outside ExecuteSuite keep the shared path, and the reconnection they may rely on. Co-Authored-By: Claude Opus 5.5 --- src/Command/ExecuteSuite.php | 27 ++++++- src/Tests/TestsSuite.php | 27 +++++++ tests/Unit/Command/BrowserOwnershipTest.php | 90 +++++++++++++++++++++ 3 files changed, 142 insertions(+), 2 deletions(-) create mode 100644 tests/Unit/Command/BrowserOwnershipTest.php diff --git a/src/Command/ExecuteSuite.php b/src/Command/ExecuteSuite.php index b9afe06..72d9dcc 100644 --- a/src/Command/ExecuteSuite.php +++ b/src/Command/ExecuteSuite.php @@ -117,6 +117,7 @@ protected function outputTitle() public function execute(InputInterface $input, OutputInterface $output): int { + $this->beginRun(); $this->handleDir(dirname($this->file)); $this->cli = true; $this->output = $output; @@ -295,14 +296,36 @@ public function execute(InputInterface $input, OutputInterface $output): int $this->success('Rapport visuel écrit dans ' . $visualPath, newLine: true, force: true); } + $this->releaseBrowser(); + + return $summary->hasFailures() ? Command::FAILURE : Command::SUCCESS; + } + + /** + * Give this run a browser of its own. + * + * The run ends by closing its browser (releaseBrowser()), so it must never + * pick up one another PrestaFlow process is driving, nor let that process + * find and close its own. Scoping the socket file to this run does both. + */ + public function beginRun(): void + { + \PrestaFlow\Library\Tests\TestsSuite::scopeBrowserFilesTo( + 'run-' . getmypid() . '-' . bin2hex(random_bytes(4)) + ); + } + + /** + * Close the browser this run launched and forget its socket file. + */ + public function releaseBrowser(): void + { try { \PrestaFlow\Library\Tests\TestsSuite::getBrowser(force: false)?->close(); } catch (\Throwable $e) { } @unlink(\PrestaFlow\Library\Tests\TestsSuite::getFilePath('.browser')); @unlink(\PrestaFlow\Library\Tests\TestsSuite::getFilePath('.browser-options')); - - return $summary->hasFailures() ? Command::FAILURE : Command::SUCCESS; } protected function handleDir($path) diff --git a/src/Tests/TestsSuite.php b/src/Tests/TestsSuite.php index 4e5af19..805272d 100644 --- a/src/Tests/TestsSuite.php +++ b/src/Tests/TestsSuite.php @@ -154,6 +154,11 @@ private static function buildVisualBlock(int $startIndex): array protected static $browserInstance = null; protected static ?string $browserInstanceSocket = null; + /** + * Owner of the keepAlive browser files, see scopeBrowserFilesTo(). + */ + protected static ?string $browserFilesScope = null; + protected $draft = false; protected $groups = 'all'; @@ -316,8 +321,30 @@ public function getGroups() : string|array return $this->groups; } + /** + * Scope the keepAlive browser files (socket, options) to one run. + * + * Without a scope they sit at one path per machine, which every PrestaFlow + * process shares: a run would reconnect to the Chrome another run is + * driving, and ExecuteSuite, which closes that browser and deletes the file + * when it ends, would tear it down under the other run ("The page was + * closed and is not available anymore"). A run scopes the files to itself + * so it only ever finds, and releases, the browser it launched. + * + * Null restores the shared path, for callers outside ExecuteSuite that + * rely on reconnecting to an existing browser. + */ + public static function scopeBrowserFilesTo(?string $scope): void + { + self::$browserFilesScope = ($scope === null || $scope === '') ? null : $scope; + } + public static function getFilePath($filename = '.browser') { + if (self::$browserFilesScope !== null) { + $filename = self::$browserFilesScope.'-'.$filename; + } + if (function_exists('storage_path')) { $dir = storage_path().'/datas'; if (!is_dir($dir)) { diff --git a/tests/Unit/Command/BrowserOwnershipTest.php b/tests/Unit/Command/BrowserOwnershipTest.php new file mode 100644 index 0000000..eb7aa3f --- /dev/null +++ b/tests/Unit/Command/BrowserOwnershipTest.php @@ -0,0 +1,90 @@ +files as $file) { + @unlink($file); + } + TestsSuite::scopeBrowserFilesTo(null); + } + + public function testARunThatEndsLeavesTheBrowserOfAnotherRunAlone(): void + { + $first = new ExecuteSuite(); + $first->beginRun(); + $firstSocketFile = TestsSuite::getFilePath('.browser'); + $this->files[] = $firstSocketFile; + file_put_contents($firstSocketFile, 'ws://127.0.0.1:1/devtools/browser/first-run'); + + // A second run starts and finishes while the first is still driving + // its browser (in real life: another process, same temp dir). + $second = new ExecuteSuite(); + $second->beginRun(); + $this->files[] = TestsSuite::getFilePath('.browser'); + $second->releaseBrowser(); + + $this->assertFileExists( + $firstSocketFile, + 'the second run deleted the socket file of the first one' + ); + } + + public function testEachRunGetsItsOwnSocketFile(): void + { + $first = new ExecuteSuite(); + $first->beginRun(); + $firstSocketFile = TestsSuite::getFilePath('.browser'); + + $second = new ExecuteSuite(); + $second->beginRun(); + + $this->assertNotSame($firstSocketFile, TestsSuite::getFilePath('.browser')); + } + + public function testARunReleasesItsOwnSocketFile(): void + { + $run = new ExecuteSuite(); + $run->beginRun(); + $socketFile = TestsSuite::getFilePath('.browser'); + $this->files[] = $socketFile; + file_put_contents($socketFile, 'ws://127.0.0.1:1/devtools/browser/own-run'); + + $run->releaseBrowser(); + + $this->assertFileDoesNotExist($socketFile); + } + + public function testWithoutARunTheSharedPathIsUnchanged(): void + { + TestsSuite::scopeBrowserFilesTo(null); + + $this->assertSame( + sys_get_temp_dir() . '/prestaflow-.browser', + TestsSuite::getFilePath('.browser') + ); + } +}