Merge pull request #307 from pact-foundation/fix/display_stderr_on_pu…

…blish_error Fix/display stderr on publish error
pact-foundation · Apr 29, 2023 · 52f2be4 · 52f2be4
2 parents 2380bd3 + d7b0b4f
commit 52f2be4
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 70 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -70,7 +70,6 @@ jobs:
         uses: ramsey/composer-install@v2
         with:
           dependency-versions: ${{ matrix.dependencies }}
-          composer-options: ${{ matrix.composer-options }}
 
       - name: Composer test
         run: composer test
diff --git a/src/PhpPact/Standalone/Broker/Broker.php b/src/PhpPact/Standalone/Broker/Broker.php
@@ -2,17 +2,11 @@
 
 namespace PhpPact\Standalone\Broker;
 
-use Amp\ByteStream\ResourceOutputStream;
-use Amp\Log\ConsoleFormatter;
-use Amp\Log\StreamHandler;
-use Monolog\Logger;
 use PhpPact\Standalone\Installer\Model\Scripts;
 use PhpPact\Standalone\Runner\ProcessRunner;
 
 class Broker
 {
-    private Logger $logger;
-
     private BrokerConfig $config;
 
     private string $command;
@@ -21,11 +15,6 @@ public function __construct(BrokerConfig $config)
     {
         $this->config  = $config;
         $this->command = Scripts::getBroker();
-        $this->logger  = (new Logger('console'))
-            ->pushHandler(
-                (new StreamHandler(new ResourceOutputStream(\STDOUT)))
-                    ->setFormatter(new ConsoleFormatter(null, null, true))
-            );
     }
 
     /**
@@ -46,10 +35,6 @@ public function canIDeploy(): mixed
         );
         $runner->runBlocking();
 
-        if ($runner->getExitCode() !== 0) {
-            throw new \Exception($runner->getStderr());
-        }
-
         return \json_decode($runner->getOutput(), true, 512, JSON_THROW_ON_ERROR);
     }
 
@@ -97,10 +82,6 @@ public function createOrUpdatePacticipant(): mixed
         );
         $runner->runBlocking();
 
-        if ($runner->getExitCode() !== 0) {
-            throw new \Exception($runner->getStderr());
-        }
-
         return \json_decode($runner->getOutput(), true, 512, JSON_THROW_ON_ERROR);
     }
 
@@ -129,10 +110,6 @@ public function createOrUpdateWebhook(): mixed
         );
         $runner->runBlocking();
 
-        if ($runner->getExitCode() !== 0) {
-            throw new \Exception($runner->getStderr());
-        }
-
         return \json_decode($runner->getOutput(), true, 512, JSON_THROW_ON_ERROR);
     }
 
@@ -155,10 +132,6 @@ public function createVersionTag(): mixed
         );
         $runner->runBlocking();
 
-        if ($runner->getExitCode() !== 0) {
-            throw new \Exception($runner->getStderr());
-        }
-
         return \json_decode($runner->getOutput(), true, 512, JSON_THROW_ON_ERROR);
     }
 
@@ -186,10 +159,6 @@ public function createWebhook(): mixed
         );
         $runner->runBlocking();
 
-        if ($runner->getExitCode() !== 0) {
-            throw new \Exception($runner->getStderr());
-        }
-
         return \json_decode($runner->getOutput(), true, 512, JSON_THROW_ON_ERROR);
     }
 
@@ -211,10 +180,6 @@ public function describeVersion(): mixed
         );
         $runner->runBlocking();
 
-        if ($runner->getExitCode() !== 0) {
-            throw new \Exception($runner->getStderr());
-        }
-
         return \json_decode($runner->getOutput(), true, 512, JSON_THROW_ON_ERROR);
     }
 
@@ -235,10 +200,6 @@ public function listLatestPactVersions(): mixed
         );
         $runner->runBlocking();
 
-        if ($runner->getExitCode() !== 0) {
-            throw new \Exception($runner->getStderr());
-        }
-
         return \json_decode($runner->getOutput(), true, 512, JSON_THROW_ON_ERROR);
     }
 
@@ -265,13 +226,8 @@ public function publish(): void
                 $this->getArguments()
             )
         );
-        $runner->runBlocking();
 
-        if ($runner->getExitCode() !== 0) {
-            $this->logger->error($runner->getStderr());
-        }
-
-        $this->logger->debug($runner->getOutput());
+        $runner->runBlocking();
     }
 
     /**
@@ -291,10 +247,6 @@ public function testWebhook(): mixed
         );
         $runner->runBlocking();
 
-        if ($runner->getExitCode() !== 0) {
-            throw new \Exception($runner->getStderr());
-        }
-
         return \json_decode($runner->getOutput(), true, 512, JSON_THROW_ON_ERROR);
     }
 

diff --git a/src/PhpPact/Standalone/ProviderVerifier/VerifierProcess.php b/src/PhpPact/Standalone/ProviderVerifier/VerifierProcess.php
@@ -40,17 +40,7 @@ public function run(array $arguments, ?int $processTimeout = null, ?int $process
 
         $logger->info("Verifying PACT with script:\n{$processRunner->getCommand()}\n\n");
 
-        try {
-            $processRunner->runBlocking();
-
-            $logger->info('out > ' . $processRunner->getOutput());
-            $logger->error('err > ' . $processRunner->getStderr());
-        } catch (\Exception $e) {
-            $logger->info('out > ' . $processRunner->getOutput());
-            $logger->error('err > ' . $processRunner->getStderr());
-
-            throw $e;
-        }
+        $processRunner->runBlocking();
     }
 
     private function getLogger(): LoggerInterface

diff --git a/src/PhpPact/Standalone/Runner/ProcessRunner.php b/src/PhpPact/Standalone/Runner/ProcessRunner.php
@@ -99,7 +99,9 @@ public function runBlocking(): int
             $logger->debug("Exit code: {$this->getExitCode()}");
 
             if ($this->getExitCode() !== 0) {
-                throw new \Exception("PactPHP Process returned non-zero exit code: {$this->getExitCode()}");
+                $this->logger->info('out > ' . $this->getOutput());
+                $this->logger->error('err > ' . $this->getStderr());
+                throw new \Exception("PactPHP Process returned non-zero exit code: {$this->getExitCode()}", $this->getExitCode());
             }
 
             Loop::stop();

diff --git a/tests/PhpPact/Standalone/Broker/BrokerTest.php b/tests/PhpPact/Standalone/Broker/BrokerTest.php
@@ -78,4 +78,22 @@ public function listLatestPactVersions(): void
         $result = $broker->listLatestPactVersions();
         $this->assertArrayHasKey('pacts', $result);
     }
+
+    /**
+     * @test
+     *
+     * @throws \Exception
+     */
+    public function publishLogsStdError(): void
+    {
+        $config = new BrokerConfig();
+        $config->setPactLocations('not a directory');
+        $broker = new Broker($config);
+        try {
+            $broker->publish();
+        } catch(\Exception $e) {
+            $this->assertEquals(1, $e->getCode());
+            $this->assertStringContainsString("PactPHP Process returned non-zero exit code: 1", $e->getMessage());
+        }
+    }
 }
diff --git a/tests/PhpPact/Standalone/Runner/ProcessRunnerTest.php b/tests/PhpPact/Standalone/Runner/ProcessRunnerTest.php
@@ -39,13 +39,13 @@ public function testBlockingProcess()
         try {
             $p->runBlocking();
         } catch (\Exception $e) {
+            $exitCode = $p->getExitCode();
+            $this->assertEquals($exitCode, $e->getCode());
+            $this->assertStringContainsString("PactPHP Process returned non-zero exit code: $exitCode", $e->getMessage());
+            $this->assertNotEquals($exitCode, 0, 'Expect the exit code to be non-zero: ' . $exitCode);
+            $this->assertStringContainsString($expectedErr, $p->getStderr(), "Expect '{$expectedErr}' to be in the stderr");
+            $this->assertEquals(null, $p->getOutput(), 'Expect a null stdout');
         }
-
-        $exitCode = $p->getExitCode();
-
-        $this->assertNotEquals($exitCode, 0, 'Expect the exit code to be non-zero: ' . $exitCode);
-        $this->assertStringContainsString($expectedErr, $p->getStderr(), "Expect '{$expectedErr}' to be in the stderr");
-        $this->assertEquals(null, $p->getOutput(), 'Expect a null stdout');
     }
 
     /**
@@ -62,12 +62,12 @@ public function testProcessRunnerShouldReturnCompleteOutput()
         $p              = new ProcessRunner($cmd, []);
         $expectedOutput = 'third line';
         $expectedErr    = 'fourth line';
-
         try {
             $p->runBlocking();
         } catch (\Exception $e) {
+            $this->assertEquals(42, $e->getCode());
+            $this->assertStringContainsString("PactPHP Process returned non-zero exit code: 42", $e->getMessage());
         }
-
         $this->assertTrue((\stripos($p->getOutput(), $expectedOutput) !== false), "Expect '{$expectedOutput}' to be in the output:");
         $this->assertTrue((\stripos($p->getStderr(), $expectedErr) !== false), "Expect '{$expectedErr}' to be in the stderr");
     }