Clean up specs organization

This avoids double-setup of test rails apps which
makes specs faster and fixes warnings

Author: solnic

sentry-rails/spec/sentry/rails/log_subscriber_spec.rb

@@ -9,7 +9,6 @@
   let!(:test_subscriber) { test_subscriber_class.new }
 
   after do
-    Sentry.logger.clear if Sentry.logger.respond_to?(:clear)
     test_subscriber_class.detach_from(:test_component)
   end
 
@@ -242,9 +241,9 @@ def filtering_event(event)
     before do
       make_basic_app do |config, app|
         config.enable_logs = true
+
         config.structured_logger_class = Sentry::DebugStructuredLogger
         config.send_default_pii = true
-        app.config.filter_parameters += [:custom_secret, :api_key, :credit_card, :authorization]
       end
     end
 

sentry-rails/spec/sentry/rails/log_subscribers/action_controller_subscriber_spec.rb

@@ -3,221 +3,197 @@
 require "spec_helper"
 
 RSpec.describe Sentry::Rails::LogSubscribers::ActionControllerSubscriber, type: :request do
-  before do
-    make_basic_app do |config, app|
-      config.enable_logs = true
-      config.rails.structured_logging.enabled = true
-      config.rails.structured_logging.subscribers = { action_controller: Sentry::Rails::LogSubscribers::ActionControllerSubscriber }
-    end
-  end
-
-  describe "integration with ActiveSupport::Notifications" do
-    it "logs controller action events when requests are processed" do
-      get "/world"
-
-      Sentry.get_current_client.flush
-
-      expect(sentry_logs).not_to be_empty
-
-      log_event = sentry_logs.find { |log| log[:body] == "HelloController#world" }
-      expect(log_event).not_to be_nil
-      expect(log_event[:level]).to eq("info")
-      expect(log_event[:attributes][:controller][:value]).to eq("HelloController")
-      expect(log_event[:attributes][:action][:value]).to eq("world")
-      expect(log_event[:attributes][:status][:value]).to eq(200)
-
-      duration = log_event[:attributes][:duration_ms][:value]
-      expect(duration).to be_a(Float)
-      expect(duration).to be_in(0.01..30.0)
-
-      expect(log_event[:attributes][:method][:value]).to eq("GET")
-      expect(log_event[:attributes][:path][:value]).to eq("/world")
-      expect(log_event[:attributes][:format][:value]).to eq(:html)
-    end
-
-    it "logs bad requests appropriately" do
-      get "/not_found"
-
-      Sentry.get_current_client.flush
-
-      expect(sentry_logs).not_to be_empty
+  context "when logging is enabled" do
+    before do
+      make_basic_app do |config, app|
+        config.enable_logs = true
 
-      log_event = sentry_logs.find { |log| log[:body] == "HelloController#not_found" }
-      expect(log_event).not_to be_nil
-      expect(log_event[:level].to_sym).to be(:warn)
-      expect(log_event[:attributes][:status][:value]).to eq(400)
+        config.rails.structured_logging.enabled = true
+        config.rails.structured_logging.subscribers = { action_controller: Sentry::Rails::LogSubscribers::ActionControllerSubscriber }
+      end
     end
 
-    it "logs error status codes with error level" do
-      get "/exception"
+    describe "integration with ActiveSupport::Notifications" do
+      it "logs controller action events when requests are processed" do
+        get "/world"
 
-      Sentry.get_current_client.flush
-
-      expect(sentry_logs).not_to be_empty
-
-      log_event = sentry_logs.find { |log| log[:body] == "HelloController#exception" }
-      expect(log_event).not_to be_nil
-      expect(log_event[:level]).to eq("error")
-      expect(log_event[:attributes][:status][:value]).to eq(500)
-    end
-
-    it "includes view runtime when available" do
-      get "/view"
+        Sentry.get_current_client.flush
 
-      Sentry.get_current_client.flush
+        expect(sentry_logs).not_to be_empty
 
-      expect(sentry_logs).not_to be_empty
+        log_event = sentry_logs.find { |log| log[:body] == "HelloController#world" }
+        expect(log_event).not_to be_nil
+        expect(log_event[:level]).to eq("info")
+        expect(log_event[:attributes][:controller][:value]).to eq("HelloController")
+        expect(log_event[:attributes][:action][:value]).to eq("world")
+        expect(log_event[:attributes][:status][:value]).to eq(200)
+
+        duration = log_event[:attributes][:duration_ms][:value]
+        expect(duration).to be_a(Float)
+        expect(duration).to be_in(0.01..30.0)
+
+        expect(log_event[:attributes][:method][:value]).to eq("GET")
+        expect(log_event[:attributes][:path][:value]).to eq("/world")
+        expect(log_event[:attributes][:format][:value]).to eq(:html)
+      end
 
-      log_event = sentry_logs.find { |log| log[:body] == "HelloController#view" }
-      expect(log_event).not_to be_nil
-      expect(log_event[:attributes][:view_runtime_ms]).to be_present
-      expect(log_event[:attributes][:view_runtime_ms][:value]).to be >= 0
-    end
+      it "logs bad requests appropriately" do
+        get "/not_found"
 
-    it "includes database runtime when available" do
-      Post.create!
-      get "/posts"
+        Sentry.get_current_client.flush
 
-      Sentry.get_current_client.flush
+        expect(sentry_logs).not_to be_empty
 
-      expect(sentry_logs).not_to be_empty
+        log_event = sentry_logs.find { |log| log[:body] == "HelloController#not_found" }
+        expect(log_event).not_to be_nil
+        expect(log_event[:level].to_sym).to be(:warn)
+        expect(log_event[:attributes][:status][:value]).to eq(400)
+      end
 
-      log_event = sentry_logs.find { |log| log[:body] == "PostsController#index" }
-      expect(log_event).not_to be_nil
+      it "logs error status codes with error level" do
+        get "/exception"
 
-      if Rails.version.to_f >= 6.0
-        expect(log_event[:attributes][:db_runtime_ms]).to be_present
-        expect(log_event[:attributes][:db_runtime_ms][:value]).to be >= 0
-      else
-        if log_event[:attributes][:db_runtime_ms]
-          expect(log_event[:attributes][:db_runtime_ms][:value]).to be >= 0
-        end
-      end
-    end
+        Sentry.get_current_client.flush
 
-    context "when send_default_pii is enabled" do
-      before do
-        Sentry.configuration.send_default_pii = true
-      end
+        expect(sentry_logs).not_to be_empty
 
-      after do
-        Sentry.configuration.send_default_pii = false
+        log_event = sentry_logs.find { |log| log[:body] == "HelloController#exception" }
+        expect(log_event).not_to be_nil
+        expect(log_event[:level]).to eq("error")
+        expect(log_event[:attributes][:status][:value]).to eq(500)
       end
 
-      it "includes filtered request parameters" do
-        get "/world", params: { safe_param: "value", password: "secret" }
+      it "includes view runtime when available" do
+        get "/view"
 
         Sentry.get_current_client.flush
 
         expect(sentry_logs).not_to be_empty
 
-        log_event = sentry_logs.find { |log| log[:body] == "HelloController#world" }
+        log_event = sentry_logs.find { |log| log[:body] == "HelloController#view" }
         expect(log_event).not_to be_nil
-        expect(log_event[:attributes][:params]).to be_present
-        expect(log_event[:attributes][:params][:value]).to include("safe_param" => "value")
-        expect(log_event[:attributes][:params][:value]).to include("password" => "[FILTERED]")
+        expect(log_event[:attributes][:view_runtime_ms]).to be_present
+        expect(log_event[:attributes][:view_runtime_ms][:value]).to be >= 0
       end
 
-      it "filters sensitive parameter names" do
-        get "/world", params: {
-          normal_param: "value",
-          password: "secret",
-          api_key: "key123",
-          credit_card: "1234567890",
-          authorization: "Bearer token"
-        }
+      it "includes database runtime when available" do
+        Post.create!
+        get "/posts"
 
         Sentry.get_current_client.flush
 
         expect(sentry_logs).not_to be_empty
 
-        log_event = sentry_logs.find { |log| log[:body] == "HelloController#world" }
+        log_event = sentry_logs.find { |log| log[:body] == "PostsController#index" }
         expect(log_event).not_to be_nil
 
-        params = log_event[:attributes][:params][:value]
-        expect(params).to include("normal_param" => "value")
-        expect(params).to include("password" => "[FILTERED]")
-        expect(params).to include("api_key" => "[FILTERED]")
-        expect(params).to include("credit_card" => "[FILTERED]")
-        expect(params).to include("authorization" => "[FILTERED]")
+        if Rails.version.to_f >= 6.0
+          expect(log_event[:attributes][:db_runtime_ms]).to be_present
+          expect(log_event[:attributes][:db_runtime_ms][:value]).to be >= 0
+        else
+          if log_event[:attributes][:db_runtime_ms]
+            expect(log_event[:attributes][:db_runtime_ms][:value]).to be >= 0
+          end
+        end
       end
 
-      it "respects Rails filter_parameters configuration" do
-        original_filter_params = Rails.application.config.filter_parameters.dup
-        Rails.application.config.filter_parameters += [:custom_secret]
+      context "when send_default_pii is enabled" do
+        before do
+          Sentry.configuration.send_default_pii = true
+        end
 
-        get "/world", params: {
-          normal_param: "value",
-          custom_secret: "should_be_filtered",
-          another_param: "visible"
-        }
+        after do
+          Sentry.configuration.send_default_pii = false
+        end
 
-        Sentry.get_current_client.flush
+        it "includes filtered request parameters" do
+          get "/world", params: { safe_param: "value", password: "secret" }
 
-        expect(sentry_logs).not_to be_empty
+          Sentry.get_current_client.flush
 
-        log_event = sentry_logs.find { |log| log[:body] == "HelloController#world" }
-        expect(log_event).not_to be_nil
+          expect(sentry_logs).not_to be_empty
 
-        params = log_event[:attributes][:params][:value]
-        expect(params).to include("normal_param" => "value")
-        expect(params).to include("another_param" => "visible")
-        expect(params).to include("custom_secret" => "[FILTERED]")
+          log_event = sentry_logs.find { |log| log[:body] == "HelloController#world" }
+          expect(log_event).not_to be_nil
+          expect(log_event[:attributes][:params]).to be_present
+          expect(log_event[:attributes][:params][:value]).to include("safe_param" => "value")
+          expect(log_event[:attributes][:params][:value]).to include("password" => "[FILTERED]")
+        end
 
-        Rails.application.config.filter_parameters = original_filter_params
-      end
+        it "filters sensitive parameter names" do
+          get "/world", params: {
+            normal_param: "value",
+            password: "secret",
+            api_key: "key123",
+            credit_card: "1234567890",
+            authorization: "Bearer token"
+          }
 
-      it "handles nested parameters correctly" do
-        get "/world", params: {
-          user: {
-            name: "John",
-            password: "secret123",
-            profile: {
-              api_key: "key456",
-              public_info: "visible"
-            }
-          },
-          normal_param: "value"
-        }
+          Sentry.get_current_client.flush
 
-        Sentry.get_current_client.flush
+          expect(sentry_logs).not_to be_empty
 
-        expect(sentry_logs).not_to be_empty
+          log_event = sentry_logs.find { |log| log[:body] == "HelloController#world" }
+          expect(log_event).not_to be_nil
 
-        log_event = sentry_logs.find { |log| log[:body] == "HelloController#world" }
-        expect(log_event).not_to be_nil
+          params = log_event[:attributes][:params][:value]
+          expect(params).to include("normal_param" => "value")
+          expect(params).to include("password" => "[FILTERED]")
+          expect(params).to include("api_key" => "[FILTERED]")
+          expect(params).to include("credit_card" => "[FILTERED]")
+          expect(params).to include("authorization" => "[FILTERED]")
+        end
 
-        params = log_event[:attributes][:params][:value]
-        expect(params).to include("normal_param" => "value")
-        expect(params["user"]).to include("name" => "John")
-        expect(params["user"]).to include("password" => "[FILTERED]")
-        expect(params["user"]["profile"]).to include("api_key" => "[FILTERED]")
-        expect(params["user"]["profile"]).to include("public_info" => "visible")
+        it "handles nested parameters correctly" do
+          get "/world", params: {
+            user: {
+              name: "John",
+              password: "secret123",
+              profile: {
+                api_key: "key456",
+                public_info: "visible"
+              }
+            },
+            normal_param: "value"
+          }
+
+          Sentry.get_current_client.flush
+
+          expect(sentry_logs).not_to be_empty
+
+          log_event = sentry_logs.find { |log| log[:body] == "HelloController#world" }
+          expect(log_event).not_to be_nil
+
+          params = log_event[:attributes][:params][:value]
+          expect(params).to include("normal_param" => "value")
+          expect(params["user"]).to include("name" => "John")
+          expect(params["user"]).to include("password" => "[FILTERED]")
+          expect(params["user"]["profile"]).to include("api_key" => "[FILTERED]")
+          expect(params["user"]["profile"]).to include("public_info" => "visible")
+        end
       end
-    end
-
-    context "when send_default_pii is disabled" do
-      it "does not include request parameters" do
-        sentry_transport.events.clear
-        sentry_transport.envelopes.clear
 
-        get "/world", params: { param: "value" }
+      context "when send_default_pii is disabled" do
+        it "does not include request parameters" do
+          get "/world", params: { param: "value" }
 
-        Sentry.get_current_client.flush
+          Sentry.get_current_client.flush
 
-        expect(sentry_logs).not_to be_empty
+          expect(sentry_logs).not_to be_empty
 
-        log_event = sentry_logs.find { |log| log[:body] == "HelloController#world" }
-        expect(log_event).not_to be_nil
-        expect(log_event[:attributes]).not_to have_key(:params)
+          log_event = sentry_logs.find { |log| log[:body] == "HelloController#world" }
+          expect(log_event).not_to be_nil
+          expect(log_event[:attributes]).not_to have_key(:params)
+        end
       end
     end
   end
 
-  describe "when logging is disabled" do
+  context "when logging is disabled" do
     before do
       make_basic_app do |config|
         config.enable_logs = false
+
         config.rails.structured_logging.enabled = true
         config.rails.structured_logging.subscribers = { action_controller: Sentry::Rails::LogSubscribers::ActionControllerSubscriber }
       end