8000 [Process] Make test AbstractProcessTest::testStartAfterATimeout useful again by ymc-dabe · Pull Request #13446 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Process] Make test AbstractProcessTest::testStartAfterATimeout useful again #13446

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

Conversation

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

The test AbstractProcessTest::testStartAfterATimeout() is pretty useless, due to two reasons:

  1. Any exception is caught
    This means even the exception thrown with
    $this->fail('A RuntimeException should have been raised.');
    is caught, making the test pretty useless.
  2. Invalid PHP code gets executed
    The command that is executed in the tests actually is:
    # php -r "$n = 1000; while ($n--) {echo ''; usleep(1000); }"
    .
    This does not wait ~1s, but produces the following error:
    PHP Parse error: syntax error, unexpected '=' in Command line code on line 1

…meout()

Without this change any \Exception is caught in this test, especially even
the one thrown with:
$this->fail('A RuntimeException should have been raised.');

Thus catching any exception in this tests makes it pretty useless.
@ymc-dabe ymc-dabe changed the title Only catch RuntimeException in AbstractProcessTest::testStartAfterATimeo... [WIP] [Process] Only catch RuntimeException in AbstractProcessTest::testStartAfterATimeo... Jan 19, 2015
@ymc-dabe ymc-dabe force-pushed the Process-AbstractProcessTest--testStartAfterATimeout-is-pretty-useless branch from 48378be to 6b45999 Compare January 19, 2015 14:22
…meout()

Previously to this commit the command executed in the tests actually was:
  # php -r "$n = 1000; while ($n--) {echo ''; usleep(1000); }"

This does not (as intended) wait ~1s, but produces the following error:
  PHP Parse error: syntax error, unexpected '=' in Command line code on line 1
@ymc-dabe ymc-dabe force-pushed the Process-AbstractProcessTest--testStartAfterATimeout-is-pretty-useless branch from 6b45999 to 675bc34 Compare January 19, 2015 14:34
@ymc-dabe ymc-dabe changed the title [WIP] [Process] Only catch RuntimeException in AbstractProcessTest::testStartAfterATimeo... [Process] Only catch RuntimeException in AbstractProcessTest::testStartAfterATimeo... Jan 19, 2015
@ymc-dabe ymc-dabe changed the title [Process] Only catch RuntimeException in AbstractProcessTest::testStartAfterATimeo... [Process] Make test AbstractProcessTest::testStartAfterATimeout useful again Jan 19, 2015
@fabpot
Copy link
Member
fabpot commented Feb 4, 2015

Thank you @ymc-dabe.

fabpot added a commit that referenced this pull request Feb 4, 2015
…Timeout useful again (ymc-dabe)

This PR was squashed before being merged into the 2.3 branch (closes #13446).

Discussion
----------

[Process] Make test AbstractProcessTest::testStartAfterATimeout useful again

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

The test AbstractProcessTest::testStartAfterATimeout() is pretty useless, due to two reasons:

1. Any exception is caught
This means even the exception thrown with
<code>$this->fail('A RuntimeException should have been raised.');</code>
is caught, making the test pretty useless.

2. Invalid PHP code gets executed
The command that is executed in the tests actually is:
<code># php -r "$n = 1000; while ($n--) {echo ''; usleep(1000); }"</code>
.
This does not wait ~1s, but produces the following error:
<code>PHP Parse error:  syntax error, unexpected '=' in Command line code on line 1</code>

Commits
-------

1be266f [Process] Make test AbstractProcessTest::testStartAfterATimeout useful again
@fabpot fabpot closed this Feb 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0