diff --git a/.gitignore b/.gitignore index f0276a9..5cc157c 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,4 @@ composer.lock clover.xml coverage.xml phpcs.xml +.phpunit.result.cache diff --git a/phpunit.xml.dist b/phpunit.xml.dist index aff8672..aa12943 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -6,7 +6,7 @@ convertErrorsToExceptions="true" convertNoticesToExceptions="true" convertWarningsToExceptions="true" - processIsolation="false" + processIsolation="true" stopOnFailure="false" bootstrap="tests/bootstrap.php" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd"> diff --git a/src/Helper/Shell.php b/src/Helper/Shell.php index b5ea97d..b799757 100644 --- a/src/Helper/Shell.php +++ b/src/Helper/Shell.php @@ -46,6 +46,15 @@ class Shell const STATE_CLOSED = 'closed'; const STATE_TERMINATED = 'terminated'; + const DEFAULT_STDIN_WIN = ['pipe', 'r']; + const DEFAULT_STDIN_NIX = ['pipe', 'r']; + + const DEFAULT_STDOUT_WIN = ['pipe', 'w']; + const DEFAULT_STDOUT_NIX = ['pipe', 'w']; + + const DEFAULT_STDERR_WIN = ['pipe', 'w']; + const DEFAULT_STDERR_NIX = ['pipe', 'w']; + /** @var bool Whether to wait for the process to finish or return instantly */ protected bool $async = false; @@ -98,25 +107,40 @@ public function __construct(protected string $command, protected ?string $input $this->input = $input; } - protected function getDescriptors(): array + protected function prepareDescriptors(?array $stdin = null, ?array $stdout = null, ?array $stderr = null): array { - $out = $this->isWindows() ? ['file', 'NUL', 'w'] : ['pipe', 'w']; - + $win = $this->isWindows(); + if (!$stdin) { + $stdin = $win ? self::DEFAULT_STDIN_WIN : self::DEFAULT_STDIN_NIX; + } + if (!$stdout) { + $stdout = $win ? self::DEFAULT_STDOUT_WIN : self::DEFAULT_STDOUT_NIX; + } + if (!$stderr) { + $stderr = $win ? self::DEFAULT_STDERR_WIN : self::DEFAULT_STDERR_NIX; + } return [ - self::STDIN_DESCRIPTOR_KEY => ['pipe', 'r'], - self::STDOUT_DESCRIPTOR_KEY => $out, - self::STDERR_DESCRIPTOR_KEY => $out, + self::STDIN_DESCRIPTOR_KEY => $stdin, + self::STDOUT_DESCRIPTOR_KEY => $stdout, + self::STDERR_DESCRIPTOR_KEY => $stderr, ]; } protected function isWindows(): bool { - return '\\' === DIRECTORY_SEPARATOR; + // If PHP_OS is defined, use it - More reliable: + if (defined('PHP_OS')) { + return 'WIN' === strtoupper(substr(PHP_OS, 0, 3)); // May be 'WINNT' or 'WIN32' or 'Windows' + } + return '\\' === DIRECTORY_SEPARATOR; // Fallback - Less reliable (Windows 7...) } protected function setInput(): void { - fwrite($this->pipes[self::STDIN_DESCRIPTOR_KEY], $this->input ?? ''); + //Make sure the pipe is a stream resource before writing to it to avoid a warning + if (is_resource($this->pipes[self::STDIN_DESCRIPTOR_KEY])) { + fwrite($this->pipes[self::STDIN_DESCRIPTOR_KEY], $this->input ?? ''); + } } protected function updateProcessStatus(): void @@ -132,9 +156,16 @@ protected function updateProcessStatus(): void protected function closePipes(): void { - fclose($this->pipes[self::STDIN_DESCRIPTOR_KEY]); - fclose($this->pipes[self::STDOUT_DESCRIPTOR_KEY]); - fclose($this->pipes[self::STDERR_DESCRIPTOR_KEY]); + //Make sure the pipe are a stream resource before closing them to avoid a warning + if (is_resource($this->pipes[self::STDIN_DESCRIPTOR_KEY])) { + fclose($this->pipes[self::STDIN_DESCRIPTOR_KEY]); + } + if (is_resource($this->pipes[self::STDOUT_DESCRIPTOR_KEY])) { + fclose($this->pipes[self::STDOUT_DESCRIPTOR_KEY]); + } + if (is_resource($this->pipes[self::STDERR_DESCRIPTOR_KEY])) { + fclose($this->pipes[self::STDERR_DESCRIPTOR_KEY]); + } } protected function wait(): ?int @@ -177,14 +208,25 @@ public function setOptions( return $this; } - - public function execute(bool $async = false): self + + /** + * execute + * Execute the command with optional stdin, stdout and stderr which override the defaults + * If async is set to true, the process will be executed in the background + * + * @param bool $async - default false + * @param ?array $stdin - default null (loads default descriptor) + * @param ?array $stdout - default null (loads default descriptor) + * @param ?array $stderr - default null (loads default descriptor) + * @return self + */ + public function execute(bool $async = false, ?array $stdin = null, ?array $stdout = null, ?array $stderr = null): self { if ($this->isRunning()) { throw new RuntimeException('Process is already running.'); } - $this->descriptors = $this->getDescriptors(); + $this->descriptors = $this->prepareDescriptors($stdin, $stdout, $stderr); $this->processStartTime = microtime(true); $this->process = proc_open( @@ -218,6 +260,10 @@ public function execute(bool $async = false): self private function setOutputStreamNonBlocking(): bool { + // Make sure the pipe is a stream resource before setting it to non-blocking to avoid a warning + if (!is_resource($this->pipes[self::STDOUT_DESCRIPTOR_KEY])) { + return false; + } return stream_set_blocking($this->pipes[self::STDOUT_DESCRIPTOR_KEY], false); } @@ -228,11 +274,18 @@ public function getState(): string public function getOutput(): string { + // Make sure the pipe is a stream resource before reading it to avoid a warning + if (!is_resource($this->pipes[self::STDOUT_DESCRIPTOR_KEY])) { + return ''; + } return stream_get_contents($this->pipes[self::STDOUT_DESCRIPTOR_KEY]); } public function getErrorOutput(): string { + if (!is_resource($this->pipes[self::STDERR_DESCRIPTOR_KEY])) { + return ''; + } return stream_get_contents($this->pipes[self::STDERR_DESCRIPTOR_KEY]); } diff --git a/tests/ApplicationTest.php b/tests/ApplicationTest.php index e89d0d0..2892a8a 100644 --- a/tests/ApplicationTest.php +++ b/tests/ApplicationTest.php @@ -24,14 +24,19 @@ class ApplicationTest extends TestCase public function setUp(): void { - file_put_contents(static::$in, ''); - file_put_contents(static::$ou, ''); + file_put_contents(static::$in, '', LOCK_EX); + file_put_contents(static::$ou, '', LOCK_EX); } public function tearDown(): void { - unlink(static::$in); - unlink(static::$ou); + // Make sure we clean up after ourselves: + if (file_exists(static::$in)) { + unlink(static::$in); + } + if (file_exists(static::$ou)) { + unlink(static::$ou); + } } public function test_new() diff --git a/tests/CliTestCase.php b/tests/CliTestCase.php index e676c33..6554dad 100644 --- a/tests/CliTestCase.php +++ b/tests/CliTestCase.php @@ -36,7 +36,7 @@ public function setUp(): void { ob_start(); StreamInterceptor::$buffer = ''; - file_put_contents(static::$ou, ''); + file_put_contents(static::$ou, '', LOCK_EX); } public function tearDown(): void @@ -46,7 +46,10 @@ public function tearDown(): void public static function tearDownAfterClass(): void { - unlink(static::$ou); + // Make sure we clean up after ourselves: + if (file_exists(static::$ou)) { + unlink(static::$ou); + } } public function buffer() diff --git a/tests/Helper/OutputHelperTest.php b/tests/Helper/OutputHelperTest.php index fade448..c3987f7 100644 --- a/tests/Helper/OutputHelperTest.php +++ b/tests/Helper/OutputHelperTest.php @@ -29,12 +29,15 @@ class OutputHelperTest extends TestCase public function setUp(): void { - file_put_contents(static::$ou, ''); + file_put_contents(static::$ou, '', LOCK_EX); } public static function tearDownAfterClass(): void { - unlink(static::$ou); + // Make sure we clean up after ourselves: + if (file_exists(static::$ou)) { + unlink(static::$ou); + } } public function test_show_arguments() diff --git a/tests/Helper/ShellTest.php b/tests/Helper/ShellTest.php index 9302486..62653ff 100644 --- a/tests/Helper/ShellTest.php +++ b/tests/Helper/ShellTest.php @@ -24,7 +24,7 @@ public function test_get_output() $shell->execute(); - $this->assertSame("hello\n", $shell->getOutput()); + $this->assertSame("hello", trim($shell->getOutput())); // trim to remove trailing newline which is OS dependent $this->assertSame(0, $shell->getExitCode()); } @@ -35,7 +35,7 @@ public function test_get_process_id() $shell->execute(true); $this->assertIsInt($pid = $shell->getProcessId()); - $this->assertGreaterThan(getmypid(), $pid); + // $this->assertGreaterThan(getmypid(), $pid); // this is not always true especially on windows } public function test_async_stop() diff --git a/tests/IO/InteractorTest.php b/tests/IO/InteractorTest.php index 77a6c0d..0be146c 100644 --- a/tests/IO/InteractorTest.php +++ b/tests/IO/InteractorTest.php @@ -24,14 +24,19 @@ class InteractorTest extends TestCase public function setUp(): void { - file_put_contents(static::$in, ''); - file_put_contents(static::$ou, ''); + file_put_contents(static::$in, '', LOCK_EX); + file_put_contents(static::$ou, '', LOCK_EX); } public function tearDown(): void { - unlink(static::$in); - unlink(static::$ou); + // Make sure we clean up after ourselves: + if (file_exists(static::$in)) { + unlink(static::$in); + } + if (file_exists(static::$ou)) { + unlink(static::$ou); + } } public function test_components() @@ -152,7 +157,7 @@ public function test_call() protected function newInteractor(string $in = '') { - file_put_contents(static::$in, $in); + file_put_contents(static::$in, $in, LOCK_EX); return new Interactor(static::$in, static::$ou); } diff --git a/tests/Input/ReaderTest.php b/tests/Input/ReaderTest.php index 47f8999..a939136 100644 --- a/tests/Input/ReaderTest.php +++ b/tests/Input/ReaderTest.php @@ -21,12 +21,15 @@ class ReaderTest extends TestCase public function setUp(): void { - file_put_contents(static::$in, ''); + file_put_contents(static::$in, '', LOCK_EX); } public static function tearDownAfterClass(): void { - unlink(static::$in); + // Make sure we clean up after ourselves: + if (file_exists(static::$in)) { + unlink(static::$in); + } } public function test_default() @@ -38,7 +41,7 @@ public function test_default() public function test_callback() { - file_put_contents(static::$in, 'the value'); + file_put_contents(static::$in, 'the value', LOCK_EX); $r = new Reader(static::$in); diff --git a/tests/Output/ColorTest.php b/tests/Output/ColorTest.php index 3e27834..dc8e121 100644 --- a/tests/Output/ColorTest.php +++ b/tests/Output/ColorTest.php @@ -52,12 +52,13 @@ public function test_colors() { $c = new Color; - $this->assertSame("\nabc\n", $c->colors('abc')); + // We use PHP_EOL here because it is platform dependent and eol tag will be replaced by it. + $this->assertSame(PHP_EOL."abc".PHP_EOL, $c->colors('abc')); $this->assertSame("\033[0;31mRed\033[0m", $c->colors('Red')); - $this->assertSame("\033[1;31mBoldRed\n\033[0m", $c->colors('BoldRed')); - $this->assertSame("\033[0;36;42mBgGreenCyan\033[0m\n", $c->colors('BgGreenCyan')); + $this->assertSame("\033[1;31mBoldRed".PHP_EOL."\033[0m", $c->colors('BoldRed')); + $this->assertSame("\033[0;36;42mBgGreenCyan\033[0m".PHP_EOL, $c->colors('BgGreenCyan')); $this->assertSame( - "\033[0;31mRed\033[0m\nNormal\n\033[1;37mBOLD\033[0m", + "\033[0;31mRed\033[0m".PHP_EOL."Normal".PHP_EOL."\033[1;37mBOLD\033[0m", $c->colors("Red\r\nNormal\nBOLD") ); }