Skip to content

[Console] Don't return early as this bypasses the auto exit feature #28664

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
Oct 1, 2018

Conversation

duncan3dc
Copy link
Contributor

@duncan3dc duncan3dc commented Oct 1, 2018

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? no
Fixed tickets #28666
License MIT

It looks like 8805cfd broke the auto exit feature by returning early.

I couldn't find any tests for this feature (presumably because it uses exit()), I was going to write one using uopz but didn't want to do this work if Symfony intentionally doesn't test the call to exit()

@stof
Copy link
Member

stof commented Oct 1, 2018

Testing this case would indeed be harder. A way could be to run a subprocess running the Application class and checking the exit code of the command (if the call to exit is not performed, it would exit with 0 rather than the expected one)

@chalasr
Copy link
Member

chalasr commented Oct 1, 2018

Thank you @duncan3dc.

@chalasr chalasr merged commit b6c17df into symfony:3.4 Oct 1, 2018
chalasr pushed a commit that referenced this pull request Oct 1, 2018
…t feature (duncan3dc)

This PR was merged into the 3.4 branch.

Discussion
----------

[Console] Don't return early as this bypasses the auto exit feature

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

It looks like 8805cfd broke the auto exit feature by returning early.

I couldn't find any tests for this feature (presumably because it uses `exit()`), I was going to write one using uopz but didn't want to do this work if Symfony intentionally doesn't test the call to `exit()`

Commits
-------

b6c17df Don't return early as this bypasses the auto exit feature
@duncan3dc duncan3dc deleted the fix-exit-code branch October 1, 2018 16:09
@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Oct 3, 2018
This was referenced Oct 3, 2018
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.

5 participants