Skip to content

Commit fed5f5c

Browse files
ondrejmirtesondrejmirtes
committed
Unit-testable logic
1 parent bd90e10 commit fed5f5c

File tree

5 files changed

+292
-161
lines changed

5 files changed

+292
-161
lines changed

‎src/Command/Bisect/BinarySearch.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Command\Bisect;
4+
5+
use InvalidArgumentException;
6+
use function array_slice;
7+
use function ceil;
8+
use function count;
9+
use function log;
10+
11+
final class BinarySearch
12+
{
13+
14+
/**
15+
* @template T
16+
* @param list<T> $items Items ordered from oldest to newest (at least 2)
17+
* @return BinarySearchStep<T>
18+
*/
19+
public static function getStep(array $items): BinarySearchStep
20+
{
21+
$count = count($items);
22+
if ($count < 2) {
23+
throw new InvalidArgumentException('Binary search requires at least 2 items.');
24+
}
25+
26+
$mid = (int) (($count - 1) / 2);
27+
28+
return new BinarySearchStep(
29+
$items[$mid],
30+
array_slice($items, $mid + 1),
31+
array_slice($items, 0, $mid + 1),
32+
(int) ceil(log($count, 2)),
33+
);
34+
}
35+
36+
}

‎src/Command/Bisect/BinarySearchStep.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Command\Bisect;
4+
5+
/**
6+
* @template T
7+
*/
8+
final class BinarySearchStep
9+
{
10+
11+
/**
12+
* @param T $item Item to test
13+
* @param list<T> $ifGood Remaining items to search if this item is good
14+
* @param list<T> $ifBad Remaining items to search if this item is bad
15+
*/
16+
public function __construct(
17+
public readonly mixed $item,
18+
public readonly array $ifGood,
19+
public readonly array $ifBad,
20+
public readonly int $stepsRemaining,
21+
)
22+
{
23+
}
24+
25+
}

‎src/Command/BisectCommand.php‎

Lines changed: 82 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use GuzzleHttp\RequestOptions;
88
use Nette\Utils\Json;
99
use Override;
10+
use PHPStan\Command\Bisect\BinarySearch;
1011
use PHPStan\File\FileReader;
1112
use Symfony\Component\Console\Command\Command;
1213
use Symfony\Component\Console\Helper\ProgressBar;
@@ -16,8 +17,9 @@
1617
use Symfony\Component\Console\Output\OutputInterface;
1718
use Symfony\Component\Console\Style\SymfonyStyle;
1819
use Throwable;
20+
use function array_filter;
1921
use function array_merge;
20-
use function ceil;
22+
use function array_values;
2123
use function chmod;
2224
use function count;
2325
use function escapeshellarg;
@@ -26,7 +28,6 @@
2628
use function is_array;
2729
use function is_file;
2830
use function is_string;
29-
use function log;
3031
use function mkdir;
3132
use function passthru;
3233
use function preg_match_all;
@@ -130,28 +131,48 @@ protected function execute(InputInterface $input, OutputInterface $output): int
130131

131132
$io->writeln(sprintf('Found <info>%d</info> commits between %s and %s.', count($commits), $good, $bad));
132133

133-
$lo = 0;
134-
$hi = count($commits) - 1;
134+
$rangeShas = [];
135+
foreach ($commits as $commit) {
136+
$rangeShas[$commit['sha']] = true;
137+
}
138+
139+
try {
140+
$checksumShas = $this->getPharChecksumCommitShas($client, $bad, $rangeShas);
141+
} catch (GuzzleException $e) {
142+
$io->error(sprintf('Failed to fetch .phar-checksum commits from GitHub: %s', $e->getMessage()));
143+
return 1;
144+
}
145+
146+
$commits = array_values(array_filter($commits, static function (array $commit) use ($checksumShas): bool {
147+
return isset($checksumShas[$commit['sha']]);
148+
}));
149+
150+
if (count($commits) === 0) {
151+
$io->error('No commits found that change phpstan.phar between the specified releases.');
152+
return 1;
153+
}
154+
155+
$io->writeln(sprintf('<info>%d</info> of them change phpstan.phar.', count($commits)));
156+
135157
$tmpDir = sys_get_temp_dir() . '/phpstan-bisect';
136158
@mkdir($tmpDir, 0777, true);
137159

138160
$analyseArgs = $this->buildAnalyseArgs($input);
139161

140-
while ($lo < $hi) {
141-
$mid = (int) (($lo + $hi) / 2);
142-
$commit = $commits[$mid];
162+
while (count($commits) > 1) {
163+
$step = BinarySearch::getStep($commits);
164+
$commit = $step->item;
143165
$sha = $commit['sha'];
144166
$shortSha = substr($sha, 0, 7);
145167
$message = $commit['commit']['message'];
146168
$firstLine = strtok($message, "\n") ?: $shortSha;
147169

148-
$stepsLeft = (int) ceil(log($hi - $lo + 1, 2));
149170
$io->section(sprintf(
150171
'Testing commit %s (%s) [~%d step%s left]',
151172
$shortSha,
152173
$firstLine,
153-
$stepsLeft,
154-
$stepsLeft === 1 ? '' : 's',
174+
$step->stepsRemaining,
175+
$step->stepsRemaining === 1 ? '' : 's',
155176
));
156177

157178
$pharPath = $tmpDir . '/phpstan-' . $shortSha . '.phar';
@@ -181,14 +202,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int
181202
['good', 'bad'],
182203
);
183204

184-
if ($answer === 'good') {
185-
$lo = $mid + 1;
186-
} else {
187-
$hi = $mid;
188-
}
205+
$commits = $answer === 'good' ? $step->ifGood : $step->ifBad;
189206
}
190207

191-
$badCommit = $commits[$lo];
208+
$badCommit = $commits[0];
192209
$this->printResult($badCommit, $io);
193210

194211
return 0;
@@ -270,6 +287,55 @@ private function getCommitsBetween(Client $client, string $good, string $bad): a
270287
return $allCommits;
271288
}
272289

290+
/**
291+
* @param array<string, true> $rangeShas
292+
* @return array<string, true>
293+
* @throws GuzzleException
294+
*/
295+
private function getPharChecksumCommitShas(Client $client, string $bad, array $rangeShas): array
296+
{
297+
$checksumShas = [];
298+
$page = 1;
299+
$perPage = 100;
300+
301+
while (true) {
302+
$response = $client->get(sprintf(
303+
'https://api.github.com/repos/%s/%s/commits?sha=%s&path=%s&per_page=%d&page=%d',
304+
self::REPO_OWNER,
305+
self::REPO_NAME,
306+
urlencode($bad),
307+
urlencode('.phar-checksum'),
308+
$perPage,
309+
$page,
310+
));
311+
312+
/** @var list<array{sha: string}> $commits */
313+
$commits = Json::decode($response->getBody()->getContents(), Json::FORCE_ARRAY);
314+
315+
if (count($commits) === 0) {
316+
break;
317+
}
318+
319+
$foundOutOfRange = false;
320+
foreach ($commits as $commit) {
321+
if (isset($rangeShas[$commit['sha']])) {
322+
$checksumShas[$commit['sha']] = true;
323+
} else {
324+
$foundOutOfRange = true;
325+
break;
326+
}
327+
}
328+
329+
if ($foundOutOfRange || count($commits) < $perPage) {
330+
break;
331+
}
332+
333+
$page++;
334+
}
335+
336+
return $checksumShas;
337+
}
338+
273339
/**
274340
* @throws GuzzleException
275341
*/

‎tests/PHPStan/Command/Bisect/BinarySearchTest.php‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Command\Bisect;
4+
5+
use InvalidArgumentException;
6+
use PHPUnit\Framework\Attributes\DataProvider;
7+
use PHPUnit\Framework\TestCase;
8+
use function array_map;
9+
use function array_search;
10+
use function count;
11+
use function range;
12+
13+
class BinarySearchTest extends TestCase
14+
{
15+
16+
/**
17+
* @param list<string> $items
18+
* @param list<string> $expectedIfGood
19+
* @param list<string> $expectedIfBad
20+
*/
21+
#[DataProvider('dataGetStep')]
22+
public function testGetStep(
23+
array $items,
24+
string $expectedItem,
25+
array $expectedIfGood,
26+
array $expectedIfBad,
27+
int $expectedStepsRemaining,
28+
): void
29+
{
30+
$step = BinarySearch::getStep($items);
31+
$this->assertSame($expectedItem, $step->item);
32+
$this->assertSame($expectedIfGood, $step->ifGood);
33+
$this->assertSame($expectedIfBad, $step->ifBad);
34+
$this->assertSame($expectedStepsRemaining, $step->stepsRemaining);
35+
}
36+
37+
public static function dataGetStep(): iterable
38+
{
39+
yield 'two items' => [
40+
['a', 'b'],
41+
'a',
42+
['b'],
43+
['a'],
44+
1,
45+
];
46+
47+
yield 'three items' => [
48+
['a', 'b', 'c'],
49+
'b',
50+
['c'],
51+
['a', 'b'],
52+
2,
53+
];
54+
55+
yield 'four items' => [
56+
['a', 'b', 'c', 'd'],
57+
'b',
58+
['c', 'd'],
59+
['a', 'b'],
60+
2,
61+
];
62+
63+
yield 'five items' => [
64+
['a', 'b', 'c', 'd', 'e'],
65+
'c',
66+
['d', 'e'],
67+
['a', 'b', 'c'],
68+
3,
69+
];
70+
71+
yield 'eight items' => [
72+
['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'],
73+
'd',
74+
['e', 'f', 'g', 'h'],
75+
['a', 'b', 'c', 'd'],
76+
3,
77+
];
78+
79+
yield 'sixteen items' => [
80+
array_map(static fn (int $i): string => (string) $i, range(1, 16)),
81+
'8',
82+
['9', '10', '11', '12', '13', '14', '15', '16'],
83+
['1', '2', '3', '4', '5', '6', '7', '8'],
84+
4,
85+
];
86+
}
87+
88+
/**
89+
* @param list<string> $items
90+
*/
91+
#[DataProvider('dataTooFewItems')]
92+
public function testGetStepWithTooFewItems(array $items): void
93+
{
94+
$this->expectException(InvalidArgumentException::class);
95+
BinarySearch::getStep($items);
96+
}
97+
98+
public static function dataTooFewItems(): iterable
99+
{
100+
yield 'empty' => [[]];
101+
yield 'single item' => [['a']];
102+
}
103+
104+
/**
105+
* @param list<string> $items
106+
*/
107+
#[DataProvider('dataFullBisect')]
108+
public function testFullBisect(array $items, string $firstBadItem): void
109+
{
110+
$badIndex = array_search($firstBadItem, $items, true);
111+
$this->assertNotFalse($badIndex);
112+
113+
$current = $items;
114+
$steps = 0;
115+
$initialStep = BinarySearch::getStep($current);
116+
117+
while (count($current) > 1) {
118+
$step = BinarySearch::getStep($current);
119+
$testIndex = array_search($step->item, $items, true);
120+
$this->assertNotFalse($testIndex);
121+
122+
$isBad = $testIndex >= $badIndex;
123+
$current = $isBad ? $step->ifBad : $step->ifGood;
124+
$steps++;
125+
}
126+
127+
$this->assertCount(1, $current);
128+
$this->assertSame($firstBadItem, $current[0]);
129+
$this->assertLessThanOrEqual($initialStep->stepsRemaining, $steps);
130+
}
131+
132+
public static function dataFullBisect(): iterable
133+
{
134+
$lists = [
135+
'2 items' => ['a', 'b'],
136+
'3 items' => ['a', 'b', 'c'],
137+
'5 items' => ['a', 'b', 'c', 'd', 'e'],
138+
'8 items' => ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'],
139+
'16 items' => array_map(static fn (int $i): string => 'commit-' . $i, range(1, 16)),
140+
];
141+
142+
foreach ($lists as $name => $items) {
143+
foreach ($items as $badItem) {
144+
yield "$name, first bad is $badItem" => [$items, $badItem];
145+
}
146+
}
147+
}
148+
149+
}

0 commit comments

Comments
 (0)