[Console] Simplify simulation of user inputs in CommandTester #18710

chalasr · 2016-05-04T22:32:49Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	n/a
License	MIT
Doc PR	symfony/symfony-docs#6623

After @javiereguiluz pointed it in #17470, I open this PR to simplify the simulation of user inputs for testing a Command.
It would be done by calling CommandTester::setUserInputs() with an array of inputs as argument, and so make the CommandTester creating an input stream from the inputs set by the developer, then call QuestionHelper::setInputStream and assign it to the helperSet of the command, sort as all is done automatically in one call.

Depends on #18999

nicolas-grekas · 2016-05-19T08:03:49Z

src/Symfony/Component/Console/Tester/CommandTester.php

+    {
+        $stream = fopen('php://memory', 'r+', false);
+
+        fputs($stream, implode("\n", $this->userInputs));


I think we use fwrite instead of fputs

I tried to follow this http://symfony.com/doc/current/components/console/helpers/questionhelper.html#testing-a-command-that-expects-input as possible, please let me know if you still think that I should use fwrite.

OskarStark · 2016-05-24T13:20:41Z

src/Symfony/Component/Console/Tester/CommandTester.php

@@ -129,4 +137,48 @@ public function getStatusCode()
    {
        return $this->statusCode;
    }
+
+    /**
+     * Sets the faked user inputs.


would remove the word faked

OskarStark · 2016-05-24T13:22:29Z

this should be documented somewhere

thanks for your effort 👍

chalasr · 2016-05-24T13:52:50Z

@OskarStark Thanks for the review.
I am waiting for the opinion of a maintainer before proposing the changes in the doc to make it in one shot.

javiereguiluz · 2016-05-24T13:54:46Z

@chalasr could you please make the test more complex to simulate several inputs in a row? Thanks!

chalasr · 2016-05-24T20:56:17Z

@javiereguiluz Of course, I added a failure test (3 questions for only 2 inputs) and made the first taking 3 questions for 3 inputs.

I also fixed the @OskarStark line notes and started to write documentation.

OskarStark · 2016-05-24T21:44:55Z

I also fixed the @OskarStark line notes and started to write documentation.

Thank you for your effort 👍

chalasr · 2016-05-26T19:30:16Z

According to #17136, this will not work if the SymfonyStyle is used.
I'm working for find a clean solution (out of this PR), but making the SymfonyQuestionHelper out of the command HelperSet made interactive command testing very hard.
In a perfect world, the SymfonyQuestionHelper should be set in the helperSet and be accessible through Command::getHelper('question'). Actually the SymfonyStyle cannot access the Command and the Command cannot access the SymfonyStyle...

EDIT Should be fixed by #18902

chalasr · 2016-05-28T08:10:10Z

Doc submitted.

fabpot · 2016-06-02T06:14:59Z

src/Symfony/Component/Console/Tester/CommandTester.php

+     *
+     * @return CommandTester
+     */
+    public function setUserInputs(array $inputs)


I would have named the method setInputs() instead.

chalasr · 2016-06-02T08:43:38Z

I made the changes and updated the doc.

xabbuh · 2016-06-05T08:49:12Z

src/Symfony/Component/Console/Tester/CommandTester.php

+     *
+     * @param array
+     *
+     * @return CommandTester


Imo this docblock as it is now is quite useless and could be removed as well (input types and the return type can be inferred by IDEs from the method itself) and the description doesn't add any value to the user apart from what the method name already states. Though explaining what $inputs exactly is expected to be would be something that justified its presence.

I added a description.

chalasr · 2016-06-09T20:22:31Z

Rebased this on #18999 to see the benefit by testing commands using both QuestionHelper and SymfonyQuestionHelper (via SymfonyStyle, no need of an HelperSet).
Would be closed otherwise.

fabpot · 2016-06-10T06:30:45Z

src/Symfony/Component/Console/Tester/CommandTester.php

-
-        return $this->statusCode = $this->command->run($this->input, $this->output);
-    }
+     /**


There is an indentation problem here for this method.

Fixed. The changes of this PR are separated in 69dd333

…(chalasr) This PR was merged into the 3.2-dev branch. Discussion ---------- [Console] Centralize input stream in base Input class | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #10844 | License | MIT | Doc PR | not yet Actually we have two classes that have an `inputStream` property with its getter+setter: the QuestionHelper ('question' helper) and the SymfonyQuestionHelper (used via SymfonyStyle, that inherits the inputStream from the QuestionHelper class). This situation makes command testing really hard. ATM we can't test a command that uses `SymfonyStyle::ask` without ugly hacks, and we can't find a generic way to set the inputs passed to a command from the CommandTester, as it need to be done on a specific helper that may not be the one used by the command (SymfonyStyle case). What I propose here is to add a `stream` property (and its getter+setter) to the abstract `Symfony\Component\Console\Input\Input` class. For now I just made the two helpers setting their `inputStream` from `Input::getStream`, as both QuestionHelper and SymfonyQuestionHelper classes have an `ask` method that takes the command Input as argument. In a next time (4.0), we could remove the `getInputStream` and `setInputStream` methods from the QuestionHelper class (this only deprecates them in favor of Input `setStream` and `getStream`). This would close PR #18902 (trying to make interactive command testing with SymfonyStyle easier). This would also make PR #18710 widely better by setting the input stream in a generic way (working for both helpers without caring about if they are used and which one is used). Please give me your opinions. Commits ------- 54d3d63 Centralize input stream in base Input class

fabpot · 2016-06-16T06:27:50Z

@chalasr Now that #18999 is merged, can you rebase this one?

fabpot · 2016-06-16T06:29:00Z

src/Symfony/Component/Console/Tester/CommandTester.php

+        return $this;
+    }
+
+    private function getInputStream()


I would name it createInputStream() and pass $this->inputs as an argument.

I would then rename it to createStream()

fabpot · 2016-06-16T06:29:58Z

Just noticed that the second commit messages contain all messages from squashed commits, can you remove them as they are useless? Thanks.

chalasr · 2016-06-16T07:35:30Z

Made the changes, rebased and cleaned the commit message.

Rename getInputStream() to getStream(), taking inputs as argument Make createStream static, typehint as array

chalasr · 2016-06-16T07:44:58Z

@fabpot As it takes the inputs as argument, I made the getStream() method static.

fabpot · 2016-06-16T12:49:38Z

src/Symfony/Component/Console/Tester/CommandTester.php

+        return $this;
+    }
+
+    private static function createStream(array $inputs)


What's the benefit of marking it public? I would have kept it private.

@fabpot : Did you mean static ?
I guess it makes sense, as this method looks like a utility one.

No, I meant public. I think it should be private, I don't see why an end user would have to use this.

@fabpot : But the createStream method is actually private. Not public. 😕

lol, sorry about the confusion.

fabpot · 2016-06-16T15:00:45Z

Thank you @chalasr.

@javiereguiluz

…dTester (chalasr) This PR was merged into the 3.2-dev branch. Discussion ---------- [Console] Simplify simulation of user inputs in CommandTester | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | symfony/symfony-docs#6623 After @javiereguiluz pointed it in #17470, I open this PR to simplify the simulation of user inputs for testing a Command. It would be done by calling `CommandTester::setUserInputs()` with an array of inputs as argument, and so make the CommandTester creating an input stream from the inputs set by the developer, then call `QuestionHelper::setInputStream` and assign it to the helperSet of the command, sort as all is done automatically in one call. Depends on #18999 Commits ------- c7ba38a [Console] Set user inputs from CommandTester

…ing user inputs (chalasr) This PR was merged into the master branch. Discussion ---------- [Console] Adapt doc for easier testing of commands needing user inputs Doc-PR for symfony/symfony#18710. This one eases testing of commands needing user interactions by simulating them from the CommandTester directly, by creating a input stream from the ones set by the developer. Commits ------- 26fdbe0 [Console] Adapt doc for easier testing of commands needing user inputs

carsonbot added the Status: Needs Review label May 4, 2016

chalasr changed the title ~~[Console] Simplify user inputs simulation in the CommandTester~~ [Console] Simplify simulation of user inputs in CommandTester May 4, 2016

javiereguiluz added Feature Console labels May 5, 2016

chalasr force-pushed the patch_commandtester branch 4 times, most recently from ad967f7 to 63fbfd1 Compare May 5, 2016 18:42

nicolas-grekas reviewed May 19, 2016
View reviewed changes

chalasr force-pushed the patch_commandtester branch from 63fbfd1 to 07c7fb1 Compare May 19, 2016 09:53

OskarStark reviewed May 24, 2016
View reviewed changes

chalasr force-pushed the patch_commandtester branch 2 times, most recently from daab5d1 to e0be96f Compare May 24, 2016 20:25

chalasr force-pushed the patch_commandtester branch from e0be96f to 9b5a645 Compare May 24, 2016 21:22

This was referenced May 27, 2016

[Console] Add SymfonyStyle::setInputStream() to ease command testing #18902

Closed

[Console] Adapt doc for easier testing of commands needing user inputs symfony/symfony-docs#6623

Merged

fabpot reviewed Jun 2, 2016
View reviewed changes

xabbuh reviewed Jun 5, 2016
View reviewed changes

chalasr force-pushed the patch_commandtester branch 6 times, most recently from b4768e0 to 4c39bc8 Compare June 9, 2016 19:59

chalasr force-pushed the patch_commandtester branch 2 times, most recently from 438ca34 to 0e64796 Compare June 9, 2016 22:46

fabpot reviewed Jun 10, 2016
View reviewed changes

chalasr force-pushed the patch_commandtester branch from 0e64796 to 69dd333 Compare June 10, 2016 07:52

fabpot mentioned this pull request Jun 15, 2016

Added getter and setter of QuestionHelper to SymfonyStyle #17136

Closed

fabpot reviewed Jun 16, 2016
View reviewed changes

chalasr force-pushed the patch_commandtester branch 3 times, most recently from 769312f to b748435 Compare June 16, 2016 07:32

[Console] Set user inputs from CommandTester

c7ba38a

Rename getInputStream() to getStream(), taking inputs as argument Make createStream static, typehint as array

chalasr force-pushed the patch_commandtester branch from b748435 to c7ba38a Compare June 16, 2016 07:44

fabpot reviewed Jun 16, 2016
View reviewed changes

fabpot merged commit c7ba38a into symfony:master Jun 16, 2016

chalasr deleted the patch_commandtester branch June 16, 2016 16:28

fabpot mentioned this pull request Oct 27, 2016

Release v3.2.0-BETA1 #20317

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Console] Simplify simulation of user inputs in CommandTester #18710

[Console] Simplify simulation of user inputs in CommandTester #18710

chalasr commented May 4, 2016 •

edited

Loading

nicolas-grekas May 19, 2016

chalasr May 19, 2016

OskarStark May 24, 2016

OskarStark commented May 24, 2016

chalasr commented May 24, 2016

javiereguiluz commented May 24, 2016

chalasr commented May 24, 2016 •

edited

Loading

OskarStark commented May 24, 2016

chalasr commented May 26, 2016 •

edited

Loading

chalasr commented May 28, 2016

fabpot Jun 2, 2016

chalasr commented Jun 2, 2016

xabbuh Jun 5, 2016

chalasr Jun 5, 2016

chalasr commented Jun 9, 2016 •

edited

Loading

fabpot Jun 10, 2016

chalasr Jun 10, 2016

fabpot commented Jun 16, 2016

fabpot Jun 16, 2016

fabpot Jun 16, 2016

fabpot commented Jun 16, 2016

chalasr commented Jun 16, 2016

chalasr commented Jun 16, 2016

fabpot Jun 16, 2016

ogizanagi Jun 16, 2016

fabpot Jun 16, 2016

ogizanagi Jun 16, 2016

fabpot Jun 16, 2016

fabpot commented Jun 16, 2016

[Console] Simplify simulation of user inputs in CommandTester #18710

[Console] Simplify simulation of user inputs in CommandTester #18710

Conversation

chalasr commented May 4, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OskarStark commented May 24, 2016

chalasr commented May 24, 2016

javiereguiluz commented May 24, 2016

chalasr commented May 24, 2016 • edited Loading

OskarStark commented May 24, 2016

chalasr commented May 26, 2016 • edited Loading

chalasr commented May 28, 2016

Choose a reason for hiding this comment

chalasr commented Jun 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalasr commented Jun 9, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Jun 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Jun 16, 2016

chalasr commented Jun 16, 2016

chalasr commented Jun 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Jun 16, 2016

chalasr commented May 4, 2016 •

edited

Loading

chalasr commented May 24, 2016 •

edited

Loading

chalasr commented May 26, 2016 •

edited

Loading

chalasr commented Jun 9, 2016 •

edited

Loading