Skip to content

[Process] Process unit tests are failing on windows #8799

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.

Sign up for GitHub

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

Jump to bottom
Closed
romainneutron opened this issue Aug 20, 2013 · 4 comments
Closed

[Process] Process unit tests are failing on windows #8799

romainneutron opened this issue Aug 20, 2013 · 4 comments
Labels
Process

Comments

@romainneutron
Copy link
Contributor

It seems that Process::start hangs on windows since #8740
@romainneutron
Copy link
Contributor Author

Edit PR description, finally not related to #8740 - some unit tests on windows are failing for a long time ; same failures reproduced on 2.3.0, 2.3.1, 2.3.2.

For example : SimpleProcessTest::testRunProcessWithTimeout.

@romainneutron romainneutron mentioned this issue Aug 23, 2013
Closed
@maryo
Copy link
Contributor

maryo commented Aug 23, 2013

SimpleProcessTest::testRunProcessWithTimeout fails because of

AbstractProcessTest::testRunProcessWithTimeout()

$process = $this->getProcess('sleep 3')

There is no sleep 3 command on Windows. It should be probably $process = $this->getProcess('php -r "sleep(3);"'); as it is on more places.

But there are more tests failing like SimpleProcessTest::testIsNotSuccessful probably because it waits 4 seconds so $process->isRunning() is false then because the command is probably finished.

Next is SigchildDisabledProcessTest::testCallbacksAreExecutedWithStart

And then Symfony\Component\Process\Tests\SigchildDisabledProcessTest::testProcessResponses
It is the one related with #8836 (comment)

which was red and is green on my pc with the hotfix... But there are still many other failures.

@romainneutron
Copy link
Contributor Author

Thanks @maryo for this report, I'll check this

@romainneutron
Copy link
Contributor Author

@fabpot please label this issue as a process issue

@romainneutron romainneutron mentioned this issue Sep 3, 2013
Closed
fabpot added a commit that referenced this issue Sep 7, 2013
@fabpot
merged branch romainneutron/process-windows (PR #8924)
2dead87 
This PR was squashed before being merged into the 2.2 branch (closes #8924).

Discussion
----------

[Process][2.2] Fix Process component on windows

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #8836, #8799, #7078
| License       | MIT

This PR fixes Process on windows (almost, see note below).
 - Some unit tests were not Windows compatible
 - Use a file handle for STDERR as well as STDOUT to avoid blocking
 - Decouple pipes and descriptors from Process

As this move some a part of Process in a sub class, I hope merging this in 2.3 and master would not be a PITA. I'm here to make some adjustments after theses merge if needed.

**Important note** :

We are using file handles instead of streams for `proc_open` pipes as described in the code (see [PHP bug #51800](https://bugs.php.net/bug.php?id=51800)). Unfortunately, this workaround may produce corrupted output/error output in some race conditions. That's why `AbstractProcessTest::testProcessPipes` randomly fails when using file handles (on unix and windows).

Commits
-------

4a76c76 [Process][2.2] Fix Process component on windows
@fabpot fabpot closed this as completed Sep 7, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Process
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@fabpot @romainneutron @maryo