Skip to content

Commit 4c2bab4

Browse files
adhocoreadhocore
authored
Merge pull request #77 from shlomohass/main
Had issues on windows - fixed them + added custom descriptors support to Shell
2 parents a91a04c + 3acd661 commit 4c2bab4

File tree

10 files changed

+111
-37
lines changed

10 files changed

+111
-37
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@ composer.lock
1010
clover.xml
1111
coverage.xml
1212
phpcs.xml
13+
.phpunit.result.cache

‎phpunit.xml.dist‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
convertErrorsToExceptions="true"
77
convertNoticesToExceptions="true"
88
convertWarningsToExceptions="true"
9-
processIsolation="false"
9+
processIsolation="true"
1010
stopOnFailure="false"
1111
bootstrap="tests/bootstrap.php"
1212
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">

‎src/Helper/Shell.php‎

Lines changed: 67 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,15 @@ class Shell
4646
const STATE_CLOSED = 'closed';
4747
const STATE_TERMINATED = 'terminated';
4848

49+
const DEFAULT_STDIN_WIN = ['pipe', 'r'];
50+
const DEFAULT_STDIN_NIX = ['pipe', 'r'];
51+
52+
const DEFAULT_STDOUT_WIN = ['pipe', 'w'];
53+
const DEFAULT_STDOUT_NIX = ['pipe', 'w'];
54+
55+
const DEFAULT_STDERR_WIN = ['pipe', 'w'];
56+
const DEFAULT_STDERR_NIX = ['pipe', 'w'];
57+
4958
/** @var bool Whether to wait for the process to finish or return instantly */
5059
protected bool $async = false;
5160

@@ -98,25 +107,40 @@ public function __construct(protected string $command, protected ?string $input
98107
$this->input = $input;
99108
}
100109

101-
protected function getDescriptors(): array
110+
protected function prepareDescriptors(?array $stdin = null, ?array $stdout = null, ?array $stderr = null): array
102111
{
103-
$out = $this->isWindows() ? ['file', 'NUL', 'w'] : ['pipe', 'w'];
104-
112+
$win = $this->isWindows();
113+
if (!$stdin) {
114+
$stdin = $win ? self::DEFAULT_STDIN_WIN : self::DEFAULT_STDIN_NIX;
115+
}
116+
if (!$stdout) {
117+
$stdout = $win ? self::DEFAULT_STDOUT_WIN : self::DEFAULT_STDOUT_NIX;
118+
}
119+
if (!$stderr) {
120+
$stderr = $win ? self::DEFAULT_STDERR_WIN : self::DEFAULT_STDERR_NIX;
121+
}
105122
return [
106-
self::STDIN_DESCRIPTOR_KEY => ['pipe', 'r'],
107-
self::STDOUT_DESCRIPTOR_KEY => $out,
108-
self::STDERR_DESCRIPTOR_KEY => $out,
123+
self::STDIN_DESCRIPTOR_KEY => $stdin,
124+
self::STDOUT_DESCRIPTOR_KEY => $stdout,
125+
self::STDERR_DESCRIPTOR_KEY => $stderr,
109126
];
110127
}
111128

112129
protected function isWindows(): bool
113130
{
114-
return '\\' === DIRECTORY_SEPARATOR;
131+
// If PHP_OS is defined, use it - More reliable:
132+
if (defined('PHP_OS')) {
133+
return 'WIN' === strtoupper(substr(PHP_OS, 0, 3)); // May be 'WINNT' or 'WIN32' or 'Windows'
134+
}
135+
return '\\' === DIRECTORY_SEPARATOR; // Fallback - Less reliable (Windows 7...)
115136
}
116137

117138
protected function setInput(): void
118139
{
119-
fwrite($this->pipes[self::STDIN_DESCRIPTOR_KEY], $this->input ?? '');
140+
//Make sure the pipe is a stream resource before writing to it to avoid a warning
141+
if (is_resource($this->pipes[self::STDIN_DESCRIPTOR_KEY])) {
142+
fwrite($this->pipes[self::STDIN_DESCRIPTOR_KEY], $this->input ?? '');
143+
}
120144
}
121145

122146
protected function updateProcessStatus(): void
@@ -132,9 +156,16 @@ protected function updateProcessStatus(): void
132156

133157
protected function closePipes(): void
134158
{
135-
fclose($this->pipes[self::STDIN_DESCRIPTOR_KEY]);
136-
fclose($this->pipes[self::STDOUT_DESCRIPTOR_KEY]);
137-
fclose($this->pipes[self::STDERR_DESCRIPTOR_KEY]);
159+
//Make sure the pipe are a stream resource before closing them to avoid a warning
160+
if (is_resource($this->pipes[self::STDIN_DESCRIPTOR_KEY])) {
161+
fclose($this->pipes[self::STDIN_DESCRIPTOR_KEY]);
162+
}
163+
if (is_resource($this->pipes[self::STDOUT_DESCRIPTOR_KEY])) {
164+
fclose($this->pipes[self::STDOUT_DESCRIPTOR_KEY]);
165+
}
166+
if (is_resource($this->pipes[self::STDERR_DESCRIPTOR_KEY])) {
167+
fclose($this->pipes[self::STDERR_DESCRIPTOR_KEY]);
168+
}
138169
}
139170

140171
protected function wait(): ?int
@@ -177,14 +208,25 @@ public function setOptions(
177208

178209
return $this;
179210
}
180-
181-
public function execute(bool $async = false): self
211+
212+
/**
213+
* execute
214+
* Execute the command with optional stdin, stdout and stderr which override the defaults
215+
* If async is set to true, the process will be executed in the background
216+
*
217+
* @param bool $async - default false
218+
* @param ?array $stdin - default null (loads default descriptor)
219+
* @param ?array $stdout - default null (loads default descriptor)
220+
* @param ?array $stderr - default null (loads default descriptor)
221+
* @return self
222+
*/
223+
public function execute(bool $async = false, ?array $stdin = null, ?array $stdout = null, ?array $stderr = null): self
182224
{
183225
if ($this->isRunning()) {
184226
throw new RuntimeException('Process is already running.');
185227
}
186228

187-
$this->descriptors = $this->getDescriptors();
229+
$this->descriptors = $this->prepareDescriptors($stdin, $stdout, $stderr);
188230
$this->processStartTime = microtime(true);
189231

190232
$this->process = proc_open(
@@ -218,6 +260,10 @@ public function execute(bool $async = false): self
218260

219261
private function setOutputStreamNonBlocking(): bool
220262
{
263+
// Make sure the pipe is a stream resource before setting it to non-blocking to avoid a warning
264+
if (!is_resource($this->pipes[self::STDOUT_DESCRIPTOR_KEY])) {
265+
return false;
266+
}
221267
return stream_set_blocking($this->pipes[self::STDOUT_DESCRIPTOR_KEY], false);
222268
}
223269

@@ -228,11 +274,18 @@ public function getState(): string
228274

229275
public function getOutput(): string
230276
{
277+
// Make sure the pipe is a stream resource before reading it to avoid a warning
278+
if (!is_resource($this->pipes[self::STDOUT_DESCRIPTOR_KEY])) {
279+
return '';
280+
}
231281
return stream_get_contents($this->pipes[self::STDOUT_DESCRIPTOR_KEY]);
232282
}
233283

234284
public function getErrorOutput(): string
235285
{
286+
if (!is_resource($this->pipes[self::STDERR_DESCRIPTOR_KEY])) {
287+
return '';
288+
}
236289
return stream_get_contents($this->pipes[self::STDERR_DESCRIPTOR_KEY]);
237290
}
238291

‎tests/ApplicationTest.php‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,19 @@ class ApplicationTest extends TestCase
2424

2525
public function setUp(): void
2626
{
27-
file_put_contents(static::$in, '');
28-
file_put_contents(static::$ou, '');
27+
file_put_contents(static::$in, '', LOCK_EX);
28+
file_put_contents(static::$ou, '', LOCK_EX);
2929
}
3030

3131
public function tearDown(): void
3232
{
33-
unlink(static::$in);
34-
unlink(static::$ou);
33+
// Make sure we clean up after ourselves:
34+
if (file_exists(static::$in)) {
35+
unlink(static::$in);
36+
}
37+
if (file_exists(static::$ou)) {
38+
unlink(static::$ou);
39+
}
3540
}
3641

3742
public function test_new()

‎tests/CliTestCase.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function setUp(): void
3636
{
3737
ob_start();
3838
StreamInterceptor::$buffer = '';
39-
file_put_contents(static::$ou, '');
39+
file_put_contents(static::$ou, '', LOCK_EX);
4040
}
4141

4242
public function tearDown(): void
@@ -46,7 +46,10 @@ public function tearDown(): void
4646

4747
public static function tearDownAfterClass(): void
4848
{
49-
unlink(static::$ou);
49+
// Make sure we clean up after ourselves:
50+
if (file_exists(static::$ou)) {
51+
unlink(static::$ou);
52+
}
5053
}
5154

5255
public function buffer()

‎tests/Helper/OutputHelperTest.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,15 @@ class OutputHelperTest extends TestCase
2929

3030
public function setUp(): void
3131
{
32-
file_put_contents(static::$ou, '');
32+
file_put_contents(static::$ou, '', LOCK_EX);
3333
}
3434

3535
public static function tearDownAfterClass(): void
3636
{
37-
unlink(static::$ou);
37+
// Make sure we clean up after ourselves:
38+
if (file_exists(static::$ou)) {
39+
unlink(static::$ou);
40+
}
3841
}
3942

4043
public function test_show_arguments()

‎tests/Helper/ShellTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public function test_get_output()
2424

2525
$shell->execute();
2626

27-
$this->assertSame("hello\n", $shell->getOutput());
27+
$this->assertSame("hello", trim($shell->getOutput())); // trim to remove trailing newline which is OS dependent
2828
$this->assertSame(0, $shell->getExitCode());
2929
}
3030

@@ -35,7 +35,7 @@ public function test_get_process_id()
3535
$shell->execute(true);
3636

3737
$this->assertIsInt($pid = $shell->getProcessId());
38-
$this->assertGreaterThan(getmypid(), $pid);
38+
// $this->assertGreaterThan(getmypid(), $pid); // this is not always true especially on windows
3939
}
4040

4141
public function test_async_stop()

‎tests/IO/InteractorTest.php‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,19 @@ class InteractorTest extends TestCase
2424

2525
public function setUp(): void
2626
{
27-
file_put_contents(static::$in, '');
28-
file_put_contents(static::$ou, '');
27+
file_put_contents(static::$in, '', LOCK_EX);
28+
file_put_contents(static::$ou, '', LOCK_EX);
2929
}
3030

3131
public function tearDown(): void
3232
{
33-
unlink(static::$in);
34-
unlink(static::$ou);
33+
// Make sure we clean up after ourselves:
34+
if (file_exists(static::$in)) {
35+
unlink(static::$in);
36+
}
37+
if (file_exists(static::$ou)) {
38+
unlink(static::$ou);
39+
}
3540
}
3641

3742
public function test_components()
@@ -152,7 +157,7 @@ public function test_call()
152157

153158
protected function newInteractor(string $in = '')
154159
{
155-
file_put_contents(static::$in, $in);
160+
file_put_contents(static::$in, $in, LOCK_EX);
156161

157162
return new Interactor(static::$in, static::$ou);
158163
}

‎tests/Input/ReaderTest.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,15 @@ class ReaderTest extends TestCase
2121

2222
public function setUp(): void
2323
{
24-
file_put_contents(static::$in, '');
24+
file_put_contents(static::$in, '', LOCK_EX);
2525
}
2626

2727
public static function tearDownAfterClass(): void
2828
{
29-
unlink(static::$in);
29+
// Make sure we clean up after ourselves:
30+
if (file_exists(static::$in)) {
31+
unlink(static::$in);
32+
}
3033
}
3134

3235
public function test_default()
@@ -38,7 +41,7 @@ public function test_default()
3841

3942
public function test_callback()
4043
{
41-
file_put_contents(static::$in, 'the value');
44+
file_put_contents(static::$in, 'the value', LOCK_EX);
4245

4346
$r = new Reader(static::$in);
4447

‎tests/Output/ColorTest.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,13 @@ public function test_colors()
5252
{
5353
$c = new Color;
5454

55-
$this->assertSame("\nabc\n", $c->colors('<eol>abc</eol>'));
55+
// We use PHP_EOL here because it is platform dependent and eol tag will be replaced by it.
56+
$this->assertSame(PHP_EOL."abc".PHP_EOL, $c->colors('<eol>abc</eol>'));
5657
$this->assertSame("\033[0;31mRed\033[0m", $c->colors('<red>Red</end>'));
57-
$this->assertSame("\033[1;31mBoldRed\n\033[0m", $c->colors('<boldRed>BoldRed<eol/></end>'));
58-
$this->assertSame("\033[0;36;42mBgGreenCyan\033[0m\n", $c->colors('<bgGreenCyan>BgGreenCyan</end><eol>'));
58+
$this->assertSame("\033[1;31mBoldRed".PHP_EOL."\033[0m", $c->colors('<boldRed>BoldRed<eol/></end>'));
59+
$this->assertSame("\033[0;36;42mBgGreenCyan\033[0m".PHP_EOL, $c->colors('<bgGreenCyan>BgGreenCyan</end><eol>'));
5960
$this->assertSame(
60-
"\033[0;31mRed\033[0m\nNormal\n\033[1;37mBOLD\033[0m",
61+
"\033[0;31mRed\033[0m".PHP_EOL."Normal".PHP_EOL."\033[1;37mBOLD\033[0m",
6162
$c->colors("<red>Red</end>\r\nNormal\n<bold>BOLD</end>")
6263
);
6364
}

0 commit comments

Comments
 (0)