Merge pull request #87 from bambamboole/mc-fix-error-output

Fix error output if no callback is provided

Author: maschmann

Asm/Ansible/Command/AbstractAnsibleCommand.php

@@ -176,16 +176,18 @@ protected function runProcess(?callable $callback): int|string
         // exit code
         $result = $process->run($callback);
 
-        // text-mode
-        if (null === $callback) {
-            $result = $process->getOutput();
+        // if a callback is set, we return the exit code
+        if (null !== $callback) {
+            return $result;
+        }
 
-            if (false === $process->isSuccessful()) {
-                $process->getErrorOutput();
-            }
+        // if no callback is set, and the process is not successful, we return the output
+        if (false === $process->isSuccessful()) {
+            return $process->getErrorOutput();
         }
 
-        return $result;
+        // if no callback is set, and the process is successful, we return the output
+        return $process->getOutput();
     }
 
     /**

Asm/Ansible/Command/AnsiblePlaybook.php

@@ -6,7 +6,6 @@
 
 use Asm\Ansible\Utils\Str;
 use InvalidArgumentException;
-use JsonException;
 
 /**
  * Class AnsiblePlaybook

Tests/Asm/Ansible/Command/AnsiblePlaybookTest.php

@@ -5,6 +5,7 @@
 namespace Asm\Ansible\Command;
 
 use Asm\Ansible\Process\ProcessBuilder;
+use Asm\Ansible\Process\ProcessBuilderInterface;
 use Asm\Ansible\Testing\AnsibleTestCase;
 use Asm\Ansible\Utils\Env;
 use DateTime;
@@ -801,21 +802,14 @@ public function testExtraVars(): void
         //$playbookFile = $this->getSamplesPathFor(AnsiblePlaybook::class) . '/playbook1.yml';
 
         $tests = [
-            // Test empty strings (with & without spaces).
             [
                 'input' => '',
                 'expect' => false,
             ],
-            [
-                'input' => '   ',
-                'expect' => false,
-            ],
-            // Test empty array.
             [
                 'input' => [],
                 'expect' => false,
             ],
-            // Test Arrays
             [
                 'input' => ['key' => 'value'],
                 'expect' => '--extra-vars=key=value',
@@ -824,12 +818,6 @@ public function testExtraVars(): void
                 'input' => ['key1' => 'value1', 'key2' => 'value2'],
                 'expect' => '--extra-vars=key1=value1 key2=value2',
             ],
-            // Test valid JSON.
-            [
-                'input' => '{ "key1": "value1", "key2": "value2" }',
-                'expect' => '--extra-vars={ "key1": "value1", "key2": "value2" }',
-            ],
-            // Test key value string.
             [
                 'input' => 'key=value',
                 'expect' => '--extra-vars=key=value',
@@ -1037,4 +1025,96 @@ public function testSshPipelining(): void
             $this->assertEquals($expect, $env['ANSIBLE_SSH_PIPELINING']);
         }
     }
+
+    public function testReturnsErrorOutputIfProcessWasNotSuccessful(): void
+    {
+        $builder = $this->createMock(ProcessBuilderInterface::class);
+        $builder
+            ->expects(self::once())
+            ->method('setArguments')
+            ->willReturnSelf();
+        $builder
+            ->expects(self::once())
+            ->method('getProcess')
+            ->willReturn($process = $this->createMock(Process::class));
+        $process
+            ->expects(self::once())
+            ->method('run');
+        $process
+            ->expects(self::once())
+            ->method('isSuccessful')
+            ->willReturn(false);
+        $process
+            ->expects(self::once())
+            ->method('getErrorOutput')
+            ->willReturn('error output');
+        $process
+            ->expects(self::never())
+            ->method('getOutput');
+
+        $playbook = new AnsiblePlaybook($builder);
+
+        self::assertEquals('error output', $playbook->execute());
+    }
+
+    public function testReturnsNormalOutputIfProcessWasSuccessful(): void
+    {
+        $builder = $this->createMock(ProcessBuilderInterface::class);
+        $builder
+            ->expects(self::once())
+            ->method('setArguments')
+            ->willReturnSelf();
+        $builder
+            ->expects(self::once())
+            ->method('getProcess')
+            ->willReturn($process = $this->createMock(Process::class));
+        $process
+            ->expects(self::once())
+            ->method('run');
+        $process
+            ->expects(self::once())
+            ->method('isSuccessful')
+            ->willReturn(true);
+        $process
+            ->expects(self::once())
+            ->method('getOutput')
+            ->willReturn('success');
+        $process
+            ->expects(self::never())
+            ->method('getErrorOutput');
+
+        $playbook = new AnsiblePlaybook($builder);
+
+        self::assertEquals('success', $playbook->execute());
+    }
+
+    public function testReturnsExitCodeIfCallbackwasPassed(): void
+    {
+        $builder = $this->createMock(ProcessBuilderInterface::class);
+        $builder
+            ->expects(self::once())
+            ->method('setArguments')
+            ->willReturnSelf();
+        $builder
+            ->expects(self::once())
+            ->method('getProcess')
+            ->willReturn($process = $this->createMock(Process::class));
+        $process
+            ->expects(self::once())
+            ->method('run')
+            ->willReturn(0);
+        $process
+            ->expects(self::never())
+            ->method('isSuccessful');
+        $process
+            ->expects(self::never())
+            ->method('getOutput');
+        $process
+            ->expects(self::never())
+            ->method('getErrorOutput');
+
+        $playbook = new AnsiblePlaybook($builder);
+
+        self::assertEquals(0, $playbook->execute(fn () => null));
+    }
 }