Skip to content
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

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

Merged
merged 1 commit into from Oct 1, 2018

Conversation

Projects
None yet
5 participants
@duncan3dc
Copy link
Contributor

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

stof approved these changes Oct 1, 2018

@stof

This comment has been minimized.

Copy link
Member

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

chalasr approved these changes Oct 1, 2018

@chalasr

This comment has been minimized.

Copy link
Member

commented Oct 1, 2018

Thank you @duncan3dc.

@chalasr chalasr merged commit b6c17df into symfony:3.4 Oct 1, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

chalasr added a commit that referenced this pull request Oct 1, 2018

bug #28664 [Console] Don't return early as this bypasses the auto exi…
…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 duncan3dc:fix-exit-code branch Oct 1, 2018

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Oct 3, 2018

This was referenced Oct 3, 2018

@m7moud m7moud referenced this pull request Jun 14, 2019

Open

EmptyAllCachesConsole doesn't return correct Error_Code #255

1 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.