From fd3c87d938c7738ee380d4c09f06accd39c35980 Mon Sep 17 00:00:00 2001 From: Markus Schirp Date: Mon, 8 Apr 2024 02:09:28 +0000 Subject: [PATCH] Change to improved command capture --- lib/mutant/license/subscription/commercial.rb | 8 ++-- lib/mutant/license/subscription/repository.rb | 4 +- lib/mutant/repository/diff.rb | 16 +++---- lib/mutant/world.rb | 22 ++++++---- spec/unit/mutant/license/repository_spec.rb | 29 ++++++------ spec/unit/mutant/license/subscription_spec.rb | 44 ++++++++++++++----- spec/unit/mutant/repository/diff_spec.rb | 21 ++++++--- spec/unit/mutant/world_spec.rb | 27 ++++++++---- 8 files changed, 108 insertions(+), 63 deletions(-) diff --git a/lib/mutant/license/subscription/commercial.rb b/lib/mutant/license/subscription/commercial.rb index 3fd626960..a7fa6c461 100644 --- a/lib/mutant/license/subscription/commercial.rb +++ b/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 diff --git a/lib/mutant/license/subscription/repository.rb b/lib/mutant/license/subscription/repository.rb index 880ef00dc..47d83a5cd 100644 --- a/lib/mutant/license/subscription/repository.rb +++ b/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) diff --git a/lib/mutant/repository/diff.rb b/lib/mutant/repository/diff.rb index 0e8e53426..4dc68344a 100644 --- a/lib/mutant/repository/diff.rb +++ b/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 diff --git a/lib/mutant/world.rb b/lib/mutant/world.rb index 80c18eb0e..14f0f1c34 100644 --- a/lib/mutant/world.rb +++ b/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] command # - # @return [Either] - def capture_stdout(command) - stdout, status = open3.capture2(*command, binmode: true) + # @return [Either] + 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 diff --git a/spec/unit/mutant/license/repository_spec.rb b/spec/unit/mutant/license/repository_spec.rb index 9803a9dd0..ac9a5c138 100644 --- a/spec/unit/mutant/license/repository_spec.rb +++ b/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\tgit@github.com:mbj/Mutant (fetch) origin\tgit@github.com: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://git@github.com/mbj/mutant (fetch) origin\tssh://git@github.com/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://git@github.com/mbj/mutant.git (fetch) origin\tssh://git@github.com/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 diff --git a/spec/unit/mutant/license/subscription_spec.rb b/spec/unit/mutant/license/subscription_spec.rb index ce014f472..f46818a5b 100644 --- a/spec/unit/mutant/license/subscription_spec.rb +++ b/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\tgit@github.com:mbj/mutant (fetch) origin\tgit@github.com: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\tgit@github.com:mbj/mutant (fetch) origin\tgit@github.com: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\tgit@github.com:mbj/Mutant (fetch) origin\tgit@github.com: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\tgit@github.com:mbj/mutant (fetch) origin\tgit@github.com:mbj/mutant (push) @@ -268,10 +276,24 @@ def self.it_fails_with_message(expected) end shared_examples 'individual licenses' do - let(:git_config_author) { "customer-a@example.com\n" } - let(:git_config_result) { Mutant::Either::Right.new(git_config_author) } - let(:git_show_author) { "customer-b@example.com\n" } - let(:git_show_result) { Mutant::Either::Right.new(git_show_author) } + let(:git_config_author) { "customer-a@example.com\n" } + let(:git_config_result) { Mutant::Either::Right.new(git_config_author_status) } + let(:git_show_author) { "customer-b@example.com\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[ diff --git a/spec/unit/mutant/repository/diff_spec.rb b/spec/unit/mutant/repository/diff_spec.rb index 79114936a..192e0a26a 100644 --- a/spec/unit/mutant/repository/diff_spec.rb +++ b/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 diff --git a/spec/unit/mutant/world_spec.rb b/spec/unit/mutant/world_spec.rb index 65eaaec1a..717ce8974 100644 --- a/spec/unit/mutant/world_spec.rb +++ b/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,14 +81,14 @@ 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 @@ -87,7 +96,7 @@ def apply 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