Skip to content

Commit

Permalink
fix(Loop,Worker) work around Windows command limit of 8k
Browse files Browse the repository at this point in the history
  • Loading branch information
lucatume committed Oct 4, 2023
1 parent 49dec3c commit 681ea19
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 5 deletions.
11 changes: 9 additions & 2 deletions src/Process/Loop.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class Loop
private array $workers = [];

private bool $fastFailureFlagRaised = false;
private bool $useFilePayloads = false;

/**
* @param array<int|string,Worker|callable> $workers
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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
*/
Expand Down
28 changes: 27 additions & 1 deletion src/Process/Protocol/Request.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
class Request
{
private Control $control;
private bool $useFilePayloads = false;

/**
* @param array{
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -65,4 +84,11 @@ public function getControl(): Control
{
return clone $this->control;
}

public function setUseFilePayloads(bool $useFilePayloads): Request
{
$this->useFilePayloads = $useFilePayloads;

return $this;
}
}
3 changes: 2 additions & 1 deletion src/Process/Worker/Running.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?
Expand All @@ -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()]);
Expand Down
16 changes: 16 additions & 0 deletions src/Process/Worker/Worker.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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'],
Expand Down
12 changes: 11 additions & 1 deletion src/Process/Worker/worker-script.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
48 changes: 48 additions & 0 deletions tests/unit/lucatume/WPBrowser/Process/LoopTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php


namespace Unit\lucatume\WPBrowser\Process;

use Codeception\Test\Unit;
use lucatume\WPBrowser\Process\Loop;

class LoopTest extends Unit
{
/**
* It should work using normal payloads
*
* @test
*/
public function should_work_using_normal_payloads(): void
{
$job = static function () {
return 'Hello from the loop';
};
$loop = new Loop([$job]);
$loop->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());
}
}
22 changes: 22 additions & 0 deletions tests/unit/lucatume/WPBrowser/Process/Protocol/RequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

0 comments on commit 681ea19

Please sign in to comment.