Skip to content

Commit 041229b

Browse files
solnicclaude
andcommitted
refactor(test_helper): use public Hub#clients instead of @stack ivar (#2951)
Address code review feedback on the #2951 fix: - Add a documented public Sentry::Hub#clients accessor returning all clients across the scope stack, and use it from TestHelper#sentry_test_transports instead of reaching into Hub's private @stack ivar. This keeps the test helper resilient to future Hub internals refactors. Covered by new hub_spec.rb examples. - Replace the final inline teardown_sentry_test in the #2951 regression and Rack consecutive-requests describe blocks with an unconditional 'after { teardown_sentry_test }' hook, matching the sibling describe style so cleanup runs even if an expectation fails mid-example. The intentional mid-scenario teardowns remain inline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 2d57269 commit 041229b

4 files changed

Lines changed: 25 additions & 5 deletions

File tree

sentry-ruby/lib/sentry/hub.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ def current_client
5454
current_layer&.client
5555
end
5656

57+
# All clients bound across the hub's scope stack, base layer first.
58+
# @return [Array<Client>]
59+
def clients
60+
@stack.map(&:client).compact
61+
end
62+
5763
def configuration
5864
current_client.configuration
5965
end

sentry-ruby/lib/sentry/test_helper.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ def sentry_transport
112112
# @return [Array<Transport>]
113113
def sentry_test_transports
114114
[Sentry.get_current_hub, Sentry.get_main_hub].compact.uniq.flat_map do |hub|
115-
hub.instance_variable_get(:@stack).map { |layer| layer.client&.transport }
115+
hub.clients.map(&:transport)
116116
end.compact.uniq
117117
end
118118

sentry-ruby/spec/sentry/hub_spec.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,20 @@
537537
end
538538
end
539539

540+
describe "#clients" do
541+
it "returns the only client for a single-layer hub" do
542+
expect(subject.clients).to eq([client])
543+
end
544+
545+
it "returns every client across the scope stack, base layer first" do
546+
new_client = Sentry::Client.new(configuration)
547+
subject.push_scope
548+
subject.bind_client(new_client)
549+
550+
expect(subject.clients).to eq([client, new_client])
551+
end
552+
end
553+
540554
describe "#pop_scope" do
541555
it "pops the current scope" do
542556
prev_scope = subject.current_scope

sentry-ruby/spec/sentry/test_helper_spec.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@
9393
end
9494

9595
describe "event leakage across clone_hub_to_current_thread (regression for #2951)" do
96+
after { teardown_sentry_test }
97+
9698
it "keeps sentry_events empty after setup_sentry_test even when an earlier request captured events through a cloned hub" do
9799
# Cycle 1: a normal test that uses the test helper.
98100
setup_sentry_test
@@ -114,8 +116,6 @@
114116
Sentry.clone_hub_to_current_thread
115117

116118
expect(sentry_events).to be_empty
117-
118-
teardown_sentry_test
119119
end
120120
end
121121

@@ -135,6 +135,8 @@
135135
end
136136

137137
describe "Sentry::Rack::CaptureExceptions across consecutive requests (regression for #2951)", when: :rack_available? do
138+
after { teardown_sentry_test }
139+
138140
# Drives a single request through the real Rack middleware. The middleware
139141
# calls Sentry.clone_hub_to_current_thread before handing off to the app,
140142
# exactly like a Rails request spec would.
@@ -171,8 +173,6 @@ def captured_exception_messages
171173
expect(messages.size).to eq(1)
172174
expect(messages.first).to include("second-request")
173175
expect(messages).not_to include(a_string_including("first-request"))
174-
175-
teardown_sentry_test
176176
end
177177
end
178178

0 commit comments

Comments
 (0)