From 681ea1928da802b4437dd15b1f5cbcbab9a96dcc Mon Sep 17 00:00:00 2001 From: Luca Tumedei Date: Wed, 4 Oct 2023 09:37:21 +0200 Subject: [PATCH] fix(Loop,Worker) work around Windows command limit of 8k --- src/Process/Loop.php | 11 ++++- src/Process/Protocol/Request.php | 28 ++++++++++- src/Process/Worker/Running.php | 3 +- src/Process/Worker/Worker.php | 16 +++++++ src/Process/Worker/worker-script.php | 12 ++++- .../lucatume/WPBrowser/Process/LoopTest.php | 48 +++++++++++++++++++ .../Process/Protocol/RequestTest.php | 22 +++++++++ 7 files changed, 135 insertions(+), 5 deletions(-) create mode 100644 tests/unit/lucatume/WPBrowser/Process/LoopTest.php diff --git a/src/Process/Loop.php b/src/Process/Loop.php index 75b9f16b9..a085160d1 100644 --- a/src/Process/Loop.php +++ b/src/Process/Loop.php @@ -40,6 +40,7 @@ class Loop private array $workers = []; private bool $fastFailureFlagRaised = false; + private bool $useFilePayloads = false; /** * @param array $workers @@ -113,7 +114,7 @@ public static function executeClosureOrFail( * cwd?: string * } $options */ - public function addWorkers(array $workers, array $options): Loop + public function addWorkers(array $workers, array $options = []): Loop { $builtWorkers = array_map([$this, 'ensureWorker'], array_keys($workers), $workers); @@ -181,7 +182,7 @@ private function startWorker(): void } try { - $w = Running::fromWorker($runnableWorker); + $w = Running::fromWorker($runnableWorker, $this->useFilePayloads); $this->started[$w->getId()] = $w; $this->running[$w->getId()] = $w; $this->peakParallelism = max((int)$this->peakParallelism, count($this->running)); @@ -347,6 +348,12 @@ public function failed(): bool return $this->fastFailure && $this->fastFailureFlagRaised; } + public function setUseFilePayloads(bool $useFilePayloads): Loop + { + $this->useFilePayloads = $useFilePayloads; + return $this; + } + /** * @throws ConfigurationException */ diff --git a/src/Process/Protocol/Request.php b/src/Process/Protocol/Request.php index 2be5e5638..e029cde31 100644 --- a/src/Process/Protocol/Request.php +++ b/src/Process/Protocol/Request.php @@ -8,6 +8,7 @@ class Request { private Control $control; + private bool $useFilePayloads = false; /** * @param array{ @@ -27,9 +28,27 @@ public function __construct(array $controlArray, private SerializableClosure $se $this->control = new Control($controlArray); } + /** + * @throws ProtocolException + */ public function getPayload(): string { - return Parser::encode([$this->control->toArray(), $this->serializableClosure]); + $payload = Parser::encode([$this->control->toArray(), $this->serializableClosure]); + + if (DIRECTORY_SEPARATOR === '\\' || $this->useFilePayloads) { + // On Windows the maximum length of the command line is 8191 characters. + // Any expanded env var, any path (e.g. to the PHP binary or the worker script) counts towards that limit. + // To avoid running into that limit we pass the payload through a temp file. + $payloadFile = sys_get_temp_dir() . DIRECTORY_SEPARATOR . uniqid('wpb_worker_payload_', true); + + if (file_put_contents($payloadFile, $payload, LOCK_EX) === false) { + throw new ProtocolException("Could not write payload to file $payloadFile"); + } + + return $payloadFile; + } + + return $payload; } /** @@ -65,4 +84,11 @@ public function getControl(): Control { return clone $this->control; } + + public function setUseFilePayloads(bool $useFilePayloads): Request + { + $this->useFilePayloads = $useFilePayloads; + + return $this; + } } diff --git a/src/Process/Worker/Running.php b/src/Process/Worker/Running.php index 3f892c21b..3c8baca34 100644 --- a/src/Process/Worker/Running.php +++ b/src/Process/Worker/Running.php @@ -36,7 +36,7 @@ public function __construct( /** * @throws ConfigurationException|ProcessException */ - public static function fromWorker(Worker $worker): Running + public static function fromWorker(Worker $worker, bool $useFilePayloads = false): Running { $workerCallable = $worker->getCallable(); $workerClosure = $workerCallable instanceof Closure ? @@ -51,6 +51,7 @@ public static function fromWorker(Worker $worker): Running $workerSerializableClosure = new SerializableClosure($workerClosure); $request = new Request($control, $workerSerializableClosure); + $request->setUseFilePayloads($useFilePayloads); try { $workerProcess = new WorkerProcess([PHP_BINARY, $workerScriptPathname, $request->getPayload()]); diff --git a/src/Process/Worker/Worker.php b/src/Process/Worker/Worker.php index 442c5d65f..0cd38f44b 100644 --- a/src/Process/Worker/Worker.php +++ b/src/Process/Worker/Worker.php @@ -2,8 +2,10 @@ namespace lucatume\WPBrowser\Process\Worker; +use Closure; use Codeception\Exception\ConfigurationException; use lucatume\WPBrowser\Process\Protocol\Control; +use ReflectionFunction; class Worker implements WorkerInterface { @@ -52,6 +54,20 @@ public function __construct( } else { $cwd = getcwd() ?: codecept_root_dir(); } + + if ($callable instanceof Closure) { + // Closures might come from files that are not autoloaded (e.g. test cases); include them in the required + // files to make sure the Closure will be bound to a valid scope. + $closureFile = (new ReflectionFunction($callable))->getFileName(); + if ($closureFile !== false) { + if (!isset($control['requireFiles'])) { + $control['requireFiles'] = []; + } + $control['requireFiles'][] = $closureFile; + $control['requireFiles'] = array_values(array_unique($control['requireFiles'])); + } + } + $this->control = [ 'autoloadFile' => $control['autoloadFile'] ?? $defaultControl['autoloadFile'], 'requireFiles' => $control['requireFiles'] ?? $defaultControl['requireFiles'], diff --git a/src/Process/Worker/worker-script.php b/src/Process/Worker/worker-script.php index 6cc03011e..0987b4b80 100644 --- a/src/Process/Worker/worker-script.php +++ b/src/Process/Worker/worker-script.php @@ -11,7 +11,17 @@ require_once $processSrcRoot . '/Protocol/ProtocolException.php'; try { - $request = Request::fromPayload($argv[1]); + if (!isset($argv[1])) { + throw new RuntimeException('Payload empty.'); + } + + if (str_starts_with($argv[1], '$')) { + $payload = $argv[1]; + } elseif (($payload = @file_get_contents($argv[1])) === false) { + throw new RuntimeException("Could not read payload from file $argv[1]"); + } + + $request = Request::fromPayload($payload); $serializableClosure = $request->getSerializableClosure(); $returnValue = $serializableClosure(); } catch (Throwable $throwable) { diff --git a/tests/unit/lucatume/WPBrowser/Process/LoopTest.php b/tests/unit/lucatume/WPBrowser/Process/LoopTest.php new file mode 100644 index 000000000..d6b007ed2 --- /dev/null +++ b/tests/unit/lucatume/WPBrowser/Process/LoopTest.php @@ -0,0 +1,48 @@ +setUseFilePayloads(false); + + $loop->run(); + $results = $loop->getResults(); + + $this->assertEquals('Hello from the loop', $results[0]->getReturnValue()); + } + + /** + * It should work using file payloads + * + * @test + */ + public function should_work_using_file_payloads(): void + { + $job = static function () { + return 'Hello from the loop with file payloads'; + }; + $loop = new Loop([$job]); + $loop->setUseFilePayloads(true); + + $loop->run(); + $results = $loop->getResults(); + + $this->assertEquals('Hello from the loop with file payloads', $results[0]->getReturnValue()); + } +} diff --git a/tests/unit/lucatume/WPBrowser/Process/Protocol/RequestTest.php b/tests/unit/lucatume/WPBrowser/Process/Protocol/RequestTest.php index 3b0ca26eb..f5b6d68f2 100644 --- a/tests/unit/lucatume/WPBrowser/Process/Protocol/RequestTest.php +++ b/tests/unit/lucatume/WPBrowser/Process/Protocol/RequestTest.php @@ -76,4 +76,26 @@ public function test_getPayload_fromPayload(array $control, SerializableClosure $this->assertEquals($request->getSerializableClosure(), $serializableClosure); $this->assertEquals($request->getControl(), $fromPayload->getControl()); } + + /** + * It should return a payload file path when getting payload on Windows + * + * @test + */ + public function should_return_a_payload_file_path_when_getting_payload_on_windows(): void + { + $serializableClosure = new SerializableClosure(static function () { + return 'foo'; + }); + $control = ['foo' => 'bar']; + $encoded = Parser::encode([(new Control($control))->toArray(), $serializableClosure]); + + $request = new Request(['foo' => 'bar'], $serializableClosure); + $request->setUseFilePayloads(true); + $payload = $request->getPayload(); + + $this->assertIsString($payload); + $this->assertFileExists($payload); + $this->assertStringEqualsFile($payload, $encoded); + } }