From fc8a00b6742aa4d51832379d6bf870d2184fe4c3 Mon Sep 17 00:00:00 2001 From: Thomas Bretzke Date: Wed, 29 Aug 2018 22:54:56 +0200 Subject: [PATCH 1/2] feat: added `capture` option and simplified spec --- .gitignore | 2 + lib/eksek_result.rb | 19 ++++++--- lib/eksekuter.rb | 71 ++++++++++++++++++++-------------- spec/eksek_result_spec.rb | 4 +- spec/eksek_spec.rb | 81 +++++++-------------------------------- 5 files changed, 72 insertions(+), 105 deletions(-) diff --git a/.gitignore b/.gitignore index b47fdd1..455f9cf 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ Gemfile.lock .idea coverage +.bundle +bin diff --git a/lib/eksek_result.rb b/lib/eksek_result.rb index e2e6043..635c657 100644 --- a/lib/eksek_result.rb +++ b/lib/eksek_result.rb @@ -6,20 +6,27 @@ # return values from a command class EksekResult # rubocop:disable Metrics/ParameterLists - def initialize env, cmd, exit_code, success, stdout, stderr + def initialize env, cmd, exit_code, out_stream, err_stream @env = env @cmd = cmd @exit_code = exit_code - @success = success - @stdout = stdout - @stderr = stderr + @out_stream = out_stream + @err_stream = err_stream end # rubocop:enable Metrics/ParameterLists - attr_reader :exit_code, :stdout, :stderr + attr_reader :exit_code + + def stdout + @stdout ||= @out_stream&.read + end + + def stderr + @stderr ||= @err_stream&.read + end def success? - @success + @exit_code.zero? end def success! diff --git a/lib/eksekuter.rb b/lib/eksekuter.rb index c23aceb..ead9840 100644 --- a/lib/eksekuter.rb +++ b/lib/eksekuter.rb @@ -9,16 +9,23 @@ class Eksekuter # @param {Logger} logger def initialize logger: nil @logger = logger + @stdout_buffer = nil + @stderr_buffer = nil + @stdin_buffer = nil end def run *args, **opts, &block env, cmd = separate_env_and_cmd(args) + # p opts + capture = opts.delete(:capture) || false + streams = get_out_err_streams(capture) + + set_streams_in_opts streams, opts + params = { env: env, cmd: cmd, opts: opts, block: block } - popen3_result = spawn_process(params) - write_and_close_stdin(params, popen3_result) - process_status = wait(popen3_result) - out_str, err_str = read_and_close_stdout_stderr(popen3_result) - assemble_result(params, process_status, out_str, err_str) + + process_status = run_process(params, streams) + assemble_result(params, process_status, streams) end private @@ -30,42 +37,48 @@ def separate_env_and_cmd args [env, cmd] end - def spawn_process params - stdin, stdout, stderr, wait_thr = Open3.popen3( - params.fetch(:env), *params.fetch(:cmd), params.fetch(:opts) - ) - { stdin: stdin, stdout: stdout, stderr: stderr, wait_thr: wait_thr } + def get_out_err_streams capture + if capture + out_readable, out_writable = IO.pipe + err_readable, err_writable = IO.pipe + return { + out: {readable: out_readable, writable: out_writable}, + err: {readable: err_readable, writable: err_writable} + } + end + + { + out: {readable: nil, writable: STDOUT}, + err: {readable: nil, writable: STDERR} + } end - def write_and_close_stdin params, popen3_result - stdin = popen3_result.fetch(:stdin) - return if stdin.closed? || params.fetch(:block).nil? - block_result = params.fetch(:block).call(stdin) - block_result = StringIO.new(block_result) if block_result.is_a? String - IO.copy_stream(block_result, stdin) if block_result.respond_to? :read - stdin.close - nil + def set_streams_in_opts streams, opts + opts[:out] = streams.fetch(:out).fetch(:writable) unless opts[:out] + opts[:err] = streams.fetch(:err).fetch(:writable) unless opts[:err] end - def wait popen3_result - popen3_result.fetch(:wait_thr).value # returns the process status + def run_process params, streams + pid = spawn(params.fetch(:env), *params.fetch(:cmd), params.fetch(:opts)) + close_streams streams + _, process_status = Process.wait2(pid) + process_status end - def read_and_close_stdout_stderr popen3_result - streams = [popen3_result.fetch(:stdout), popen3_result.fetch(:stderr)] - out_str, err_str = streams.map(&:read).map(&:chomp) - streams.each(&:close) - [out_str, err_str] + def close_streams streams + out_writable = streams.fetch(:out).fetch(:writable) + err_writable = streams.fetch(:err).fetch(:writable) + out_writable&.close if out_writable != STDOUT + err_writable&.close if err_writable != STDERR end - def assemble_result params, process_status, out_str, err_str + def assemble_result params, process_status, streams EksekResult.new( params.fetch(:env), params.fetch(:cmd), process_status.exitstatus, - process_status.success?, - out_str, - err_str, + streams.fetch(:out).fetch(:readable), + streams.fetch(:err).fetch(:readable) ) end end diff --git a/spec/eksek_result_spec.rb b/spec/eksek_result_spec.rb index 0be310b..45d44c7 100644 --- a/spec/eksek_result_spec.rb +++ b/spec/eksek_result_spec.rb @@ -4,8 +4,8 @@ RSpec.describe 'EksekResult#to_s' do it 'returns the stdout' do - command = 'echo HelloStdout && echo HelloStderr >&2' - output = "The output was: #{Eksekuter.new.run(command)}." + command = 'printf HelloStdout && printf HelloStderr >&2' + output = "The output was: #{Eksekuter.new.run(command, capture: true)}." expect(output).to eq('The output was: HelloStdout.') end end diff --git a/spec/eksek_spec.rb b/spec/eksek_spec.rb index 64e10b4..4b7e964 100644 --- a/spec/eksek_spec.rb +++ b/spec/eksek_spec.rb @@ -1,28 +1,19 @@ # frozen_string_literal: true -require 'tempfile' - require 'eksek' require 'eksekuter' -RSpec.describe 'Eksekuter#run.success?' do - it 'returns true or false' do +RSpec.describe 'Eksekuter success methods' do + it 'returns true or false depending on the exit code' do expect(Eksekuter.new.run('true').success?).to be(true) expect(Eksekuter.new.run('exit 1').success?).to be(false) end -end -RSpec.describe 'Eksekuter#run.success! and #eksek!' do - it 'fails on Eksekuter#success! and :eksek! when appropriatly' do + it 'fails when appropriate' do expect { Eksekuter.new.run('true').success! }.not_to raise_error expect { Eksekuter.new.run('exit 1').success! }.to raise_error EksekError - - expect { eksek! 'true' }.not_to raise_error - expect { eksek! 'exit 1' }.to raise_error EksekError end -end -RSpec.describe 'Eksekuter#run.exit_code' do it 'returns the exit code' do expect(Eksekuter.new.run('exit 0').exit_code).to be(0) expect(Eksekuter.new.run('exit 1').exit_code).to be(1) @@ -30,77 +21,31 @@ end end -RSpec.describe 'Eksekuter#run.stdout, Eksekuter#run.stderr' do +RSpec.describe 'Eksekuter capturing options' do it 'captures the stdout and stderr separately' do - expect(Eksekuter.new.run('echo Hello').stdout).to eq('Hello') - expect(Eksekuter.new.run('echo Hello >&2').stderr).to eq('Hello') + expect(Eksekuter.new.run('printf Hello', capture: true).stdout).to eq('Hello') + expect(Eksekuter.new.run('printf Hello >&2', capture: true).stderr).to eq('Hello') end -end -RSpec.describe 'Eksekuter#run.success! chaining' do - it 'lets you combine EksekResult#run.success! and stdout/stderr/exit_code' do - expect(Eksekuter.new.run('echo Hello').success!.stdout).to eq('Hello') - expect(Eksekuter.new.run('echo Hello >&2').success!.stderr).to eq('Hello') - expect(Eksekuter.new.run('exit 0').success!.exit_code).to be(0) - expect { Eksekuter.new.run('exit 1').success!.stdout } - .to raise_error EksekError - end -end - -RSpec.describe 'Standard input' do - it 'accepts a block where the stdin can be written to' do - result = Eksekuter.new.run('read A B; echo $A, $B') do |i| - i.write('Hi world') - end - expect(result.stdout).to eq('Hi, world') - - result = eksek('read A B; echo $A, $B') do |i| - i.write('Hi world') - end - expect(result.stdout).to eq('Hi, world') - end - - it 'reads a String that the block returns' do - result = Eksekuter.new.run('read A; echo $A') { 'Hello' } - expect(result.stdout).to eq('Hello') - end - - it 'reads an IO that the block returns' do - file = Tempfile.open - file.write('Hello') - file.close - - File.open(file.path) do |f| - result = Eksekuter.new.run('read A; echo $A!!!') { f } - expect(result.stdout).to eq('Hello!!!') - end - - file.unlink + it 'outputs to stdout/stderr when disabling I/O capturing' do + expect { Eksekuter.new.run('printf Hello', capture: false) }.to output('Hello').to_stdout_from_any_process + expect { Eksekuter.new.run('printf Hello >&2', capture: false) }.to output('Hello').to_stderr_from_any_process end end RSpec.describe 'Kernel#spawn-style parameters' do it 'accepts a Hash as an optional first parameter' do - result = Eksekuter.new.run({ 'TEXT' => 'Hello' }, 'echo $TEXT') - expect(result.stdout).to eq('Hello') - - result = eksek({ 'TEXT' => 'Hello' }, 'echo $TEXT') + result = Eksekuter.new.run({ 'TEXT' => 'Hello' }, 'printf $TEXT', capture: true) expect(result.stdout).to eq('Hello') end it 'stringifies the keys of the environment' do - result = Eksekuter.new.run({ TEXT: 'Hello' }, 'echo $TEXT') - expect(result.stdout).to eq('Hello') - - result = eksek({ TEXT: 'Hello' }, 'echo $TEXT') + result = Eksekuter.new.run({ TEXT: 'Hello' }, 'printf $TEXT', capture: true) expect(result.stdout).to eq('Hello') end it 'accepts a variable-length parameter list as command' do - result = Eksekuter.new.run('echo', 'Hello', 'World') - expect(result.stdout).to eq('Hello World') - - result = eksek('echo', 'Hello', 'World') - expect(result.stdout).to eq('Hello World') + result = Eksekuter.new.run('echo', 'Hello', 'World', capture: true) + expect(result.stdout).to eq("Hello World\n") end end From ad16c049316f7c9614d5395b44af2c36b8449cb7 Mon Sep 17 00:00:00 2001 From: Thomas Bretzke Date: Tue, 13 Nov 2018 19:36:51 +0100 Subject: [PATCH 2/2] style: Fix style issues and adapt rubocop settings --- .rubocop.yml | 4 ++++ lib/eksek_result.rb | 1 - lib/eksekuter.rb | 10 +++++----- spec/eksek_spec.rb | 21 ++++++++++++++------- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index b9121cd..62095c5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -8,3 +8,7 @@ Style/TrailingCommaInArrayLiteral: EnforcedStyleForMultiline: comma Style/TrailingCommaInHashLiteral: EnforcedStyleForMultiline: comma +Metrics/MethodLength: + Max: 15 +Layout/MultilineMethodCallIndentation: + EnforcedStyle: indented diff --git a/lib/eksek_result.rb b/lib/eksek_result.rb index 635c657..8e2c312 100644 --- a/lib/eksek_result.rb +++ b/lib/eksek_result.rb @@ -5,7 +5,6 @@ # Describes a result object to be used for evaluating # return values from a command class EksekResult - # rubocop:disable Metrics/ParameterLists def initialize env, cmd, exit_code, out_stream, err_stream @env = env @cmd = cmd diff --git a/lib/eksekuter.rb b/lib/eksekuter.rb index ead9840..d9e6736 100644 --- a/lib/eksekuter.rb +++ b/lib/eksekuter.rb @@ -42,14 +42,14 @@ def get_out_err_streams capture out_readable, out_writable = IO.pipe err_readable, err_writable = IO.pipe return { - out: {readable: out_readable, writable: out_writable}, - err: {readable: err_readable, writable: err_writable} + out: { readable: out_readable, writable: out_writable }, + err: { readable: err_readable, writable: err_writable } } end { - out: {readable: nil, writable: STDOUT}, - err: {readable: nil, writable: STDERR} + out: { readable: nil, writable: STDOUT }, + err: { readable: nil, writable: STDERR } } end @@ -78,7 +78,7 @@ def assemble_result params, process_status, streams params.fetch(:cmd), process_status.exitstatus, streams.fetch(:out).fetch(:readable), - streams.fetch(:err).fetch(:readable) + streams.fetch(:err).fetch(:readable), ) end end diff --git a/spec/eksek_spec.rb b/spec/eksek_spec.rb index 4b7e964..dc64a6d 100644 --- a/spec/eksek_spec.rb +++ b/spec/eksek_spec.rb @@ -23,29 +23,36 @@ RSpec.describe 'Eksekuter capturing options' do it 'captures the stdout and stderr separately' do - expect(Eksekuter.new.run('printf Hello', capture: true).stdout).to eq('Hello') - expect(Eksekuter.new.run('printf Hello >&2', capture: true).stderr).to eq('Hello') + expect(Eksekuter.new.run('printf Hello', capture: true).stdout) + .to eq('Hello') + expect(Eksekuter.new.run('printf Hello >&2', capture: true).stderr) + .to eq('Hello') end it 'outputs to stdout/stderr when disabling I/O capturing' do - expect { Eksekuter.new.run('printf Hello', capture: false) }.to output('Hello').to_stdout_from_any_process - expect { Eksekuter.new.run('printf Hello >&2', capture: false) }.to output('Hello').to_stderr_from_any_process + expect { Eksekuter.new.run('printf Hello', capture: false) } + .to output('Hello').to_stdout_from_any_process + expect { Eksekuter.new.run('printf Hello >&2', capture: false) } + .to output('Hello').to_stderr_from_any_process end end RSpec.describe 'Kernel#spawn-style parameters' do it 'accepts a Hash as an optional first parameter' do - result = Eksekuter.new.run({ 'TEXT' => 'Hello' }, 'printf $TEXT', capture: true) + result = Eksekuter.new + .run({ 'TEXT' => 'Hello' }, 'printf $TEXT', capture: true) expect(result.stdout).to eq('Hello') end it 'stringifies the keys of the environment' do - result = Eksekuter.new.run({ TEXT: 'Hello' }, 'printf $TEXT', capture: true) + result = Eksekuter.new + .run({ TEXT: 'Hello' }, 'printf $TEXT', capture: true) expect(result.stdout).to eq('Hello') end it 'accepts a variable-length parameter list as command' do - result = Eksekuter.new.run('echo', 'Hello', 'World', capture: true) + result = Eksekuter.new + .run('echo', 'Hello', 'World', capture: true) expect(result.stdout).to eq("Hello World\n") end end