Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change PopenBased finders to ProcOpenBased finders #75

Merged
merged 1 commit into from
Dec 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions infection.json5
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
]
Expand Down
58 changes: 58 additions & 0 deletions src/Exec/ProcOpen.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

/*
* This file is part of the Fidry CPUCounter Config package.
*
* (c) Théo FIDRY <[email protected]>
*
* 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()
{
}
}
2 changes: 1 addition & 1 deletion src/Finder/HwLogicalFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
2 changes: 1 addition & 1 deletion src/Finder/HwPhysicalFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
2 changes: 1 addition & 1 deletion src/Finder/LinuxyNProcessorFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
2 changes: 1 addition & 1 deletion src/Finder/NProcessorFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
}

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/Finder/WindowsWmicLogicalFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/Finder/WindowsWmicPhysicalFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
56 changes: 56 additions & 0 deletions tests/Exec/ProcOpenTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php

/*
* This file is part of the Fidry CPUCounter Config package.
*
* (c) Théo FIDRY <[email protected]>
*
* 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
6 changes: 3 additions & 3 deletions tests/Finder/HwLogicalFinderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
6 changes: 3 additions & 3 deletions tests/Finder/HwPhysicalFinderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
6 changes: 3 additions & 3 deletions tests/Finder/LinuxyNProcessorFinderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Loading