diff --git a/test/test_application_n_plus_one.rb b/test/test_application_n_plus_one.rb new file mode 100644 index 0000000..13813e3 --- /dev/null +++ b/test/test_application_n_plus_one.rb @@ -0,0 +1,91 @@ +require 'minitest/autorun' +require 'prism' +require 'tmpdir' +require 'stringio' +require 'support' + +# Regression coverage for the real bug found while wiring raise_on_violation +# into examples/blog: Application#dispatch used to call +# Orm::NPlusOneDetector.report! AFTER the instrumented span's rescue reach had +# already closed, so a violation blew straight out of #dispatch (and, over a +# real socket, out of Server#serve too - server.rb only rescues EPIPE/ +# ECONNRESET/IOError around @app.call, not StandardError) instead of getting +# the same 500 treatment every other uncaught exception in this app gets. +# These tests go through Application.call end-to-end (not +# NPlusOneDetector.report! directly, which test_orm_n_plus_one_detector.rb +# already covers in isolation) to prove the fix at the layer that matters. +class ApplicationNPlusOneTest < Minitest::Test + include TestSupport + + def setup + @dir = Dir.mktmpdir('prism-app-n-plus-one') + @store = Prism::Orm::Store.new(File.join(@dir, 'test.pstore')) + @store.create_table(:authors, %w[id name]) + @store.create_table(:posts, %w[id title author_id]) + + author_class = Class.new(Prism::Orm::Model) do + table_name :authors + has_many :posts, class_name: 'NPlusOnePost', foreign_key: 'author_id' + end + post_class = Class.new(Prism::Orm::Model) do + table_name :posts + belongs_to :author, class_name: 'NPlusOneAuthor', foreign_key: 'author_id' + end + [author_class, post_class].each { |klass| klass.store = @store } + Object.const_set(:NPlusOneAuthor, author_class) + Object.const_set(:NPlusOnePost, post_class) + + 3.times do |i| + author = NPlusOneAuthor.create!(name: "author#{i}") + NPlusOnePost.create!(title: "post#{i}", author_id: author.id) + end + + @io = StringIO.new + author = NPlusOneAuthor + app = Class.new(Prism::Application) do + get('/n_plus_one') do + author.all.to_a.each { |a| a.posts.to_a } + 'rendered' + end + end + @app = app + end + + def teardown + Prism::Orm::NPlusOneDetector.disable! + Prism::Orm::NPlusOneDetector.reset! + FileUtils.remove_entry(@dir) + %i[NPlusOneAuthor NPlusOnePost].each { |name| Object.send(:remove_const, name) if Object.const_defined?(name) } + end + + def test_log_only_mode_returns_a_normal_response_and_warns_to_io + Prism::Orm::NPlusOneDetector.enable!(threshold: 2, io: @io, raise_on_violation: false) + + status, _headers, body = @app.call(env_for('GET', '/n_plus_one')) + + assert_equal 200, status + assert_equal 'rendered', body.join + assert_match(/N\+1 query detected: NPlusOneAuthor#posts/, @io.string) + end + + def test_raise_on_violation_turns_into_a_real_500_response_not_an_escaped_exception + Prism::Orm::NPlusOneDetector.enable!(threshold: 2, io: @io, raise_on_violation: true) + + status, headers, body = @app.call(env_for('GET', '/n_plus_one')) + + assert_equal 500, status + assert_equal 'text/plain', headers['Content-Type'] + assert_equal 'Internal Server Error', body.join + assert_match(/N\+1 query detected: NPlusOneAuthor#posts/, @io.string) + end + + def test_raise_on_violation_below_threshold_still_returns_the_normal_response + Prism::Orm::NPlusOneDetector.enable!(threshold: 10, io: @io, raise_on_violation: true) + + status, _headers, body = @app.call(env_for('GET', '/n_plus_one')) + + assert_equal 200, status + assert_equal 'rendered', body.join + assert_empty @io.string + end +end diff --git a/test/test_application_routing.rb b/test/test_application_routing.rb index 810a272..19bbd90 100644 --- a/test/test_application_routing.rb +++ b/test/test_application_routing.rb @@ -8,11 +8,16 @@ class ResourceApp < Prism::Application root(to: 'welcome#show') resources :routing_posts, only: %i[index show create] + resources :routing_comments, only: %i[destroy] get '/whoami' do json(path: url_for(:routing_post, id: 5)) end + get '/destroy-link' do + json(path: url_for(:routing_comment, id: 7)) + end + get '/write-flash' do flash[:notice] = 'saved!' redirect '/after-flash' @@ -43,6 +48,12 @@ class RoutingPostsController < Prism::Controller end end +class RoutingCommentsController < Prism::Controller + def destroy + json(action: 'destroy', id: params['id']) + end +end + class ApplicationRoutingTest < Minitest::Test include TestSupport @@ -81,6 +92,20 @@ class ApplicationRoutingTest < Minitest::Test assert_equal '/routing_posts/5', JSON.parse(body.join)['path'] end + def test_resources_destroy_dispatches + _, _, body = call('DELETE', '/routing_comments/3') + assert_equal({ 'action' => 'destroy', 'id' => '3' }, JSON.parse(body.join)) + end + + # Regression: add_resource_route used to skip `as:` for :destroy, so a + # resource declared with only: [:destroy] (no :show alongside it) had no + # named route at all - url_for/link_to/button_to for a delete link had no + # path to build from. + def test_url_for_named_resource_route_for_destroy_only_resource + _, _, body = call('GET', '/destroy-link') + assert_equal '/routing_comments/7', JSON.parse(body.join)['path'] + end + def test_flash_survives_a_redirect status, headers, = call('GET', '/write-flash') assert_equal 302, status @@ -97,4 +122,19 @@ class ApplicationRoutingTest < Minitest::Test _, _, body = call('GET', '/after-flash', cookies: cookie2) assert_nil JSON.parse(body.join)['notice'] end + + def test_controller_dispatch_enriches_process_action_payload + events = [] + subscriber = Prism::Instrumentation.subscribe('process_action.prism') { |event| events << event } + begin + call('GET', '/routing_posts/9') + ensure + Prism::Instrumentation.unsubscribe(subscriber) + end + + assert_equal 1, events.size + assert_equal 'routing_posts', events.first.payload[:controller] + assert_equal 'show', events.first.payload[:action] + assert_equal 200, events.first.payload[:status] + end end diff --git a/test/test_asset_helpers.rb b/test/test_asset_helpers.rb new file mode 100644 index 0000000..8e927d5 --- /dev/null +++ b/test/test_asset_helpers.rb @@ -0,0 +1,44 @@ +require 'minitest/autorun' +require 'prism' + +class AssetHelpersTest < Minitest::Test + def setup + Prism::Assets.reset! + @context = Prism::Template::Context.new + end + + def teardown + Prism::Assets.reset! + end + + def test_asset_path_falls_back_to_the_logical_path_without_a_manifest + assert_equal '/assets/application.css', @context.asset_path('application.css') + end + + def test_asset_path_uses_the_manifest_when_one_is_loaded + Prism::Assets.manifest = Prism::Assets::Manifest.new('application.css' => 'application-deadbeef.css') + assert_equal '/assets/application-deadbeef.css', @context.asset_path('application.css') + end + + def test_asset_path_raises_for_an_unknown_asset_once_a_manifest_is_loaded + Prism::Assets.manifest = Prism::Assets::Manifest.new + assert_raises(Prism::Assets::Manifest::AssetNotFound) { @context.asset_path('missing.css') } + end + + def test_stylesheet_link_tag + Prism::Assets.manifest = Prism::Assets::Manifest.new('application.css' => 'application-deadbeef.css') + assert_equal '', + @context.stylesheet_link_tag('application.css') + end + + def test_javascript_include_tag + Prism::Assets.manifest = Prism::Assets::Manifest.new('application.js' => 'application-cafef00d.js') + assert_equal '', + @context.javascript_include_tag('application.js') + end + + def test_image_tag_with_extra_attributes + assert_equal 'Logo', + @context.image_tag('logo.png', alt: 'Logo') + end +end