Skip to content

Commit

Permalink
redirect stderr via descriptor spec
Browse files Browse the repository at this point in the history
  • Loading branch information
staabm committed Mar 23, 2024
1 parent 75efa73 commit 26a323b
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 4 deletions.
4 changes: 0 additions & 4 deletions src/Util/PHP/AbstractPhpProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,6 @@ public function getCommand(array $settings, ?string $file = null): array
$command[] = $this->arguments;
}

if ($this->stderrRedirection) {
$command[] = '2>&1';
}

return $command;
}

Expand Down
3 changes: 3 additions & 0 deletions src/Util/PHP/DefaultPhpProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ protected function runProcess(string $job, array $settings): array
1 => $handles[1] ?? ['pipe', 'w'],
2 => $handles[2] ?? ['pipe', 'w'],
];
if ($this->stderrRedirection) {
$pipeSpec[2] = ['redirect', 1];
}

$process = proc_open(
$this->getCommand($settings, $this->tempFile),
Expand Down

6 comments on commit 26a323b

@crrodriguez
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the parent class getCommand method still returns string..so your command is still been run through the shell. it must return an array ['ls', '-l', .......] for the shell to be ommited and in large test suites reduce the memory usage and runtime, depending on the OS from "just noise" to a quite significant speedup. (reported 2x to like 8,10x)

@staabm
Copy link
Owner Author

@staabm staabm commented on 26a323b Mar 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for looking into this.

I guess you did not have a look into my PR but the master branch?
or did I miss something..?

interessting are also the windows errors

@crrodriguez
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not see that, can you point to which windows errors ? that's interesting...

@staabm
Copy link
Owner Author

@staabm staabm commented on 26a323b Mar 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@staabm
Copy link
Owner Author

@staabm staabm commented on 26a323b Mar 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like stderr or stdout is not properly returned back.

@crrodriguez
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like stderr or stdout is not properly returned back.

Is it test specific or all tests are failing in the target ?

Please sign in to comment.