Skip to content

Commit 7ea2ec1

Browse files
authored
Merge pull request rails#53819 from larouxn/notification_assertions_actionpack
Migrate applicable `actionpack` tests to use `NotificationAssertions`
2 parents 9499632 + e1bed25 commit 7ea2ec1

File tree

7 files changed

+32
-70
lines changed

7 files changed

+32
-70
lines changed

actionpack/test/controller/allow_browser_test.rb

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ class AllowBrowserTest < ActionController::TestCase
8484
end
8585

8686
test "a blocked request instruments a browser_block.action_controller event" do
87-
event, *rest = capture_instrumentation_events "browser_block.action_controller" do
87+
event, *rest = capture_notifications "browser_block.action_controller" do
8888
get_with_agent :modern, CHROME_118
8989
end
9090

@@ -98,10 +98,4 @@ def get_with_agent(action, agent)
9898
@request.headers["User-Agent"] = agent
9999
get action
100100
end
101-
102-
def capture_instrumentation_events(pattern, &block)
103-
events = []
104-
ActiveSupport::Notifications.subscribed(->(e) { events << e }, pattern, &block)
105-
events
106-
end
107101
end

actionpack/test/controller/caching_test.rb

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -255,15 +255,9 @@ def test_render_inline_before_fragment_caching
255255
end
256256

257257
def test_fragment_cache_instrumentation
258-
payload = nil
259-
260-
subscriber = proc do |event|
261-
payload = event.payload
262-
end
263-
264-
ActiveSupport::Notifications.subscribed(subscriber, "read_fragment.action_controller") do
258+
payload = capture_notifications("read_fragment.action_controller") do
265259
get :inline_fragment_cached
266-
end
260+
end.first.payload
267261

268262
assert_equal "functional_caching", payload[:controller]
269263
assert_equal "inline_fragment_cached", payload[:action]

actionpack/test/controller/live_stream_test.rb

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -388,16 +388,15 @@ def test_send_stream
388388
end
389389

390390
def test_send_stream_instrumentation
391-
payload = nil
392-
subscriber = proc { |event| payload = event.payload }
391+
expected_payload = {
392+
filename: "sample.csv",
393+
disposition: "attachment",
394+
type: "text/csv"
395+
}
393396

394-
ActiveSupport::Notifications.subscribed(subscriber, "send_stream.action_controller") do
397+
assert_notification("send_stream.action_controller", expected_payload) do
395398
get :send_stream_with_explicit_content_type
396399
end
397-
398-
assert_equal "sample.csv", payload[:filename]
399-
assert_equal "attachment", payload[:disposition]
400-
assert_equal "text/csv", payload[:type]
401400
end
402401

403402
def test_send_stream_with_options

actionpack/test/controller/redirect_test.rb

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -608,15 +608,9 @@ def test_url_from_fallback
608608
end
609609

610610
def test_redirect_to_instrumentation
611-
payload = nil
612-
613-
subscriber = proc do |event|
614-
payload = event.payload
615-
end
616-
617-
ActiveSupport::Notifications.subscribed(subscriber, "redirect_to.action_controller") do
611+
payload = capture_notifications("redirect_to.action_controller") do
618612
get :simple_redirect
619-
end
613+
end.first.payload
620614

621615
assert_equal request, payload[:request]
622616
assert_equal 302, payload[:status]

actionpack/test/controller/send_file_test.rb

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -209,33 +209,18 @@ def test_send_file_from_before_action
209209

210210
def test_send_file_instrumentation
211211
@controller.options = { disposition: :inline }
212-
payload = nil
213212

214-
subscriber = proc do |event|
215-
payload = event.payload
216-
end
217-
218-
ActiveSupport::Notifications.subscribed(subscriber, "send_file.action_controller") do
213+
assert_notification("send_file.action_controller", path: __FILE__, disposition: :inline) do
219214
process("file")
220215
end
221-
222-
assert_equal __FILE__, payload[:path]
223-
assert_equal :inline, payload[:disposition]
224216
end
225217

226218
def test_send_data_instrumentation
227219
@controller.options = { content_type: "application/x-ruby" }
228-
payload = nil
229220

230-
subscriber = proc do |event|
231-
payload = event.payload
232-
end
233-
234-
ActiveSupport::Notifications.subscribed(subscriber, "send_data.action_controller") do
221+
assert_notification("send_data.action_controller", content_type: "application/x-ruby") do
235222
process("data")
236223
end
237-
238-
assert_equal("application/x-ruby", payload[:content_type])
239224
end
240225

241226
%w(file data).each do |method|

actionpack/test/dispatch/middleware_stack_test.rb

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -193,23 +193,22 @@ def test_delete_works
193193
end
194194

195195
test "instruments the execution of middlewares" do
196-
events = []
197-
198-
subscriber = proc { |event| events << event }
199-
200-
ActiveSupport::Notifications.subscribed(subscriber, "process_middleware.action_dispatch") do
201-
app = Rack::Lint.new(
202-
@stack.build(Rack::Lint.new(proc { |env| [200, {}, []] }))
203-
)
204-
205-
env = Rack::MockRequest.env_for("", {})
206-
assert_nothing_raised do
207-
app.call(env)
196+
notification_name = "process_middleware.action_dispatch"
197+
198+
assert_notifications_count(notification_name, 2) do
199+
assert_notification(notification_name, { middleware: "MiddlewareStackTest::BarMiddleware" }) do
200+
assert_notification(notification_name, { middleware: "MiddlewareStackTest::FooMiddleware" }) do
201+
app = Rack::Lint.new(
202+
@stack.build(Rack::Lint.new(proc { |env| [200, {}, []] }))
203+
)
204+
205+
env = Rack::MockRequest.env_for("", {})
206+
assert_nothing_raised do
207+
app.call(env)
208+
end
209+
end
208210
end
209211
end
210-
211-
assert_equal 2, events.count
212-
assert_equal ["MiddlewareStackTest::BarMiddleware", "MiddlewareStackTest::FooMiddleware"], events.map { |e| e.payload[:middleware] }
213212
end
214213

215214
test "includes a middleware" do

actionpack/test/dispatch/routing/instrumentation_test.rb

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ class RoutingInstrumentationTest < ActionDispatch::IntegrationTest
88
get "redirect", to: redirect("/login")
99
end
1010

11-
event = subscribed("redirect.action_dispatch") { get "/redirect" }
11+
event = capture_notifications("redirect.action_dispatch") do
12+
assert_notifications_count("redirect.action_dispatch", 1) do
13+
get "/redirect"
14+
end
15+
end.first
1216

1317
assert_equal 301, event.payload[:status]
1418
assert_equal "http://www.example.com/login", event.payload[:location]
@@ -23,11 +27,4 @@ def draw(&block)
2327
@app = RoutedRackApp.new routes
2428
end
2529
end
26-
27-
def subscribed(event_pattern, &block)
28-
event = nil
29-
subscriber = -> (_event) { event = _event }
30-
ActiveSupport::Notifications.subscribed(subscriber, event_pattern, &block)
31-
event
32-
end
3330
end

0 commit comments

Comments
 (0)