Skip to content

Commit 19f91dc

Browse files
alfredbezampagent
andauthored
fix(parallel): report error limit only when reached (#8844)
Co-authored-by: Amp <amp@ampcode.com>
1 parent 6aff6ac commit 19f91dc

5 files changed

Lines changed: 111 additions & 1 deletion

File tree

‎src/Parallel/Application/ParallelFileProcessor.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public function process(
132132
));
133133

134134
++$systemErrorsCount;
135-
$reachedSystemErrorsCountLimit = true;
135+
$reachedSystemErrorsCountLimit = $systemErrorsCount >= self::SYSTEM_ERROR_LIMIT;
136136
$this->processPool->quitAll();
137137

138138
// give the child processes time to actually die, they can still write to cache meanwhile
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<?php
2+
3+
pause();
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\Bin\Fixture;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr\FuncCall;
9+
use Rector\Rector\AbstractRector;
10+
use Rector\RuleDoc\CodeSample\CodeSample;
11+
use Rector\RuleDoc\RuleDefinition;
12+
13+
final class TimeoutRector extends AbstractRector
14+
{
15+
public function getRuleDefinition(): RuleDefinition
16+
{
17+
return new RuleDefinition('Exceed the worker timeout on pause()', [new CodeSample('', '')]);
18+
}
19+
20+
/**
21+
* @return array<class-string<Node>>
22+
*/
23+
public function getNodeTypes(): array
24+
{
25+
return [FuncCall::class];
26+
}
27+
28+
/**
29+
* @param FuncCall $node
30+
*/
31+
public function refactor(Node $node): ?Node
32+
{
33+
if ($this->isName($node, 'pause')) {
34+
sleep(10);
35+
}
36+
37+
return null;
38+
}
39+
}

‎tests/Bin/RectorTest.php‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,58 @@ public function testConsoleOutput(string $command, string $expectedOutput): void
4141
$process->getErrorOutput()
4242
));
4343
}
44+
45+
/**
46+
* @return Iterator<string, array{int, string[]}>
47+
*/
48+
public static function provideParallelTimeouts(): Iterator
49+
{
50+
yield 'one timeout below the limit' => [0, [
51+
'Child process timed out after 1 seconds',
52+
'Some parallel jobs have not finished, results for 1 of 1 files are missing',
53+
]];
54+
55+
yield '49 errors followed by a timeout reach the limit' => [49, [
56+
...array_fill(0, 49, "Syntax error, unexpected ';'"),
57+
'Child process timed out after 1 seconds',
58+
'Reached system errors count limit of 50, exiting...',
59+
'Some parallel jobs have not finished, results for 1 of 50 files are missing',
60+
]];
61+
}
62+
63+
/**
64+
* @param string[] $expectedMessages
65+
*/
66+
#[DataProvider('provideParallelTimeouts')]
67+
public function testParallelTimeoutReportsErrorLimitOnlyWhenReached(
68+
int $previousErrorCount,
69+
array $expectedMessages
70+
): void {
71+
$filePaths = [];
72+
for ($i = 5; $i < 5 + $previousErrorCount; ++$i) {
73+
$filePaths[] = __DIR__ . sprintf('/../../e2e/parallel-system-errors-limit/src/File%02d.php', $i);
74+
}
75+
76+
// The syntax-error files are larger, so the single worker processes them before the timeout file.
77+
$filePaths[] = __DIR__ . '/Fixture/ParallelTimeout/Timeout.php';
78+
79+
$process = new Process([
80+
PHP_BINARY,
81+
'bin/rector',
82+
'--config',
83+
__DIR__ . '/config/parallel-worker-timeout.php',
84+
'--dry-run',
85+
'--clear-cache',
86+
'--no-progress-bar',
87+
'--output-format=json',
88+
...$filePaths,
89+
]);
90+
$process->setTimeout(20);
91+
$process->run();
92+
93+
$this->assertSame(1, $process->getExitCode(), $process->getOutput() . $process->getErrorOutput());
94+
$result = json_decode($process->getOutput(), true, flags: JSON_THROW_ON_ERROR);
95+
$this->assertSame(count($expectedMessages), $result['totals']['errors']);
96+
$this->assertSame($expectedMessages, array_column($result['errors'], 'message'));
97+
}
4498
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Rector\Config\RectorConfig;
6+
use Rector\Tests\Bin\Fixture\TimeoutRector;
7+
8+
require_once __DIR__ . '/../Fixture/TimeoutRector.php';
9+
10+
return static function (RectorConfig $rectorConfig): void {
11+
$rectorConfig->paths([__DIR__ . '/../Fixture/ParallelTimeout']);
12+
$rectorConfig->parallel(processTimeout: 1, maxNumberOfProcess: 1, jobSize: 1);
13+
$rectorConfig->rules([TimeoutRector::class]);
14+
};

0 commit comments

Comments
 (0)