Merge pull request #2 from netsells/feature/pre-pr-tidy

Pre-PR Tidy
netsells · Jul 27, 2020 · df18ce0 · df18ce0
2 parents 39e3a07 + 16e81b8
commit df18ce0
Show file tree

Hide file tree

Showing 13 changed files with 149 additions and 92 deletions.
diff --git a/app/Commands/AwsSsmStartSession.php b/app/Commands/AwsSsmStartSession.php
@@ -2,11 +2,9 @@
 
 namespace App\Commands;
 
-use App\Exceptions\ProcessFailed;
 use App\Helpers\Helpers;
-use Symfony\Component\Process\Process;
+use App\Exceptions\ProcessFailed;
 use LaravelZero\Framework\Commands\Command;
-use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Input\InputArgument;
 
 class AwsSsmStartSession extends Command
@@ -101,7 +99,6 @@ private function generateTempSshKey()
                     '-f', $keyName,
                     '-C', "netsells-cli-ssm-ssh-session"
                 ])
-                ->echoLineByLineOutput(false)
                 ->run();
         } catch (ProcessFailed $e) {
             $this->error("Unable to generate temp ssh key.");

diff --git a/app/Commands/DockerBuildCommand.php b/app/Commands/DockerBuildCommand.php
@@ -2,6 +2,7 @@
 
 namespace App\Commands;
 
+use App\Exceptions\ProcessFailed;
 use App\Helpers\Helpers;
 use App\Helpers\NetsellsFile;
 use Symfony\Component\Process\Process;
@@ -93,24 +94,18 @@ public function handle()
 
     protected function callBuild(string $tag, string $service = null): bool
     {
-        $process = new Process([
-            'docker-compose',
-            '-f', 'docker-compose.yml',
-            '-f', 'docker-compose.prod.yml',
-            'build', '--no-cache', $service
-        ], null, [
-            'TAG' => $tag,
-        ], null, 1200); // 20min timeout
-
-        $process->start();
-
-        foreach ($process as $data) {
-            echo $data;
-        }
-
-        $process->wait();
-
-        if ($process->getExitCode() !== 0) {
+        try {
+            $this->helpers->process()->withCommand([
+                'docker-compose',
+                '-f', 'docker-compose.yml',
+                '-f', 'docker-compose.prod.yml',
+                'build', '--no-cache', $service
+            ])
+            ->withEnvironmentVars(['TAG' => $tag])
+            ->withTimeout(1200) // 20mins
+            ->echoLineByLineOutput(true)
+            ->run();
+        } catch (ProcessFailed $e) {
             $this->error("Unable to build all images, check the above output for reasons why.");
             return false;
         }

diff --git a/app/Commands/DockerPushCommand.php b/app/Commands/DockerPushCommand.php
@@ -4,6 +4,7 @@
 
 use App\Helpers\Helpers;
 use App\Helpers\NetsellsFile;
+use App\Exceptions\ProcessFailed;
 use Symfony\Component\Process\Process;
 use LaravelZero\Framework\Commands\Command;
 use Symfony\Component\Console\Input\InputOption;
@@ -93,24 +94,18 @@ public function handle()
 
     protected function callPush(string $tag, string $service = null): bool
     {
-        $process = new Process([
-            'docker-compose',
-            '-f', 'docker-compose.yml',
-            '-f', 'docker-compose.prod.yml',
-            'push', $service
-        ], null, [
-            'TAG' => $tag,
-        ], null, 1200); // 20min timeout
-
-        $process->start();
-
-        foreach ($process as $data) {
-            echo $data;
-        }
-
-        $process->wait();
-
-        if ($process->getExitCode() !== 0) {
+         try {
+            $this->helpers->process()->withCommand([
+                'docker-compose',
+                '-f', 'docker-compose.yml',
+                '-f', 'docker-compose.prod.yml',
+                'push', $service
+            ])
+            ->withEnvironmentVars(['TAG' => $tag])
+            ->withTimeout(1200) // 20mins
+            ->echoLineByLineOutput(true)
+            ->run();
+        } catch (ProcessFailed $e) {
             $this->error("Unable to push all items to AWS, check the above output for reasons why.");
             return false;
         }

diff --git a/app/Helpers/Aws/Ec2.php b/app/Helpers/Aws/Ec2.php
@@ -32,7 +32,6 @@ public function listInstances(Command $command, $query): ?Collection
 
         try {
             $processOutput = $this->aws->newProcess($command, $commandOptions)
-            ->echoLineByLineOutput(false)
             ->run();
         } catch (ProcessFailed $e) {
             $command->error("Unable to list ec2 instances");

diff --git a/app/Helpers/Aws/Ecs.php b/app/Helpers/Aws/Ecs.php
@@ -27,7 +27,6 @@ public function authenticateDocker(Command $command): bool
             $processOutput = $this->aws->newProcess($command, [
                 'ecr', 'get-login-password',
             ])
-            ->echoLineByLineOutput(false)
             ->run();
         } catch (ProcessFailed $e) {
             $command->error("Unable to get docker password from AWS.");
@@ -43,7 +42,6 @@ public function authenticateDocker(Command $command): bool
                 "--password={$password}",
                 "{$awsAccountId}.dkr.ecr.{$awsRegion}.amazonaws.com"
             ])
-            ->echoLineByLineOutput(false)
             ->run();
         } catch (ProcessFailed $e) {
             $command->error("Unable to login to docker.");
@@ -59,7 +57,6 @@ public function getTaskDefinition(Command $command, $name): ?array
             $processOutput = $this->aws->newProcess($command, [
                 'ecs', 'describe-task-definition', "--task-definition={$name}",
             ])
-            ->echoLineByLineOutput(false)
             ->run();
         } catch (ProcessFailed $e) {
             $command->error("Unable to get task definition [{$name}] from AWS.");
@@ -75,7 +72,6 @@ public function registerTaskDefinition(Command $command, string $taskDefinitionJ
             $processOutput = $this->aws->newProcess($command, [
                 'ecs', 'register-task-definition', "--cli-input-json", $taskDefinitionJson,
             ])
-            ->echoLineByLineOutput(false)
             ->run();
         } catch (ProcessFailed $e) {
             $command->error("Unable to register task definition in AWS.");
@@ -94,7 +90,6 @@ public function updateService(Command $command, string $clusterName, string $ser
                 "--service={$serviceName}",
                 "--task-definition={$taskDefinition}",
             ])
-            ->echoLineByLineOutput(false)
             ->run();
         } catch (ProcessFailed $e) {
             $command->error("Unable to update service in AWS.");
@@ -123,7 +118,6 @@ public function runTaskWithCommand(Command $command, string $clusterName, string
                 "--overrides={$overrides}",
                 "--task-definition={$taskDefinition}",
             ])
-            ->echoLineByLineOutput(false)
             ->run();
         } catch (ProcessFailed $e) {
             $command->error("Unable to start migration task in AWS.");

diff --git a/app/Helpers/Aws/Ssm.php b/app/Helpers/Aws/Ssm.php
@@ -27,7 +27,6 @@ public function sendRemoteCommand(Command $command, string $instanceId, string $
                 '--parameters', "commands=\"{$remoteCommand}\"",
                 '--comment', '"Temporary SSM SSH Access via Netsells CLI"'
             ])
-            ->echoLineByLineOutput(false)
             ->run();
 
             return true;

diff --git a/app/Helpers/Helpers.php b/app/Helpers/Helpers.php
@@ -26,7 +26,7 @@ public function aws(): Aws
 
     public function process(): Process
     {
-        return new Process($this);
+        return app(Process::class);
     }
 
     public function netsellsFile(): NetsellsFile

diff --git a/app/Helpers/Process.php b/app/Helpers/Process.php
@@ -8,7 +8,7 @@
 class Process
 {
     protected $echoOnFailure = true;
-    protected $echoLineByLineOutput = true;
+    protected $echoLineByLineOutput = false;
 
     protected $timeout = 60;
     protected $environmentVars = [];
@@ -19,12 +19,12 @@ public function withCommand(array $arguments)
     {
         $this->arguments = $arguments;
 
-        $this->process = new SymfonyProcess($this->arguments, null, $this->environmentVars, null, $this->timeout);
         return $this;
     }
 
     public function run()
     {
+        $this->process = new SymfonyProcess($this->arguments, null, $this->environmentVars, null, $this->timeout);
         $this->process->start();
 
         if ($this->echoLineByLineOutput) {
@@ -36,7 +36,8 @@ public function run()
         $this->process->wait();
 
         if ($this->process->getExitCode() !== 0) {
-            if ($this->echoOnFailure) {
+            // If we're already echo'ing line by line, there's no point echoing it again
+            if ($this->echoOnFailure && !$this->echoLineByLineOutput) {
                 foreach ($this->process as $data) {
                     echo $data;
                 }

diff --git a/composer.json b/composer.json
@@ -11,14 +11,15 @@
         }
     ],
     "require": {
-        "php": "^7.2.5",
+        "php": "^7.3",
         "laminas/laminas-text": "^2.7",
         "laravel-zero/framework": "^7.0",
         "nunomaduro/laravel-console-menu": "^3.0",
         "padraic/phar-updater": "^1.0.6",
         "symfony/yaml": "^5.0"
     },
     "require-dev": {
+        "fzaninotto/faker": "^1.9",
         "mockery/mockery": "^1.3.1",
         "phpunit/phpunit": "^8.5"
     },

diff --git a/composer.lock b/composer.lock
diff --git a/tests/Feature/AwsEc2ListCommandTest.php b/tests/Feature/AwsEc2ListCommandTest.php
@@ -0,0 +1,64 @@
+<?php
+
+namespace Tests\Feature;
+
+use App\Exceptions\ProcessFailed;
+use App\Helpers\Process;
+use Illuminate\Foundation\Testing\WithFaker;
+use Mockery;
+use Tests\TestCase;
+
+class AwsEc2ListCommandTest extends TestCase
+{
+    use WithFaker;
+
+    public function testHandlesError()
+    {
+        $this->mock(Process::class, function ($mock) {
+            $mock->shouldReceive('withCommand')->once()->andReturnSelf();
+            $mock->shouldReceive('run')->once()->andThrow(ProcessFailed::class, "Something bad happened", 1);
+        });
+
+        $this->artisan('aws:ec2:list')
+             ->assertExitCode(1);
+    }
+
+    public function testHandlesNoInstances()
+    {
+        $this->mock(Process::class, function ($mock) {
+            $mock->shouldReceive('withCommand')->once()->andReturnSelf();
+            $mock->shouldReceive('run')->once()->andReturn(null);
+        });
+
+        $this->artisan('aws:ec2:list')
+             ->assertExitCode(0);
+    }
+
+    public function testHandlesInstances()
+    {
+        $instance = [
+            'InstanceId' => $this->faker->iban,
+            'Name' => $this->faker->company,
+            'PrivateIpAddress' => $this->faker->localIpv4,
+            'InstanceType' => $this->faker->creditCardType,
+        ];
+
+        $this->mock(Process::class, function ($mock) use ($instance) {
+            $mock->shouldReceive('withCommand')->once()->andReturnSelf();
+            $mock->shouldReceive('run')->once()->andReturn(json_encode([
+                [
+                    [
+                        'InstanceId' => $instance['InstanceId'],
+                        'Name' => $instance['Name'],
+                        'PrivateIpAddress' => $instance['PrivateIpAddress'],
+                        'InstanceType' => $instance['InstanceType'],
+                    ]
+                ]
+            ]));
+        });
+
+        // TODO: Learn how to read output and assert, laravel's implementation is crap
+        $this->artisan('aws:ec2:list')
+            ->assertExitCode(0);
+    }
+}
diff --git a/tests/Feature/InspiringCommandTest.php b/tests/Feature/InspiringCommandTest.php