From 95674968a62f7dc5fa748ff31e0efdd12df8dcfe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= <theo.fidry@gmail.com>
Date: Fri, 31 Mar 2023 01:03:21 +0200
Subject: [PATCH] refactor: Simplify the error handling of Extract

Simply throw the exception rather than handling it to write the error
our own way.
---
 src/Console/Command/Extract.php       | 23 ++------------
 tests/Console/Command/ExtractTest.php | 44 ++++++++++++++++++---------
 2 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/src/Console/Command/Extract.php b/src/Console/Command/Extract.php
index 3a27e5ae5..b2ae9caa5 100644
--- a/src/Console/Command/Extract.php
+++ b/src/Console/Command/Extract.php
@@ -21,7 +21,6 @@
 use KevinGH\Box\Box;
 use KevinGH\Box\Pharaoh\Pharaoh;
 use RecursiveIteratorIterator;
-use RuntimeException;
 use Symfony\Component\Console\Exception\RuntimeException as ConsoleRuntimeException;
 use Symfony\Component\Console\Input\InputArgument;
 use Throwable;
@@ -70,7 +69,7 @@ public function execute(IO $io): int
             return ExitCode::FAILURE;
         }
 
-        [$box, $cleanUpTmpPhar] = $this->getBox($filePath, $io);
+        [$box, $cleanUpTmpPhar] = $this->getBox($filePath);
 
         if (null === $box) {
             return ExitCode::FAILURE;
@@ -83,13 +82,7 @@ public function execute(IO $io): int
             $restoreLimit();
         };
 
-        try {
-            self::dumpPhar($outputDir, $box, $cleanUp);
-        } catch (RuntimeException $exception) {
-            $io->error($exception->getMessage());
-
-            return ExitCode::FAILURE;
-        }
+        self::dumpPhar($outputDir, $box, $cleanUp);
 
         return ExitCode::SUCCESS;
     }
@@ -115,7 +108,7 @@ private static function getPharFilePath(IO $io): ?string
     /**
      * @return array{Box, callable(): void}|array{null, null}
      */
-    private function getBox(string $filePath, IO $io): ?array
+    private function getBox(string $filePath): ?array
     {
         $cleanUp = static fn () => null;
 
@@ -133,10 +126,6 @@ private function getBox(string $filePath, IO $io): ?array
                 $cleanUp,
             ];
         } catch (Throwable $throwable) {
-            // Continue
-        }
-
-        if ($io->isDebug()) {
             $cleanUp();
 
             throw new ConsoleRuntimeException(
@@ -145,12 +134,6 @@ private function getBox(string $filePath, IO $io): ?array
                 $throwable,
             );
         }
-
-        $io->error('The given file is not a valid PHAR.');
-
-        $cleanUp();
-
-        return [null, null];
     }
 
     /**
diff --git a/tests/Console/Command/ExtractTest.php b/tests/Console/Command/ExtractTest.php
index b042ad510..d3417fd7c 100644
--- a/tests/Console/Command/ExtractTest.php
+++ b/tests/Console/Command/ExtractTest.php
@@ -114,23 +114,33 @@ private static function pharProvider(): iterable
      */
     public function test_it_cannot_extract_an_invalid_phar(
         string $pharPath,
+        string $exceptionClassName,
+        string $expectedExceptionMessage,
     ): void {
-        $this->commandTester->execute(
-            [
-                'command' => 'extract',
-                'phar' => $pharPath,
-                'output' => $this->tmp,
-            ],
-        );
-
-        $expectedOutput = <<<'OUTPUT'
-
-             [ERROR] The given file is not a valid PHAR.
+        try {
+            $this->commandTester->execute(
+                [
+                    'command' => 'extract',
+                    'phar' => $pharPath,
+                    'output' => $this->tmp,
+                ],
+            );
 
+            self::fail('Expected exception to be thrown.');
+        } catch (RuntimeException $exception) {
+            // Continue
+            $innerException = $exception->getPrevious();
+        }
 
-            OUTPUT;
+        self::assertSame(
+            $exceptionClassName,
+            $innerException::class,
+        );
+        self::assertMatchesRegularExpression(
+            $expectedExceptionMessage,
+            $innerException->getMessage(),
+        );
 
-        $this->assertSameOutput($expectedOutput, ExitCode::FAILURE);
         self::assertSame([], $this->collectExtractedFiles());
     }
 
@@ -138,14 +148,20 @@ public static function invalidPharPath(): iterable
     {
         yield 'not a valid PHAR with the PHAR extension' => [
             self::FIXTURES.'/invalid.phar',
+            InvalidPhar::class,
+            '/^Could not create a Phar or PharData instance for the file/',
         ];
 
         yield 'not a valid PHAR without the PHAR extension' => [
-            self::FIXTURES.'/invalid.phar',
+            self::FIXTURES.'/invalid',
+            InvalidPhar::class,
+            '/^Could not create a Phar or PharData instance for the file .+$/',
         ];
 
         yield 'corrupted PHAR (was valid; got tempered with' => [
             self::FIXTURES.'/corrupted.phar',
+            InvalidPhar::class,
+            '/^Could not create a Phar or PharData instance for the file .+$/',
         ];
     }