Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call super in Formatter.close methods to ensure sync is reset #3094

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
15 changes: 13 additions & 2 deletions lib/rspec/core/formatters/base_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class BaseFormatter
def initialize(output)
@output = output || StringIO.new
@example_group = nil
@sync_started = false
end

# @api public
Expand Down Expand Up @@ -54,16 +55,26 @@ def close(_notification)
private

def start_sync_output
@old_sync, output.sync = output.sync, true if output_supports_sync
if output_supports_sync
@sync_started = true
@old_sync, output.sync = output.sync, true
end
end

def restore_sync_output
output.sync = @old_sync if output_supports_sync && !output.closed?
if sync_started?
@sync_started = false
output.sync = @old_sync unless output.closed?
end
end

def output_supports_sync
output.respond_to?(:sync=)
end

def sync_started?
@sync_started
end
end
end
end
Expand Down
10 changes: 5 additions & 5 deletions lib/rspec/core/formatters/base_text_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ def seed(notification)
#
# @param _notification [NullNotification] (Ignored)
def close(_notification)
return if output.closed?

output.puts

output.flush
unless output.closed?
output.puts
output.flush
end
super
rahearn marked this conversation as resolved.
Show resolved Hide resolved
end
end
end
Expand Down
1 change: 1 addition & 0 deletions lib/rspec/core/formatters/json_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def seed(notification)

def close(_notification)
output.write @output_hash.to_json
super
end

def dump_profile(profile)
Expand Down
2 changes: 2 additions & 0 deletions spec/rspec/core/formatters/base_text_formatter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
output_to_close.close unless output_to_close.closed?
end

it_behaves_like "formatter stream reset sync on close"

it 'does not error on an already closed output stream' do
output_to_close.close

Expand Down
14 changes: 14 additions & 0 deletions spec/rspec/core/formatters/json_formatter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,20 @@
formatter.close(RSpec::Core::Notifications::NullNotification)
expect(formatter_output.closed?).to be(false)
end

context "file output", :isolated_directory => true do
# Use a File output because StringIO.sync is always true
let(:output_to_close) { File.new("./output_to_close", "w") }
let(:formatter) { described_class.new(output_to_close) }

after do
# Windows appears to not let the `:isolated_directory` shared group
# cleanup if the file isn't closed.
output_to_close.close unless output_to_close.closed?
end

it_behaves_like "formatter stream reset sync on close"
end
end

describe "#message" do
Expand Down
13 changes: 13 additions & 0 deletions spec/support/shared_example_groups.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,16 @@
expect(loaded_files).to contain_files("spec/lib/DD/dd_foo_spec.rb")
end
end

RSpec.shared_examples_for "formatter stream reset sync on close" do
before do
# Call `formatter.start` in order to initialize `@old_sync` before using in `BaseFormatter#close`
formatter.start(RSpec::Core::Notifications::StartNotification.new({:count => 1}))
end

it "restores the output's sync setting" do
expect {
formatter.close(RSpec::Core::Notifications::NullNotification)
}.to change(output_to_close, :sync).from(true).to(false)
end
end