Skip to content

Avoid mutating global STDOUT & STDERR (#1837) #1

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.

Sign up for GitHub

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

Open
MitchLewis930 wants to merge 1 commit into
base: pr_051_before
Choose a base branch
from
Open

Avoid mutating global STDOUT & STDERR (#1837) #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions lib/puma/events.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ def call(str)
#
def initialize(stdout, stderr)
@formatter = DefaultFormatter.new
@stdout = stdout
@stderr = stderr
@stdout = stdout.dup
@stderr = stderr.dup

@stdout.sync = true
@stderr.sync = true
Expand Down
14 changes: 11 additions & 3 deletions test/test_events.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ def test_null

assert_instance_of Puma::NullIO, events.stdout
assert_instance_of Puma::NullIO, events.stderr
assert_equal events.stdout, events.stderr
end

def test_strings
Expand All @@ -19,8 +18,17 @@ def test_strings
def test_stdio
events = Puma::Events.stdio

assert_equal STDOUT, events.stdout
assert_equal STDERR, events.stderr
# events.stdout is a dup, so same file handle, different ruby object, but inspect should show the same file handle
assert_equal STDOUT.inspect, events.stdout.inspect
assert_equal STDERR.inspect, events.stderr.inspect
end

def test_stdio_respects_sync
STDOUT.sync = false
events = Puma::Events.stdio

assert !STDOUT.sync
assert events.stdout.sync
Comment on lines +26 to +31
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

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

This test mutates the global STDOUT object by setting sync to false, which could affect other tests. The test should restore the original sync value in an ensure block to prevent side effects on other tests.

Suggested change
def test_stdio_respects_sync
STDOUT.sync = false
events = Puma::Events.stdio
assert !STDOUT.sync
assert events.stdout.sync
def test_stdio_respects_sync
original_sync = STDOUT.sync
STDOUT.sync = false
events = Puma::Events.stdio
assert !STDOUT.sync
assert events.stdout.sync
ensure
STDOUT.sync = original_sync

Copilot uses AI. Check for mistakes.
end

def test_register_callback_with_block
Expand Down
1 change: 1 addition & 0 deletions test/test_tcp_logger.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def test_events
# in lib/puma/launcher.rb:85
# Puma::Events is default tcp_logger for cluster mode
logger = Puma::Events.new(STDOUT, STDERR)
logger.instance_variable_set(:@stdout, $stdout) # ensure capture_process_io has access to the loggers output
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

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

Corrected spelling of 'loggers' to 'logger's'.

Suggested change
logger.instance_variable_set(:@stdout, $stdout) # ensure capture_process_io has access to the loggers output
logger.instance_variable_set(:@stdout, $stdout) # ensure capture_process_io has access to the logger's output

Copilot uses AI. Check for mistakes.
out, err = capture_subprocess_io do
Puma::TCPLogger.new(logger, @server.app).call({}, @socket)
end
Expand Down