From 2cbc7a7df6e04faf18bb0da9fbe1db691d711f42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Sat, 10 Dec 2022 11:33:45 +0100 Subject: [PATCH] Change PopenBased finders to ProcOpenBased finders --- infection.json5 | 8 +++ src/Exec/ProcOpen.php | 58 +++++++++++++++++++ src/Finder/HwLogicalFinder.php | 2 +- src/Finder/HwPhysicalFinder.php | 2 +- src/Finder/LinuxyNProcessorFinder.php | 2 +- src/Finder/NProcessorFinder.php | 2 +- ...asedFinder.php => ProcOpenBasedFinder.php} | 55 ++++++++---------- src/Finder/WindowsWmicLogicalFinder.php | 2 +- src/Finder/WindowsWmicPhysicalFinder.php | 2 +- tests/Exec/ProcOpenTest.php | 56 ++++++++++++++++++ ...inder.php => DummyProcOpenBasedFinder.php} | 4 +- tests/Finder/HwLogicalFinderTest.php | 6 +- tests/Finder/HwPhysicalFinderTest.php | 6 +- tests/Finder/LinuxyNProcessorFinderTest.php | 6 +- tests/Finder/NProcessorFinderTest.php | 6 +- ...erTest.php => ProcOpenBasedFinderTest.php} | 10 ++-- ...se.php => ProcOpenBasedFinderTestCase.php} | 6 +- tests/Finder/WindowsWmicLogicalFinderTest.php | 6 +- .../Finder/WindowsWmicPhysicalFinderTest.php | 6 +- 19 files changed, 179 insertions(+), 66 deletions(-) create mode 100644 src/Exec/ProcOpen.php rename src/Finder/{PopenBasedFinder.php => ProcOpenBasedFinder.php} (54%) create mode 100644 tests/Exec/ProcOpenTest.php rename tests/Finder/{DummyPopenBasedFinder.php => DummyProcOpenBasedFinder.php} (77%) rename tests/Finder/{PopenBasedFinderTest.php => ProcOpenBasedFinderTest.php} (81%) rename tests/Finder/{PopenBasedFinderTestCase.php => ProcOpenBasedFinderTestCase.php} (88%) diff --git a/infection.json5 b/infection.json5 index c972f89..6c3636f 100644 --- a/infection.json5 +++ b/infection.json5 @@ -20,6 +20,12 @@ "Fidry\\CpuCoreCounter\\CpuCoreCounter::getDefaultFinders" ] }, + "CastString": { + "ignore": [ + // I can't find a case in practice where this would happen + "Fidry\\CpuCoreCounter\\Exec\\ProcOpen::execute" + ] + }, "FalseValue": { "ignore": [ // This is from thecodingmachine/safe – leave it alone @@ -28,6 +34,8 @@ }, "FunctionCallRemoval": { "ignore": [ + // I can't find a case in practice where this would happen + "Fidry\\CpuCoreCounter\\Exec\\ProcOpen::execute", // This is from thecodingmachine/safe – leave it alone "Fidry\\CpuCoreCounter\\Exec\\ShellExec::execute" ] diff --git a/src/Exec/ProcOpen.php b/src/Exec/ProcOpen.php new file mode 100644 index 0000000..78ed77b --- /dev/null +++ b/src/Exec/ProcOpen.php @@ -0,0 +1,58 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Fidry\CpuCoreCounter\Exec; + +use function fclose; +use function is_resource; +use function proc_close; +use function proc_open; +use function stream_get_contents; + +final class ProcOpen +{ + /** + * @return array{string, string} STDOUT & STDERR tuple + */ + public static function execute(string $command): ?array + { + $pipes = []; + + $process = @proc_open( + $command, + [ + ['pipe', 'rb'], + ['pipe', 'wb'], // stdout + ['pipe', 'wb'], // stderr + ], + $pipes + ); + + if (!is_resource($process)) { + return null; + } + + fclose($pipes[0]); + + $stdout = (string) stream_get_contents($pipes[1]); + $stderr = (string) stream_get_contents($pipes[2]); + + proc_close($process); + + return [$stdout, $stderr]; + } + + private function __construct() + { + } +} diff --git a/src/Finder/HwLogicalFinder.php b/src/Finder/HwLogicalFinder.php index 29ce494..a45156e 100644 --- a/src/Finder/HwLogicalFinder.php +++ b/src/Finder/HwLogicalFinder.php @@ -19,7 +19,7 @@ * @see https://github.com/paratestphp/paratest/blob/c163539818fd96308ca8dc60f46088461e366ed4/src/Runners/PHPUnit/Options.php#L903-L909 * @see https://opensource.apple.com/source/xnu/xnu-792.2.4/libkern/libkern/sysctl.h.auto.html */ -final class HwLogicalFinder extends PopenBasedFinder +final class HwLogicalFinder extends ProcOpenBasedFinder { protected function getCommand(): string { diff --git a/src/Finder/HwPhysicalFinder.php b/src/Finder/HwPhysicalFinder.php index 9aabaff..918af50 100644 --- a/src/Finder/HwPhysicalFinder.php +++ b/src/Finder/HwPhysicalFinder.php @@ -19,7 +19,7 @@ * @see https://github.com/paratestphp/paratest/blob/c163539818fd96308ca8dc60f46088461e366ed4/src/Runners/PHPUnit/Options.php#L903-L909 * @see https://opensource.apple.com/source/xnu/xnu-792.2.4/libkern/libkern/sysctl.h.auto.html */ -final class HwPhysicalFinder extends PopenBasedFinder +final class HwPhysicalFinder extends ProcOpenBasedFinder { protected function getCommand(): string { diff --git a/src/Finder/LinuxyNProcessorFinder.php b/src/Finder/LinuxyNProcessorFinder.php index 6a607dc..ddfe20d 100644 --- a/src/Finder/LinuxyNProcessorFinder.php +++ b/src/Finder/LinuxyNProcessorFinder.php @@ -18,7 +18,7 @@ * * @see https://twitter.com/freebsdfrau/status/1052016199452700678?s=20&t=M2pHkRqmmna-UF68lfL2hw */ -final class LinuxyNProcessorFinder extends PopenBasedFinder +final class LinuxyNProcessorFinder extends ProcOpenBasedFinder { protected function getCommand(): string { diff --git a/src/Finder/NProcessorFinder.php b/src/Finder/NProcessorFinder.php index 200f6c3..58603c3 100644 --- a/src/Finder/NProcessorFinder.php +++ b/src/Finder/NProcessorFinder.php @@ -18,7 +18,7 @@ * * @see https://twitter.com/freebsdfrau/status/1052016199452700678?s=20&t=M2pHkRqmmna-UF68lfL2hw */ -final class NProcessorFinder extends PopenBasedFinder +final class NProcessorFinder extends ProcOpenBasedFinder { protected function getCommand(): string { diff --git a/src/Finder/PopenBasedFinder.php b/src/Finder/ProcOpenBasedFinder.php similarity index 54% rename from src/Finder/PopenBasedFinder.php rename to src/Finder/ProcOpenBasedFinder.php index 7fb48d0..039db28 100644 --- a/src/Finder/PopenBasedFinder.php +++ b/src/Finder/ProcOpenBasedFinder.php @@ -13,59 +13,50 @@ namespace Fidry\CpuCoreCounter\Finder; -use function fgets; +use Fidry\CpuCoreCounter\Exec\ProcOpen; use function filter_var; use function function_exists; use function is_int; -use function is_resource; -use function pclose; -use function popen; use function sprintf; use function strrpos; use function substr; +use function trim; use const FILTER_VALIDATE_INT; use const PHP_EOL; -abstract class PopenBasedFinder implements CpuCoreFinder +abstract class ProcOpenBasedFinder implements CpuCoreFinder { public function diagnose(): string { - if (!function_exists('popen')) { - return 'The function "popen" is not available.'; + if (!function_exists('proc_open')) { + return 'The function "proc_open" is not available.'; } - // Redirect the STDERR to the STDOUT since popen cannot capture the - // STDERR. - // We could use proc_open but this would be a greater difference between - // the command we really execute when using the finder and what we will - // diagnose. - $command = $this->getCommand().' 2>&1'; + $command = $this->getCommand(); + $output = ProcOpen::execute($command); - $process = popen($command, 'rb'); - - if (!is_resource($process)) { + if (null === $output) { return sprintf( - 'Could not execute the function "popen" with the command "%s".', + 'Failed to execute the command "%s".', $command ); } - $processResult = fgets($process); - $exitCode = pclose($process); + [$stdout, $stderr] = $output; + $failed = '' !== trim($stderr); - return 0 === $exitCode + return $failed ? sprintf( - 'Executed the command "%s" and got the following output:%s%s', + 'Executed the command "%s" which wrote the following output to the STDERR:%s%s', $command, PHP_EOL, - $processResult + $stderr ) : sprintf( - 'Executed the command "%s" which exited with a non-success exit code (%d) with the following output:%s%s', + 'Executed the command "%s" and got the following (STDOUT) output:%s%s', $command, - $exitCode, PHP_EOL, - $processResult + $stdout ); } @@ -74,22 +65,22 @@ public function diagnose(): string */ public function find(): ?int { - if (!function_exists('popen')) { + if (!function_exists('proc_open')) { return null; } - $process = popen($this->getCommand(), 'rb'); + $output = ProcOpen::execute($this->getCommand()); - if (!is_resource($process)) { + if (null === $output) { return null; } - $processResult = fgets($process); - pclose($process); + [$stdout, $stderr] = $output; + $failed = '' !== trim($stderr); - return false === $processResult + return $failed ? null - : self::countCpuCores($processResult); + : self::countCpuCores($stdout); } public function toString(): string diff --git a/src/Finder/WindowsWmicLogicalFinder.php b/src/Finder/WindowsWmicLogicalFinder.php index 5a56485..77ef63d 100644 --- a/src/Finder/WindowsWmicLogicalFinder.php +++ b/src/Finder/WindowsWmicLogicalFinder.php @@ -20,7 +20,7 @@ * * @see https://github.com/paratestphp/paratest/blob/c163539818fd96308ca8dc60f46088461e366ed4/src/Runners/PHPUnit/Options.php#L912-L916 */ -final class WindowsWmicLogicalFinder extends PopenBasedFinder +final class WindowsWmicLogicalFinder extends ProcOpenBasedFinder { /** * @return positive-int|null diff --git a/src/Finder/WindowsWmicPhysicalFinder.php b/src/Finder/WindowsWmicPhysicalFinder.php index 78c947b..5dedcfe 100644 --- a/src/Finder/WindowsWmicPhysicalFinder.php +++ b/src/Finder/WindowsWmicPhysicalFinder.php @@ -20,7 +20,7 @@ * * @see https://github.com/paratestphp/paratest/blob/c163539818fd96308ca8dc60f46088461e366ed4/src/Runners/PHPUnit/Options.php#L912-L916 */ -final class WindowsWmicPhysicalFinder extends PopenBasedFinder +final class WindowsWmicPhysicalFinder extends ProcOpenBasedFinder { /** * @return positive-int|null diff --git a/tests/Exec/ProcOpenTest.php b/tests/Exec/ProcOpenTest.php new file mode 100644 index 0000000..781364f --- /dev/null +++ b/tests/Exec/ProcOpenTest.php @@ -0,0 +1,56 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Fidry\CpuCoreCounter\Test\Exec; + +use Fidry\CpuCoreCounter\Exec\ProcOpen; +use PHPUnit\Framework\TestCase; +use const PHP_EOL; + +/** + * @covers \Fidry\CpuCoreCounter\Exec\ProcOpen + * + * @internal + */ +final class ProcOpenTest extends TestCase +{ + public function test_it_can_execute_a_command_writing_output_to_the_stdout(): void + { + $command = 'echo "Hello world!"'; + + $expected = ['Hello world!'.PHP_EOL, '']; + $actual = ProcOpen::execute($command); + + self::assertSame($expected, $actual); + } + + public function test_it_can_execute_a_command_writing_output_to_the_stderr_instead_of_the_stdout(): void + { + $command = 'echo "Hello world!" 1>&2'; + + $expected = ['', 'Hello world!'.PHP_EOL]; + $actual = ProcOpen::execute($command); + + self::assertSame($expected, $actual); + } + + public function test_it_can_execute_a_command_writing_output_to_the_stdout_instead_of_the_stderr(): void + { + $command = 'echoerr() { echo "$@" 1>&2; }; echoerr "Hello world!" 2>&1'; + + $expected = ['Hello world!'.PHP_EOL, '']; + $actual = ProcOpen::execute($command); + + self::assertSame($expected, $actual); + } +} diff --git a/tests/Finder/DummyPopenBasedFinder.php b/tests/Finder/DummyProcOpenBasedFinder.php similarity index 77% rename from tests/Finder/DummyPopenBasedFinder.php rename to tests/Finder/DummyProcOpenBasedFinder.php index cfbf36e..51800d7 100644 --- a/tests/Finder/DummyPopenBasedFinder.php +++ b/tests/Finder/DummyProcOpenBasedFinder.php @@ -13,9 +13,9 @@ namespace Fidry\CpuCoreCounter\Test\Finder; -use Fidry\CpuCoreCounter\Finder\PopenBasedFinder; +use Fidry\CpuCoreCounter\Finder\ProcOpenBasedFinder; -final class DummyPopenBasedFinder extends PopenBasedFinder +final class DummyProcOpenBasedFinder extends ProcOpenBasedFinder { protected function getCommand(): string { diff --git a/tests/Finder/HwLogicalFinderTest.php b/tests/Finder/HwLogicalFinderTest.php index a690ab2..5b9c57a 100644 --- a/tests/Finder/HwLogicalFinderTest.php +++ b/tests/Finder/HwLogicalFinderTest.php @@ -14,21 +14,21 @@ namespace Fidry\CpuCoreCounter\Test\Finder; use Fidry\CpuCoreCounter\Finder\HwLogicalFinder; -use Fidry\CpuCoreCounter\Finder\PopenBasedFinder; +use Fidry\CpuCoreCounter\Finder\ProcOpenBasedFinder; /** * @covers \Fidry\CpuCoreCounter\Finder\HwLogicalFinder * * @internal */ -final class HwLogicalFinderTest extends PopenBasedFinderTestCase +final class HwLogicalFinderTest extends ProcOpenBasedFinderTestCase { public function test_it_can_describe_itself(): void { self::assertSame('HwLogicalFinder', $this->getFinder()->toString()); } - protected function getFinder(): PopenBasedFinder + protected function getFinder(): ProcOpenBasedFinder { return new HwLogicalFinder(); } diff --git a/tests/Finder/HwPhysicalFinderTest.php b/tests/Finder/HwPhysicalFinderTest.php index 8bd0745..29d49be 100644 --- a/tests/Finder/HwPhysicalFinderTest.php +++ b/tests/Finder/HwPhysicalFinderTest.php @@ -14,21 +14,21 @@ namespace Fidry\CpuCoreCounter\Test\Finder; use Fidry\CpuCoreCounter\Finder\HwPhysicalFinder; -use Fidry\CpuCoreCounter\Finder\PopenBasedFinder; +use Fidry\CpuCoreCounter\Finder\ProcOpenBasedFinder; /** * @covers \Fidry\CpuCoreCounter\Finder\HwPhysicalFinder * * @internal */ -final class HwPhysicalFinderTest extends PopenBasedFinderTestCase +final class HwPhysicalFinderTest extends ProcOpenBasedFinderTestCase { public function test_it_can_describe_itself(): void { self::assertSame('HwPhysicalFinder', $this->getFinder()->toString()); } - protected function getFinder(): PopenBasedFinder + protected function getFinder(): ProcOpenBasedFinder { return new HwPhysicalFinder(); } diff --git a/tests/Finder/LinuxyNProcessorFinderTest.php b/tests/Finder/LinuxyNProcessorFinderTest.php index 553464a..3007abb 100644 --- a/tests/Finder/LinuxyNProcessorFinderTest.php +++ b/tests/Finder/LinuxyNProcessorFinderTest.php @@ -14,21 +14,21 @@ namespace Fidry\CpuCoreCounter\Test\Finder; use Fidry\CpuCoreCounter\Finder\LinuxyNProcessorFinder; -use Fidry\CpuCoreCounter\Finder\PopenBasedFinder; +use Fidry\CpuCoreCounter\Finder\ProcOpenBasedFinder; /** * @covers \Fidry\CpuCoreCounter\Finder\LinuxyNProcessorFinder * * @internal */ -final class LinuxyNProcessorFinderTest extends PopenBasedFinderTestCase +final class LinuxyNProcessorFinderTest extends ProcOpenBasedFinderTestCase { public function test_it_can_describe_itself(): void { self::assertSame('LinuxyNProcessorFinder', $this->getFinder()->toString()); } - protected function getFinder(): PopenBasedFinder + protected function getFinder(): ProcOpenBasedFinder { return new LinuxyNProcessorFinder(); } diff --git a/tests/Finder/NProcessorFinderTest.php b/tests/Finder/NProcessorFinderTest.php index efdc888..e2ac8c1 100644 --- a/tests/Finder/NProcessorFinderTest.php +++ b/tests/Finder/NProcessorFinderTest.php @@ -14,21 +14,21 @@ namespace Fidry\CpuCoreCounter\Test\Finder; use Fidry\CpuCoreCounter\Finder\NProcessorFinder; -use Fidry\CpuCoreCounter\Finder\PopenBasedFinder; +use Fidry\CpuCoreCounter\Finder\ProcOpenBasedFinder; /** * @covers \Fidry\CpuCoreCounter\Finder\NProcessorFinder * * @internal */ -final class NProcessorFinderTest extends PopenBasedFinderTestCase +final class NProcessorFinderTest extends ProcOpenBasedFinderTestCase { public function test_it_can_describe_itself(): void { self::assertSame('NProcessorFinder', $this->getFinder()->toString()); } - protected function getFinder(): PopenBasedFinder + protected function getFinder(): ProcOpenBasedFinder { return new NProcessorFinder(); } diff --git a/tests/Finder/PopenBasedFinderTest.php b/tests/Finder/ProcOpenBasedFinderTest.php similarity index 81% rename from tests/Finder/PopenBasedFinderTest.php rename to tests/Finder/ProcOpenBasedFinderTest.php index c1c51f5..1c54e34 100644 --- a/tests/Finder/PopenBasedFinderTest.php +++ b/tests/Finder/ProcOpenBasedFinderTest.php @@ -16,11 +16,11 @@ use PHPUnit\Framework\TestCase; /** - * @covers \Fidry\CpuCoreCounter\Finder\PopenBasedFinder + * @covers \Fidry\CpuCoreCounter\Finder\ProcOpenBasedFinder * * @internal */ -final class PopenBasedFinderTest extends TestCase +final class ProcOpenBasedFinderTest extends TestCase { /** * @dataProvider popenFgetsProvider @@ -29,7 +29,7 @@ public function test_it_can_count_the_number_of_cpu_cores( string $nproc, ?int $expected ): void { - $actual = DummyPopenBasedFinder::countCpuCores($nproc); + $actual = DummyProcOpenBasedFinder::countCpuCores($nproc); self::assertSame($expected, $actual); } @@ -82,8 +82,8 @@ public static function popenFgetsProvider(): iterable public function test_it_can_describe_itself(): void { - $finder = new DummyPopenBasedFinder(); + $finder = new DummyProcOpenBasedFinder(); - self::assertSame('DummyPopenBasedFinder', $finder->toString()); + self::assertSame('DummyProcOpenBasedFinder', $finder->toString()); } } diff --git a/tests/Finder/PopenBasedFinderTestCase.php b/tests/Finder/ProcOpenBasedFinderTestCase.php similarity index 88% rename from tests/Finder/PopenBasedFinderTestCase.php rename to tests/Finder/ProcOpenBasedFinderTestCase.php index 95c7f5f..1b9a261 100644 --- a/tests/Finder/PopenBasedFinderTestCase.php +++ b/tests/Finder/ProcOpenBasedFinderTestCase.php @@ -13,10 +13,10 @@ namespace Fidry\CpuCoreCounter\Test\Finder; -use Fidry\CpuCoreCounter\Finder\PopenBasedFinder; +use Fidry\CpuCoreCounter\Finder\ProcOpenBasedFinder; use PHPUnit\Framework\TestCase; -abstract class PopenBasedFinderTestCase extends TestCase +abstract class ProcOpenBasedFinderTestCase extends TestCase { /** * @dataProvider processResultProvider @@ -76,5 +76,5 @@ public static function processResultProvider(): iterable ]; } - abstract protected function getFinder(): PopenBasedFinder; + abstract protected function getFinder(): ProcOpenBasedFinder; } diff --git a/tests/Finder/WindowsWmicLogicalFinderTest.php b/tests/Finder/WindowsWmicLogicalFinderTest.php index ac108f6..55c74c2 100644 --- a/tests/Finder/WindowsWmicLogicalFinderTest.php +++ b/tests/Finder/WindowsWmicLogicalFinderTest.php @@ -13,7 +13,7 @@ namespace Fidry\CpuCoreCounter\Test\Finder; -use Fidry\CpuCoreCounter\Finder\PopenBasedFinder; +use Fidry\CpuCoreCounter\Finder\ProcOpenBasedFinder; use Fidry\CpuCoreCounter\Finder\WindowsWmicLogicalFinder; /** @@ -21,14 +21,14 @@ * * @internal */ -final class WindowsWmicLogicalFinderTest extends PopenBasedFinderTestCase +final class WindowsWmicLogicalFinderTest extends ProcOpenBasedFinderTestCase { public function test_it_can_describe_itself(): void { self::assertSame('WindowsWmicLogicalFinder', $this->getFinder()->toString()); } - protected function getFinder(): PopenBasedFinder + protected function getFinder(): ProcOpenBasedFinder { return new WindowsWmicLogicalFinder(); } diff --git a/tests/Finder/WindowsWmicPhysicalFinderTest.php b/tests/Finder/WindowsWmicPhysicalFinderTest.php index dd74182..9ae8084 100644 --- a/tests/Finder/WindowsWmicPhysicalFinderTest.php +++ b/tests/Finder/WindowsWmicPhysicalFinderTest.php @@ -13,7 +13,7 @@ namespace Fidry\CpuCoreCounter\Test\Finder; -use Fidry\CpuCoreCounter\Finder\PopenBasedFinder; +use Fidry\CpuCoreCounter\Finder\ProcOpenBasedFinder; use Fidry\CpuCoreCounter\Finder\WindowsWmicPhysicalFinder; /** @@ -21,14 +21,14 @@ * * @internal */ -final class WindowsWmicPhysicalFinderTest extends PopenBasedFinderTestCase +final class WindowsWmicPhysicalFinderTest extends ProcOpenBasedFinderTestCase { public function test_it_can_describe_itself(): void { self::assertSame('WindowsWmicPhysicalFinder', $this->getFinder()->toString()); } - protected function getFinder(): PopenBasedFinder + protected function getFinder(): ProcOpenBasedFinder { return new WindowsWmicPhysicalFinder(); }