Skip to content

[Process] Fix PhpProcess with phpdbg runtime #16574

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

Merged
merged 1 commit into from
Nov 18, 2015

Conversation

nicolas-grekas
Copy link
Member

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

With this PR, I'm able to successfully run the test suite of the Process component using
phpdbg -qrr ./phpunit src/Symfony/Component/Process/

$this->assertFalse($process->hasBeenSignaled());
}

public function testProcessWithoutTermSignalIsNotSignaled()
Copy link
Member

Choose a reason for hiding this comment

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

why are you removing these tests ?

Copy link
Member Author

Choose a reason for hiding this comment

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

they are exact duplicates if I did not make any mistake. And I moved the only assertion in the below test.

Copy link
Member

Choose a reason for hiding this comment

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

they are not, because of overwrites changing the behavior of some tests based on sigchild being enabled (see child classes). hasBeenSignaled and getTermSignal need to be tested separately to handle these cases properly.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, reverted, thanks for checking

@nicolas-grekas nicolas-grekas force-pushed the phpdbg branch 2 times, most recently from 107fd3e to 7a97093 Compare November 18, 2015 14:13
@@ -49,7 +52,6 @@ if (!file_exists("$PHPUNIT_DIR/phpunit-$PHPUNIT_VERSION/phpunit") || md5_file(__
$zip->close();
chdir("phpunit-$PHPUNIT_VERSION");
passthru("$COMPOSER remove --no-update symfony/yaml");
passthru("$COMPOSER require --no-update phpunit/phpunit-mock-objects \"<=3.0.0\"");
Copy link
Member Author

Choose a reason for hiding this comment

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

phpunit-mock-objects 3.0.4 fixes the issue we had

@nicolas-grekas nicolas-grekas merged commit 9669238 into symfony:2.3 Nov 18, 2015
nicolas-grekas added a commit that referenced this pull request Nov 18, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

[Process] Fix PhpProcess with phpdbg runtime

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

With this PR, I'm able to successfully run the test suite of the Process component using
`phpdbg -qrr ./phpunit src/Symfony/Component/Process/`

Commits
-------

9669238 [Process] Fix PhpProcess with phpdbg runtime
@nicolas-grekas nicolas-grekas deleted the phpdbg branch November 18, 2015 16:32
This was referenced Nov 23, 2015
This was referenced Nov 30, 2015
nicolas-grekas added a commit that referenced this pull request Oct 29, 2023
…ality is enabled (a.dmitryuk)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[Process] remove fixing of legacy bug, when PTS functionality is enabled

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        |
| License       | MIT

The PR drop fixing PHP bug, that was closed in PHP 7.0

Original issues:
php/php-src#1588
#16574

Original commit:
php/php-src@ff6b309

Commits
-------

03590f1 [Process] remove fixing of legacy bug, when PTS functionality is enabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants