[Console] add setInputs to ApplicationTester and share some code #24819

Simperfit · 2017-11-04T18:11:15Z

Q	A
Branch?	4.1
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#24784
License	MIT
Doc PR	todo

I didn't implemented the tests because I don't know how to write them on ApplicationTester.

chalasr

here is a test case

diff --git a/src/Symfony/Component/Console/Tests/Tester/ApplicationTesterTest.php b/src/Symfony/Component/Console/Tests/Tester/ApplicationTesterTest.php
index 062d414..0cc002d 100644
--- a/src/Symfony/Component/Console/Tests/Tester/ApplicationTesterTest.php
+++ b/src/Symfony/Component/Console/Tests/Tester/ApplicationTesterTest.php
@@ -13,7 +13,9 @@ namespace Symfony\Component\Console\Tests\Tester;
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Console\Application;
+use Symfony\Component\Console\Helper\QuestionHelper;
 use Symfony\Component\Console\Output\Output;
+use Symfony\Component\Console\Question\Question;
 use Symfony\Component\Console\Tester\ApplicationTester;
 
 class ApplicationTesterTest extends TestCase
@@ -65,6 +67,22 @@ class ApplicationTesterTest extends TestCase
         $this->assertEquals('foo'.PHP_EOL, $this->tester->getDisplay(), '->getDisplay() returns the display of the last execution');
     }
 
+    public function testSetInputs()
+    {
+        $this->application->register('foo')->setCode(function ($input, $output) {
+            $helper = new QuestionHelper();
+            $helper->ask($input, $output, new Question('Q1'));
+            $helper->ask($input, $output, new Question('Q2'));
+            $helper->ask($input, $output, new Question('Q3'));
+        });
+
+        $this->tester->setInputs(array('I1', 'I2', 'I3'));
+        $this->tester->run(array('command' => 'foo'));
+
+        $this->assertSame(0, $this->tester->getStatusCode());
+        $this->assertEquals('Q1Q2Q3', $this->tester->getDisplay(true));
+    }
+
     public function testGetStatusCode()
     {
         $this->assertSame(0, $this->tester->getStatusCode(), '->getStatusCode() returns the status code');

chalasr · 2017-11-05T12:14:43Z

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

+     *
+     * @return self
+     */
+    public function setInputs(array $inputs)


For this to work in ApplicationTester, the tester needs to set the input stream from this value.
So CommandTester::createInputStream() should be shared between both testers (that's my motivation for a trait) and this line should be added to ApplicationTester::run().

chalasr · 2017-11-05T12:15:11Z

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

+     */
+    public function setInputs(array $inputs)
+    {
+        $this->inputs = $inputs;


the property does not exist in ApplicationTester, it should be created

or moved to the trait along with:

/** @var StreamOutput */ private $output;

Simperfit · 2017-11-05T16:32:33Z

@chalasr So it seems that the test is failling. Is here something the modify directly in the Application to make this work properly ?

chalasr · 2017-11-05T18:12:37Z

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

+ */
+trait TesterTrait
+{
+    /** @var StreamOutput */


not useful and differs with getOutput() return type, I would drop it

To me it is useful actually (for autocompletion and SCA). This trait needs a StreamOutput instance here, not any OutputInterface. That's why I suggested this docblock in #24819 (comment) :)

chalasr · 2017-11-05T18:23:38Z

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

        if (isset($options['interactive'])) {
            $this->input->setInteractive($options['interactive']);
        }

+        if ($this->inputs) {
+            $this->input->setStream(self::createStream($this->inputs));


Here is the failure culprit, I suggest to putenv('SHELL_INTERACTIVE=1') here and, after run(), either destroy it or restore its previous value if any

Simperfit · 2017-11-05T19:47:36Z

Thanks for the review guys, tests are now green ;).

chalasr · 2017-11-05T19:49:51Z

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

+    private $inputs = array();
+
+    /**
+     * Gets the display returned by the last execution of the application.


command or application, same below

chalasr · 2017-11-05T19:50:52Z

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

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


this one must stay private

chalasr · 2017-11-05T20:00:52Z

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

-        rewind($this->output->getStream());
-
-        $display = stream_get_contents($this->output->getStream());
+        putenv('SHELL_INTERACTIVE');


putenv($shellInteractive ? "SHELL_INTERACTIVE=$shellInteractive" : 'SHELL_INTERACTIVE'); instead, removing the condition below

chalasr

👍

ogizanagi · 2017-11-06T09:03:56Z

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

@@ -62,10 +58,18 @@ public function __construct(Application $application)
    public function run(array $input, $options = array())
    {
        $this->input = new ArrayInput($input);
+


nitpicking but should be reverted ^^

chalasr · 2017-11-26T01:09:40Z

Thank you @Simperfit.

… some code (Simperfit) This PR was merged into the 4.1-dev branch. Discussion ---------- [Console] add setInputs to ApplicationTester and share some code | Q | A | ------------- | --- | Branch? | 4.1 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #24784 | License | MIT | Doc PR | todo I didn't implemented the tests because I don't know how to write them on ApplicationTester. Commits ------- ea86ed8 [Console] add setInputs to ApplicationTest and share some code

carsonbot added Status: Needs Review Console Feature labels Nov 4, 2017

Simperfit force-pushed the feature/refactor-application-and-command-tester-to-share-code branch from be6fbc2 to dbbf3fb Compare November 4, 2017 18:11

Simperfit changed the title ~~[Console] add setInputs to ApplicationTest and share some code~~ [Console] add setInputs to ApplicationTester and share some code Nov 5, 2017

ogizanagi added this to the 4.1 milestone Nov 5, 2017

chalasr requested changes Nov 5, 2017

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Nov 5, 2017

Simperfit force-pushed the feature/refactor-application-and-command-tester-to-share-code branch 2 times, most recently from 370851e to efe61cc Compare November 5, 2017 16:38

chalasr reviewed Nov 5, 2017

View reviewed changes

Simperfit force-pushed the feature/refactor-application-and-command-tester-to-share-code branch from efe61cc to 8b59c06 Compare November 5, 2017 19:44

chalasr reviewed Nov 5, 2017

View reviewed changes

Simperfit force-pushed the feature/refactor-application-and-command-tester-to-share-code branch from 8b59c06 to 3513236 Compare November 5, 2017 20:20

chalasr approved these changes Nov 5, 2017

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Nov 5, 2017

ogizanagi approved these changes Nov 6, 2017

View reviewed changes

20uf approved these changes Nov 11, 2017

View reviewed changes

[Console] add setInputs to ApplicationTest and share some code

ea86ed8

Simperfit force-pushed the feature/refactor-application-and-command-tester-to-share-code branch from 3513236 to ea86ed8 Compare November 12, 2017 06:57

chalasr merged commit ea86ed8 into symfony:master Nov 26, 2017

Simperfit deleted the feature/refactor-application-and-command-tester-to-share-code branch December 20, 2017 15:42

fabpot mentioned this pull request May 7, 2018

Release v4.1.0-BETA1 #27181

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Console] add setInputs to ApplicationTester and share some code #24819

[Console] add setInputs to ApplicationTester and share some code #24819

Simperfit commented Nov 4, 2017

chalasr left a comment

chalasr Nov 5, 2017

chalasr Nov 5, 2017

ogizanagi Nov 5, 2017 •

edited

Loading

Simperfit commented Nov 5, 2017

chalasr Nov 5, 2017

ogizanagi Nov 5, 2017 •

edited

Loading

chalasr Nov 5, 2017

Simperfit commented Nov 5, 2017

chalasr Nov 5, 2017

chalasr Nov 5, 2017

chalasr Nov 5, 2017

chalasr left a comment

ogizanagi Nov 6, 2017

chalasr commented Nov 26, 2017

[Console] add setInputs to ApplicationTester and share some code #24819

[Console] add setInputs to ApplicationTester and share some code #24819

Conversation

Simperfit commented Nov 4, 2017

chalasr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogizanagi Nov 5, 2017 • edited Loading

Choose a reason for hiding this comment

Simperfit commented Nov 5, 2017

Choose a reason for hiding this comment

ogizanagi Nov 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Simperfit commented Nov 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalasr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalasr commented Nov 26, 2017

ogizanagi Nov 5, 2017 •

edited

Loading

ogizanagi Nov 5, 2017 •

edited

Loading