Merge pull request #1436 from mbj/improve/capture-stdout

Change to improved command capture

Author: mbj

lib/mutant/license/subscription/commercial.rb

@@ -75,11 +75,9 @@ def commit_author(world)
 
           def capture(world, command)
             world
-              .capture_stdout(command)
-              .fmap(&:chomp)
-              .fmap { |email| Author.new(email: email) }
-              .fmap { |value| Set.new([value]) }
-              .from_right { Set.new }
+              .capture_command(command)
+              .either(->(_) { EMPTY_ARRAY }, ->(status) { [Author.new(email: status.stdout.chomp)] })
+              .to_set
           end
         end # Individual
       end # Commercial

lib/mutant/license/subscription/repository.rb

@@ -20,8 +20,8 @@ def to_s
 
         def self.load_from_git(world)
           world
-            .capture_stdout(%w[git remote --verbose])
-            .fmap(&method(:parse_remotes))
+            .capture_command(%w[git remote --verbose])
+            .fmap { |status| parse_remotes(status.stdout) }
         end
 
         def self.parse_remotes(input)

lib/mutant/repository/diff.rb

@@ -36,9 +36,8 @@ def touches_path?(path)
 
       def repository_root
         world
-          .capture_stdout(%w[git rev-parse --show-toplevel])
-          .fmap(&:chomp)
-          .fmap(&world.pathname.public_method(:new))
+          .capture_command(%w[git rev-parse --show-toplevel])
+          .fmap { |status| world.pathname.new(status.stdout.chomp) }
       end
 
       def touched_path(path, &block)
@@ -52,11 +51,10 @@ def touched_paths
 
       def diff_index(root)
         world
-          .capture_stdout(%W[git diff-index #{to}])
-          .fmap(&:lines)
-          .bind do |lines|
+          .capture_command(%W[git diff-index #{to}])
+          .bind do |status|
             Mutant
-              .traverse(->(line) { parse_line(root, line) }, lines)
+              .traverse(->(line) { parse_line(root, line) }, status.stdout.lines)
               .fmap do |paths|
                 paths.to_h { |path| [path.path, path] }
               end
@@ -105,8 +103,8 @@ def touches?(line_range)
 
         def diff_ranges
           world
-            .capture_stdout(%W[git diff --unified=0 #{to} -- #{path}])
-            .fmap(&Ranges.public_method(:parse))
+            .capture_command(%W[git diff --unified=0 #{to} -- #{path}])
+            .fmap { |status| Ranges.parse(status.stdout) }
             .from_right
         end
         memoize :diff_ranges

lib/mutant/world.rb

@@ -39,19 +39,25 @@ def inspect
       INSPECT
     end
 
+    class CommandStatus
+      include Adamantium, Anima.new(:process_status, :stderr, :stdout)
+    end # CommandStatus
+
     # Capture stdout of a command
     #
     # @param [Array<String>] command
     #
-    # @return [Either<String,String>]
-    def capture_stdout(command)
-      stdout, status = open3.capture2(*command, binmode: true)
+    # @return [Either<CommandStatus,CommandStatus>]
+    def capture_command(command)
+      stdout, stderr, process_status = open3.capture3(*command, binmode: true)
 
-      if status.success?
-        Either::Right.new(stdout)
-      else
-        Either::Left.new("Command #{command} failed!")
-      end
+      (process_status.success? ? Either::Right : Either::Left).new(
+        CommandStatus.new(
+          process_status: process_status,
+          stderr:         stderr,
+          stdout:         stdout
+        )
+      )
     end
 
     # Try const get

spec/unit/mutant/license/repository_spec.rb

@@ -60,16 +60,19 @@ def apply
       described_class.load_from_git(world)
     end
 
-    let(:world) { instance_double(Mutant::World) }
-
     before do
-      allow(world).to receive(:capture_stdout, &commands.public_method(:fetch))
+      allow(world).to receive(:capture_command, &commands.public_method(:fetch))
     end
 
-    let(:git_remote_result)    { right(git_remote)         }
-    let(:allowed_repositories) { %w[github.com/mbj/mutant] }
+    let(:allowed_repositories) { %w[github.com/mbj/mutant]      }
+    let(:git_remote_result)    { right(git_remote_status)       }
+    let(:world)                { instance_double(Mutant::World) }
+
+    let(:git_remote_status) do
+      instance_double(Mutant::World::CommandStatus, stdout: git_remote_stdout)
+    end
 
-    let(:git_remote) do
+    let(:git_remote_stdout) do
       <<~REMOTE
         origin\t[email protected]:mbj/Mutant (fetch)
         origin\t[email protected]:mbj/Mutant (push)
@@ -102,7 +105,7 @@ def apply
     end
 
     context 'on ssh url with protocol and without suffix' do
-      let(:git_remote) do
+      let(:git_remote_stdout) do
         <<~REMOTE
           origin\tssh://[email protected]/mbj/mutant (fetch)
           origin\tssh://[email protected]/mbj/mutant (push)
@@ -113,7 +116,7 @@ def apply
     end
 
     context 'on ssh url with protocol and suffix' do
-      let(:git_remote) do
+      let(:git_remote_stdout) do
         <<~REMOTE
           origin\tssh://[email protected]/mbj/mutant.git (fetch)
           origin\tssh://[email protected]/mbj/mutant.git (push)
@@ -124,7 +127,7 @@ def apply
     end
 
     context 'on https url without suffix' do
-      let(:git_remote) do
+      let(:git_remote_stdout) do
         <<~REMOTE
           origin\thttps://github.com/mbj/mutant (fetch)
           origin\thttps://github.com/mbj/mutant (push)
@@ -135,7 +138,7 @@ def apply
     end
 
     context 'on multiple different urls' do
-      let(:git_remote) do
+      let(:git_remote_stdout) do
         <<~REMOTE
           origin\thttps://github.com/mbj/mutant (fetch)
           origin\thttps://github.com/mbj/mutant (push)
@@ -161,7 +164,7 @@ def apply
     end
 
     context 'on https url with .git suffix' do
-      let(:git_remote) do
+      let(:git_remote_stdout) do
         <<~REMOTE
           origin\thttps://github.com/mbj/mutant.git (fetch)
           origin\thttps://github.com/mbj/mutant.git (push)
@@ -172,13 +175,13 @@ def apply
     end
 
     context 'when git remote line cannot be parsed' do
-      let(:git_remote) { "some-bad-remote-line\n" }
+      let(:git_remote_stdout) { "some-bad-remote-line\n" }
 
       it_fails 'Unmatched remote line: "some-bad-remote-line\n"'
     end
 
     context 'when git remote url cannot be parsed' do
-      let(:git_remote) { "some-unknown\thttp://github.com/mbj/mutant (fetch)\n" }
+      let(:git_remote_stdout) { "some-unknown\thttp://github.com/mbj/mutant (fetch)\n" }
 
       it_fails 'Unmatched git remote URL: "http://github.com/mbj/mutant"'
     end

spec/unit/mutant/license/subscription_spec.rb

@@ -77,7 +77,7 @@ def apply
     let(:world) { instance_double(Mutant::World) }
 
     before do
-      allow(world).to receive(:capture_stdout, &commands.public_method(:fetch))
+      allow(world).to receive(:capture_command, &commands.public_method(:fetch))
     end
 
     def self.it_fails(expected)
@@ -99,9 +99,13 @@ def self.it_fails_with_message(expected)
     end
 
     describe 'on opensource license' do
-      let(:git_remote_result)    { right(git_remote)         }
+      let(:git_remote_result)    { right(git_remote_status)  }
       let(:allowed_repositories) { %w[github.com/mbj/mutant] }
 
+      let(:git_remote_status) do
+        instance_double(Mutant::World::CommandStatus, stdout: git_remote_stdout)
+      end
+
       let(:license_json) do
         {
           'type'     => 'oss',
@@ -117,7 +121,7 @@ def self.it_fails_with_message(expected)
         }
       end
 
-      let(:git_remote) do
+      let(:git_remote_stdout) do
         <<~REMOTE
           origin\t[email protected]:mbj/mutant (fetch)
           origin\t[email protected]:mbj/mutant (push)
@@ -127,7 +131,7 @@ def self.it_fails_with_message(expected)
       context 'on one of many match' do
         let(:allowed_repositories) { %w[github.com/mbj/something github.com/mbj/mutant] }
 
-        let(:git_remote) do
+        let(:git_remote_stdout) do
           <<~REMOTE
             origin\t[email protected]:mbj/mutant (fetch)
             origin\t[email protected]:mbj/mutant (push)
@@ -182,7 +186,11 @@ def self.it_fails_with_message(expected)
 
     describe 'on commercial license' do
       context 'on organization licenses' do
-        let(:git_remote_result) { right(git_remote) }
+        let(:git_remote_result) { right(git_remote_status) }
+
+        let(:git_remote_status) do
+          instance_double(Mutant::World::CommandStatus, stdout: git_remote_stdout)
+        end
 
         let(:license_json) do
           {
@@ -200,7 +208,7 @@ def self.it_fails_with_message(expected)
           }
         end
 
-        let(:git_remote) do
+        let(:git_remote_stdout) do
           <<~REMOTE
             origin\t[email protected]:mbj/Mutant (fetch)
             origin\t[email protected]:mbj/Mutant (push)
@@ -227,7 +235,7 @@ def self.it_fails_with_message(expected)
         context 'on a one of many match' do
           let(:allowed_repositories) { %w[github.com/mbj/something github.com/mbj/mutant] }
 
-          let(:git_remote) do
+          let(:git_remote_stdout) do
             <<~REMOTE
               origin\t[email protected]:mbj/mutant (fetch)
               origin\t[email protected]:mbj/mutant (push)
@@ -268,10 +276,24 @@ def self.it_fails_with_message(expected)
       end
 
       shared_examples 'individual licenses' do
-        let(:git_config_author) { "[email protected]\n"                   }
-        let(:git_config_result) { Mutant::Either::Right.new(git_config_author) }
-        let(:git_show_author)   { "[email protected]\n"                   }
-        let(:git_show_result)   { Mutant::Either::Right.new(git_show_author)   }
+        let(:git_config_author) { "[email protected]\n"                          }
+        let(:git_config_result) { Mutant::Either::Right.new(git_config_author_status) }
+        let(:git_show_author)   { "[email protected]\n"                          }
+        let(:git_show_result)   { Mutant::Either::Right.new(git_show_author_status)   }
+
+        let(:git_config_author_status) do
+          instance_double(
+            Mutant::World::CommandStatus,
+            stdout: git_config_author
+          )
+        end
+
+        let(:git_show_author_status) do
+          instance_double(
+            Mutant::World::CommandStatus,
+            stdout: git_show_author
+          )
+        end
 
         let(:licensed_authors) do
           %w[

spec/unit/mutant/repository/diff_spec.rb

@@ -16,19 +16,28 @@
     )
   end
 
+  def mk_command_status_result(stdout)
+    right(
+      instance_double(
+        Mutant::World::CommandStatus,
+        stdout: stdout
+      )
+    )
+  end
+
   let(:raw_expectations) do
     [
       {
         receiver:  world,
-        selector:  :capture_stdout,
+        selector:  :capture_command,
         arguments: [%w[git rev-parse --show-toplevel]],
-        reaction:  { return: Mutant::Either::Right.new("/foo\n") }
+        reaction:  { return: mk_command_status_result("/foo\n") }
       },
       {
         receiver:  world,
-        selector:  :capture_stdout,
+        selector:  :capture_command,
         arguments: [%w[git diff-index to_rev]],
-        reaction:  { return: Mutant::Either::Right.new(index_stdout) }
+        reaction:  { return: mk_command_status_result(index_stdout) }
       },
       *file_diff_expectations
     ]
@@ -83,9 +92,9 @@ def apply
         [
           {
             receiver:  world,
-            selector:  :capture_stdout,
+            selector:  :capture_command,
             arguments: [%w[git diff --unified=0 to_rev -- /foo/bar.rb]],
-            reaction:  { return: Mutant::Either::Right.new(diff_stdout) }
+            reaction:  { return: mk_command_status_result(diff_stdout) }
           }
         ]
       end

spec/unit/mutant/world_spec.rb

@@ -54,15 +54,24 @@ def apply
     end
   end
 
-  describe '#capture_stdout' do
+  describe '#capture_command' do
     def apply
-      subject.capture_stdout(command)
+      subject.capture_command(command)
     end
 
-    let(:open3)          { class_double(Open3)              }
-    let(:stdout)         { instance_double(String, :stdout) }
-    let(:subject)        { super().with(open3: open3)       }
-    let(:command)        { %w[foo bar baz]                  }
+    let(:open3)   { class_double(Open3)              }
+    let(:stdout)  { instance_double(String, :stdout) }
+    let(:stderr)  { instance_double(String, :stderr) }
+    let(:subject) { super().with(open3: open3)       }
+    let(:command) { %w[foo bar baz]                  }
+
+    let(:command_status) do
+      described_class::CommandStatus.new(
+        process_status: process_status,
+        stderr:         stderr,
+        stdout:         stdout
+      )
+    end
 
     let(:process_status) do
       instance_double(
@@ -72,22 +81,22 @@ def apply
     end
 
     before do
-      allow(open3).to receive_messages(capture2: [stdout, process_status])
+      allow(open3).to receive_messages(capture3: [stdout, stderr, process_status])
     end
 
     context 'when process exists successful' do
       let(:success?) { true }
 
       it 'returns stdout' do
-        expect(apply).to eql(Mutant::Either::Right.new(stdout))
+        expect(apply).to eql(right(command_status))
       end
     end
 
     context 'when process exists unsuccessful' do
       let(:success?) { false }
 
       it 'returns stdout' do
-        expect(apply).to eql(Mutant::Either::Left.new("Command #{command.inspect} failed!"))
+        expect(apply).to eql(Mutant::Either::Left.new(command_status))
       end
     end
   end