Recommendations for automated testing of Robo Scripts/Commands?

[defunctl]

Hey Greg,

What's the best way to go about testing custom Robo Scripts, or anything extending \Robo\Tasks?

For example, say I wanted to assert the foo method said This is a Robo script, $name in https://github.com/consolidation/Robo/blob/master/examples/robo.script ?

Or do any unit/functional tests exist that I can look at that test https://github.com/consolidation/Robo/blob/master/examples/src/Robo/Plugin/Commands/ExampleCommands.php ?

Thanks!

[greg-1-anderson]

See g1a/starter. If you follow the composer create-project instructions in that repo, you will end up with your own Robo-based standalone commandline tool with tests. Port your own scripts into your new project, and expand the existing tests to cover your custom functionality. Profit.

[lpeabody]

@greg-1-anderson How would you go about testing a series of sequential input to interactive commands? My research so far has led me to https://symfony.com/doc/current/components/console/helpers/questionhelper.html#testing-a-command-that-expects-input.

How would you approach using that with Robo in PHPUnit tests?

[greg-1-anderson]

CommandTester looks useful. I have been ambivalent about exposing the $command objects in Robo; see #675

With some work, it would be possible to modify the CommandTesterTrait to work with the Symfony CommandTester. What we'd want to do is replace the $runner->execute() with something that returned the $command object. Then the unit test could use CommandTester directly.

[lpeabody]

I apologize for the very long post in advance!

I've played around with this a bit, lots of hacking so far so no code to officially post as yet. For the most part I've been able to figure out a decent mechanism for extracting the command using the patch from #675

Input works fine so long as you put all interactive questions in an interact hook. I'm able to successfully capture the input and set options/arguments, and these get used in the primary command callback.

However, I am running into a bit of a pain-point - extracting the output generated by the execution of the command. CommandTester creates its own input and output streams via fopen('php://memory'), and Runner creates its own streams to be used by annotated commands IO. The interact hook receives the output streams created by the CommandTester, but the main command callback uses the output stream created by the Runner. So, when I try to extract the output in the test via CommandTester::getDisplay(), I don't get any output that was created in the command callback.

Snippet of custom Robo command class GitCommand:

    /**
     * @command git:interactive
     */
    public function gitInteractive($answer) {
        $output = $this->output(); // Uses output stream created by Runner.
        $this->io()->text("The answer is $answer.");
    }

    /**
     * @hook interact git:interactive
     */
    public function gitInteractiveInteract(InputInterface $input, OutputInterface $output, AnnotationData $annotationData) {
        // $output here is injected via CommandTester.
        $io = new SymfonyStyle($input, $output);
        $answer = $io->ask("what is the answer? ");
        $input->setArgument('answer', $answer);
    }

Snippet of test case:

class CommandInteractiveTest extends TestCase {

    use CommandTesterTrait;

    public function testThis() {
        $this->setupCommandTester('Release Manager', '1.0.0-alpha1');
        /** @var Command $command */
        list($command) = $this->getTestCommand(['Release Manager', 'git:interactive'], 'Release Manager');
        $tester = new CommandTester($command);
        $tester->setInputs(['42']);
        $status = $tester->execute(['command' => $command->getName()], ['capture_stderr_separately' => true]);
        $display = $tester->getDisplay();
        $this->assertEquals(0, $status);
        // The below fails because the output stream used by the command
        // callback differs from the output stream instantiated by
        // CommandTester.
        $this->assertStringContainsString('The answer is 42.', $display);
    }
}

The disconnect between output streams seems to come into play on line 257 of CommandProcessor, where the command callback function is invoked.

    /**
     * Run the main command callback
     */
    protected function runCommandCallback($commandCallback, CommandData $commandData)
    {
        $result = false;
        try {
            $args = $this->parameterInjection()->args($commandData);
            $result = call_user_func_array($commandCallback, $args);
        } catch (\Exception $e) {
            $result = $this->commandErrorForException($e);
        }
        return $result;
    }

At this point, the output already assigned to GitCommand::$output will be used no matter what, even though the output stream pointed to by $commandData->output() is the one provided by CommandTester.

I'm brand new to this framework, so I'll just ask a few follow up questions:

1. Is the disconnect I noted above a bug? Should it be possible for the output stream pointed at by $commandData to be different from what is assigned to the GitCommand class? My sense is no, it should not be different and that it in fact is a bug.

2. If it's a bug, what would be the most ideal path forward? Prior to invoking the callback, ensure that the outputs are aligned? Updating CommandProcessor::runCommandCallback like so seems to do the trick:

   /**
    * Run the main command callback
    */
   protected function runCommandCallback($commandCallback, CommandData $commandData)
   {
       $result = false;
       try {
           $args = $this->parameterInjection()->args($commandData);
           // Ensure output streams are aligned.
           $commandCallback[0]->setOutput($commandData->output());
           $result = call_user_func_array($commandCallback, $args);
       } catch (\Exception $e) {
           $result = $this->commandErrorForException($e);
       }
       return $result;
   }
   

   However, I am unsure if this could result in any unintended side effects.

3. If that is not an appropriate place to address this, where else might we attack this issue of output stream alignment? Should Robo extend CommandTester and alter CommandTester so that it is capable of having its output stream explicitly set so that CommandTester aligns with Robo's output rather than Robo having to align with CommandTester? As of this moment, CommandTester does not permit you to explicitly set the output stream, it demands to be used with php://memory.

Ultimately, I like that CommandTester shoves everything into a memory stream, it keeps the PHPUnit console output clean. My preference would be to move in the direction of ensuring that Robo and annotated commands use the streams created by CommandTester.

I'll work on getting some example code pushed up, probably tomorrow at this point.

[greg-1-anderson]

Sorry I only read the first bit of that. My suggestion was:

replace the $runner->execute() with something that returned the $command object.

So, don't worry about what Runner does with the output stream, because we wouldn't use that. Just make a helper that returns the Command object, and then use the Symfony CommandTester as it was intended to be used.

I think it's weird that CommandTester uses fopen('php://memory') to capture output, but no matter, that works.

[lpeabody]

Sorry for the confusion, I did actually implement the return of the Symfony command instance https://bitbucket.org/lpeabody/release-manager/src/8108b6ccc1446388f48e80288c58a0be29e0c301/tests/CommandTesterTrait.php?at=feature%2Ftest-interactive-commands#lines-83

And the update I made to Runner https://github.com/lpeabody/Robo/blob/e1a2af011b2dfec3f9b80495c21beefae9b3dd85/src/Runner.php#L166-L177.

[greg-1-anderson]

So what you want is a new method of Runner that does:

        $app = Robo::createDefaultApplication($appName, $appVersion);
        $commandFiles = $this->getRoboFileCommands($output); // $output is just used for printing error messages, it can be a throwaway stream
        $container = Robo::createDefaultContainer($input, $output, $app, $config);
        $this->registerCommandClasses($app, $commandFiles);

        return $app;

The general idea is that you should get back the Symfony application object initialized and ready to go. Then you can use it to look up the $command object and use it with the CommandTester.

The wrinkles are:

• $input and $output are cached in the container. Output will be used by the logger. Not sure if $input is used, but this could make the code untestable with CommandTester if it is.
• You'll need to make a Config object and put whatever values are appropriate for testing in it.

It is an aspirational goal to remove $input and $output from the container, but that's a Robo 3 thing. Again, I'm not sure if this will get in your way or not. It might depend on what your command does.

[greg-1-anderson]

OK it seems like you are basically doing what I recommended. One nit is that your new method shouldn't be called getCommand(), because that looks like something someone might want to use e.g. for one command to reference another. Something like createCommandForTesting(), perhpas.

Anyway, the $this->io() method is going to mess you up any place where it's used. Getting $input and $output out of the DI container will help for this. The thing is tho that you'd need to re-inject the $input and $output objects from the CommandTester into the command file instance. I'm not sure if this is easy / practical.

[greg-1-anderson]

I am working on making this better. I am doing some work in consolidaiton/annotated-command to insure that the $input and $output objects used with the Symfony command object are injected into the Robo command instance iff it is input aware / output aware. Robo files are supposed to extend \Robo\Tasks; any that do will be input and output aware. Those that do not should use IO. The task builder already injects the $input and $output objects from the Robo command instance into each task that is created (for any task that is input / output aware).

This should fix the problem you're having with $this->io() and make it possible to use CommandTester.

[lpeabody]

That would be a huge win for testing fully interactive applications, really appreciate you digging in on this. If you push something up I'd be happy to test it out.

[greg-1-anderson]

consolidation/annotated-command#207 is the consolidation/annotated-command PR. It still needs tests, and Robo still needs changes to make this PR work correctly. Work in progress.

[greg-1-anderson]

Here's the Robo PR:

#966

Does not do anything yet except this should allow $this->io() to work correctly in your situation.

[greg-1-anderson]

The existing PRs are not rigorous enough yet to work correctly e.g. with most hooks. Still working on improving this.

[lpeabody]

The quest to enable robust testing methods is always a worthy pursuit. I did not have an opportunity yesterday to get to testing your modifications, unfortunately work got in the way. Sometimes I just wish I could contribute to open source as my job...

Per your last comment, are your thoughts that the work done so far permits CommandTester to be used in a way that gives the expected results for my usecase (e.g. using the same input/output streams in the interact hook + command callback)? You're saying that the input/output streams aren't consolidated across all command hooks yet (validate, initialize, option, etc..)?

[greg-1-anderson]

I haven't tried to use CommandTester yet. I'm still fixing bugs with the Robo integration of that PR as I find them.

[lpeabody]

Success!

Setup

Command and interact hook being tested:
https://bitbucket.org/lpeabody/release-manager/src/a9ea89973886b67012b34fe85770eaf7b8779024/src/Commands/GitCommand.php?at=feature%2Frobo-966-g1a-updates#lines-61:75

Robo patch from #675 + some custom work to return a command from a built container:
https://github.com/consolidation/Robo/compare/main...lpeabody:feature/947-modifications-for-interactive-testing.patch

Trait for execute command via CommandTester:
https://bitbucket.org/lpeabody/release-manager/src/a9ea89973886b67012b34fe85770eaf7b8779024/tests/CommandTesterTrait.php?at=feature%2Frobo-966-g1a-updates#lines-105:114.

Test invoking CommandTesterTrait::executeCommandNativeTester:
https://bitbucket.org/lpeabody/release-manager/src/a9ea89973886b67012b34fe85770eaf7b8779024/tests/Functional/CommandInteractiveTest.php?at=feature%2Frobo-966-g1a-updates#lines-17

Result

Test passes using native CommandTester trait. I confirmed the output and input streams used in both the command function and its interact hook function were the ones provided by CommandTester.

[greg-1-anderson]

I haven't looked at your changes yet, and I haven't written any tests of my own to prove it yet either, but I believe that #675 basically works for what it was intended to do -- re-inject $input and $output as provided to methods such as Command::execute(). I pushed a couple of minor fixes after you did ^^.

[greg-1-anderson]

I just skimmed your Robo modifications, and they look good in concept. I think that what we need in terms of an API is a simple method (in Runner or elsewhere) to initialize the Robo DI container and return the Symfony Application object. Then the test can call $app->get() directly to recover the $command object.

I'm still not sure that there is any need to return the commandfile instance itself directly. If folks call methods of the commandfile, then they subvert hooks and other processing. I think direct calls to $app->get() and $command->execute() are the way.

With a one or two-line change to use that suggested API, then I think that your CommandTester trait is spot on.

[greg-1-anderson]

I made a 4.2 release of consolidation/annotated-command and updated #675 to use it.

[lpeabody]

I added a Runner method for returning an application, meant to be used by CommandTesterTrait for getting a command and running it through CommandTester.

CommandTesterTrait setup method now gets the runner ready to use:
https://bitbucket.org/lpeabody/release-manager/src/df4636eb151bf54bfdaebb2af8fbf317fb0b7252/tests/CommandTesterTrait.php?at=feature%2Frobo-966-g1a-updates#lines-31:45

Execute method using application getter in CommandTesterTrait:
https://bitbucket.org/lpeabody/release-manager/src/df4636eb151bf54bfdaebb2af8fbf317fb0b7252/tests/CommandTesterTrait.php?at=feature%2Frobo-966-g1a-updates#lines-132:141

the application getter function added to Runner:
https://github.com/lpeabody/Robo/blob/feature/947-modifications-for-interactive-testing/src/Runner.php#L180-L187

Working very well. I can't think of any drawbacks.

Just saw your note about releasing 4.2 of annotated-command and updating #675. I will update my codebase accordingly... right after I get some lunch 👍

[greg-1-anderson]

I merged #675, so at this point you could make a PR off of the main branch.

[greg-1-anderson]

getAppForTesting looks about right. Just a couple comments:

1. Let the user pass in the config object, and pass that along to createDefaultContainer, just in case someone wants to test with specific configuration values.
2. Use Symfony\Component\Console\Output\NullOutput for $output, and pass that to both getRoboFileCommands() and createDefaultContainer();
3. I'm undecided on this point, but it might be helpful to allow the caller to provide $commandFiles, and only use getRoboFileCommands if it's null.

[greg-1-anderson]

Bonus points for adding at least one CommandTester-based test to Robo itself.

[lpeabody]

I'm undecided on this point, but it might be helpful to allow the caller to provide $commandFiles, and only use getRoboFileCommands if it's null.

I had a similar thought process. If I only care about testing a command from a single file, then why go through the command file discovery process when I can just hand it the file. I think this is a good suggestion. Honestly, I would have done it if I wasn't so tunnel visioned into getting a passing test...

Bonus points for adding at least one CommandTester-based test to Robo itself.

Challenge accepted.

[lpeabody]

I merged #675, so at this point you could make a PR off of the main branch.

Just noting that #675 appears to still be open. It also does not seem to contain any new commits since Feb 1 2018 https://github.com/consolidation/Robo/pull/675/commits.

[greg-1-anderson]

Oh sorry I merged #966. Do you really need #675? I am still ambivalent about that one. It seems that if you use $app->get() to get the Symfony Command object that should be good enough; you don't really need a reference to the commandfile instance, do you?

[lpeabody]

Definitely don't need it to accomplish what we're trying to accomplish in this issue! I was just following up since you mentioned it was merged, and it was not merged :)

[greg-1-anderson]

Cool, thanks. I just grabbed the wrong number from the wrong open tab. 😝