Return a rich object after running buildkite-agent command (#107)

* Retrun a rich object after running buildkite-agent command

* Tweak

CHANGELOG.md

@@ -1,3 +1,6 @@
+### 4.6.0
+* Remove `capture` concept on `Buildkite::Pipelines::Command#run` and replaced with `Buildkite::Pipelines::Command::Result` object to represent `Open3.capture3` result.
+
 ### 4.5.0
 * Do not upload `pipeline.yml` when steps is empty.
 

VERSION

@@ -1 +1 @@
-4.5.0
+4.6.0

lib/buildkite/builder/commands/run.rb

@@ -17,7 +17,7 @@ def run
           # variables to be set. It also uploads the pipeline to Buildkite.
           log.info "+++ 🧰 #{'Buildkite Builder'.color(:springgreen)} v#{Buildkite::Builder.version} ─ #{relative_pipeline_path.to_s.yellow}"
 
-          if Buildkite::Pipelines::Command.meta_data(:exists, Builder.meta_data.fetch(:job))
+          if Buildkite::Pipelines::Command.meta_data(:exists, Builder.meta_data.fetch(:job)).success?
             log.info "Pipeline already uploaded in #{Buildkite.env.step_id} step".color(:dimgray)
           else
             Pipeline.new(pipeline_path, logger: log).upload

lib/buildkite/builder/pipeline.rb

@@ -54,8 +54,11 @@ def upload
             file.write(contents)
 
             logger.info "+++ :pipeline: Uploading pipeline"
-            unless Buildkite::Pipelines::Command.pipeline(:upload, file.path)
-              logger.info "Pipeline upload failed, saving as artifact…"
+
+            result = Buildkite::Pipelines::Command.pipeline(:upload, file.path)
+
+            unless result.success?
+              logger.info "Pipeline upload failed with #{result.stderr}, saving as artifact…"
               Buildkite::Pipelines::Command.artifact!(:upload, file.path)
               abort
             end

lib/buildkite/pipelines/command.rb

@@ -7,6 +7,23 @@ module Pipelines
     class Command
       class CommandFailedError < StandardError; end
 
+      class Result
+        attr_reader :stdout, :stderr
+        def initialize(stdout, stderr, status)
+          @stdout = stdout.strip
+          @stderr = stderr.strip
+          @status = status
+        end
+
+        def success?
+          @status.success?
+        end
+
+        def output
+          @output ||= "#{stdout}\n#{stderr}".strip
+        end
+      end
+
       BIN_PATH = 'buildkite-agent'
       COMMANDS = %w(
         pipeline
@@ -21,25 +38,25 @@ def pipeline(subcommand, *args, exception: false)
         end
 
         def artifact(subcommand, *args, exception: false)
-          capture = case subcommand.to_s
-          when 'shasum', 'search' then true
-          else false
-          end
+          result = new(:artifact, subcommand, *args).run(exception: exception)
 
-          new(:artifact, subcommand, *args).run(capture: capture, exception: exception)
+          case subcommand.to_s
+          when 'shasum', 'search' then result.output
+          else result
+          end
         end
 
         def annotate(body, *args, exception: false)
           new(:annotate, body, *args).run(exception: exception)
         end
 
         def meta_data(subcommand, *args, exception: false)
-          capture = case subcommand.to_s
-          when 'get', 'keys' then true
-          else false
-          end
+          result = new(:'meta-data', subcommand, *args).run(exception: exception)
 
-          new(:'meta-data', subcommand, *args).run(capture: capture, exception: exception)
+          case subcommand.to_s
+          when 'get', 'keys' then result.output
+          else result
+          end
         end
       end
 
@@ -58,16 +75,14 @@ def initialize(command, subcommand, *args)
         @args = transform_args(args)
       end
 
-      def run(capture: false, exception: false)
+      def run(exception: false)
         stdout, stderr, status = Open3.capture3(*to_a)
-        if capture
-          stdout
-        elsif status.success?
-          true
-        elsif exception
-          raise CommandFailedError, "#{stdout}\n#{stderr}"
+        result = Result.new(stdout, stderr, status)
+
+        if !result.success? && exception
+          raise CommandFailedError, "#{result.output}"
         else
-          false
+          result
         end
       end
 

spec/buildkite/builder/commands/run_spec.rb

@@ -4,6 +4,7 @@
   let(:argv) { [] }
   let(:fixture_project) { :single_pipeline }
   let(:pipeline) { instance_double(Buildkite::Builder::Pipeline) }
+  let(:result) { instance_double(Buildkite::Pipelines::Command::Result, success?: success) }
 
   before do
     stub_const('ARGV', argv)
@@ -13,10 +14,10 @@
 
   describe '.execute' do
     context 'when step key exists' do
-      let(:exists) { true }
+      let(:success) { true }
 
       before do
-        allow(Buildkite::Pipelines::Command).to receive(:meta_data).with(:exists, Buildkite::Builder.meta_data.fetch(:job)).and_return(exists)
+        allow(Buildkite::Pipelines::Command).to receive(:meta_data).with(:exists, Buildkite::Builder.meta_data.fetch(:job)).and_return(result)
       end
 
       it 'does not upload the pipeline' do
@@ -26,10 +27,10 @@
       end
 
       context 'when step key does not exists' do
-        let(:exists) { false }
+        let(:success) { false }
 
         before do
-          allow(Buildkite::Pipelines::Command).to receive(:meta_data).with(:exists, Buildkite::Builder.meta_data.fetch(:job)).and_return(exists)
+          allow(Buildkite::Pipelines::Command).to receive(:meta_data).with(:exists, Buildkite::Builder.meta_data.fetch(:job)).and_return(result)
         end
 
         it 'uploads the context' do

spec/buildkite/builder/pipeline_spec.rb

@@ -73,7 +73,7 @@
         expect(subcommand).to eq(:upload)
         pipeline_path = path
         pipeline_contents = File.read(path)
-        true
+        instance_double(Buildkite::Pipelines::Command::Result, success?: true)
       end
 
       pipeline.upload
@@ -89,7 +89,7 @@
     end
 
     it 'uploads the pipeline as an artifact on failure' do
-      expect(Buildkite::Pipelines::Command).to receive(:pipeline).once.ordered.and_return(false)
+      expect(Buildkite::Pipelines::Command).to receive(:pipeline).once.ordered.and_return(instance_double(Buildkite::Pipelines::Command::Result, success?: false, stderr: 'error'))
       expect(Buildkite::Pipelines::Command).to receive(:artifact!).ordered.once do |subcommand, path|
         expect(subcommand).to eq(:upload)
         expect(File.read(path)).to eq(<<~YAML)
@@ -139,7 +139,7 @@
           expect(subcommand).to eq(:upload)
           pipeline_path = path
           pipeline_contents = File.read(path)
-          true
+          instance_double(Buildkite::Pipelines::Command::Result, success?: true)
         end
 
         expect(Buildkite::Pipelines::Command).to receive(:meta_data!).with(:set, anything, anything)

spec/buildkite/pipelines/command_spec.rb

@@ -9,9 +9,10 @@
       instance = double
       subcommand = double
       args = double
+      result = instance_double(described_class::Result, success?: true)
 
       expect(described_class).to receive(:new).with(command, subcommand, args).and_return(instance)
-      expect(instance).to receive(:run)
+      expect(instance).to receive(:run).and_return(result)
 
       described_class.public_send(method_name, subcommand, args)
     end
@@ -94,12 +95,13 @@
       instance.run
     end
 
-    it 'returns the stdout of the command when capture kwarg is true' do
-      expect(instance.run(capture: true)).to eq('stdout')
+    it 'returns result object' do
+      expect(instance.run).to be_an_instance_of(described_class::Result)
     end
 
     it 'returns the status success of the command when capture kwarg is false' do
-      expect(instance.run).to eq(mock_status.success?)
+      result = instance.run
+      expect(result.success?).to eq(mock_status.success?)
     end
   end
 end