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

Update Input- and OutputInterface on cached Manager #1056

Merged
merged 1 commit into from
Mar 18, 2017

Conversation

moritz-h
Copy link
Contributor

Fix: #960
The commands get a fresh InputInterface and OutputInterface Instance for each time, the command runs. But the commands cache the Manager instance, which gets a reference to the Input/OutputInterface on construction. So when the command runs a second time, they get fresh Input/OutputInterfaces but uses the cached Manager Instance with reference to old Input/OutputInterface Instances.
This PR updates the Input and OutputInterface of the cached Manager to the Input/OutputInterface of the current command run. (See #960 comment for more details)

@moritz-h moritz-h changed the title update input and output on cached manager Update Input- and OutputInterface on cached Manager Mar 14, 2017
@rquadling
Copy link
Collaborator

That makes sense to me. Can you write a test that proves getting the manager gets the injected Input/Output.

@moritz-h
Copy link
Contributor Author

moritz-h commented Mar 16, 2017

I thougth of writing a test for this and read througth your tests. I wanted to start here:
https://github.com/robmorgan/phinx/blob/v0.8.0/tests/Phinx/Console/Command/StatusTest.php

I saw you are using a manager stub in the tests and my idea was to test somehow if the stub gets the new input/output Interfaces on command execution. But i don't find any code where you are testing the bootstrap part of a command. So i had no startpoint where i could add these tests.
If you can give me a hint where i could add these tests, i will try to write some.

@rquadling rquadling self-assigned this Mar 16, 2017
@rquadling rquadling merged commit 1b71e03 into cakephp:master Mar 18, 2017
@piotr-cz
Copy link
Contributor

piotr-cz commented Jun 2, 2017

Thanks, this fixed the issue

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.

3 participants