Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/overcommit/hook/pre_push/r_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Overcommit::Hook::PrePush
# @see http://rspec.info/
class RSpec < Base
def run
result = execute(command)
result = execute(command, live_output: config['live_output'])
return :pass if result.success?

output = result.stdout + result.stderr
Expand Down
23 changes: 17 additions & 6 deletions lib/overcommit/subprocess.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def spawn(args, options = {})

process = ChildProcess.build(*args)

out, err = assign_output_streams(process)
out, err = assign_output_streams(process, options)

process.duplex = true if options[:input] # Make stdin available if needed
process.start
Expand All @@ -48,10 +48,7 @@ def spawn(args, options = {})
end
process.wait

err.rewind
out.rewind

Result.new(process.exit_code, out.read, err.read)
build_result(process, out, err, options)
end

# Spawns a new process in the background using the given array of
Expand Down Expand Up @@ -85,14 +82,28 @@ def win32_prepare_args(args)

# @param process [ChildProcess]
# @return [Array<IO>]
def assign_output_streams(process)
def assign_output_streams(process, options = {})
if options[:live_output]
process.io.inherit!
return []
end

%w[out err].map do |stream_name|
::Tempfile.new(stream_name).tap do |stream|
stream.sync = true
process.io.send("std#{stream_name}=", stream)
end
end
end

def build_result(process, out, err, options = {})
return Result.new(process.exit_code, '', '') if options[:live_output]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can potentially break any processing done by the hook. If the exit_code is non-zero, hooks can sometimes read the stderr/stdout to determine the kind of failure that occurred. Given all the different ways Overcommit hooks could be used, we can't introduce a change that fundamentally breaks the behavior.


err.rewind
out.rewind

Result.new(process.exit_code, out.read, err.read)
end
end
end
end
43 changes: 43 additions & 0 deletions spec/overcommit/hook/pre_push/r_spec_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,47 @@
it { should fail_hook }
end
end

context 'when live_output option is true' do
let(:config) do
Overcommit::ConfigurationLoader.default_configuration.merge(
Overcommit::Configuration.new(
'PrePush' => {
'RSpec' => {
'live_output' => true
}
}
)
)
end
let(:result) { double('result') }

before do
result.stub(:success?).and_return(true)
end

it 'calls execute with the live_output flag set' do
expect(subject).to receive(:execute).with(['rspec'], live_output: true).and_return(result)
expect(subject).to pass
end

context do
let(:child_process) { double('child process') }
let(:io) { double('io') }

it 'the child process inherits the io' do
io.stub(:inherit!)
child_process.stub(io: io, start: nil, wait: nil, exit_code: 0)
expect(ChildProcess).to receive(:build).with('rspec').and_return(child_process)
expect(subject).to pass
end

it 'the child process inherits the io' do
io.stub(:inherit!)
child_process.stub(io: io, start: nil, wait: nil, exit_code: 1)
expect(ChildProcess).to receive(:build).with('rspec').and_return(child_process)
expect(subject).to fail_hook
end
end
end
end