Merge pull request #1236 from appsignal/spec-refactor

A couple test suite refactors

spec/lib/appsignal/cli/demo_spec.rb

@@ -7,12 +7,6 @@
   let(:out_stream) { std_stream }
   let(:output) { out_stream.read }
   before(:context) { Appsignal.stop }
-  before do
-    ENV.delete("APPSIGNAL_APP_ENV")
-    ENV.delete("RAILS_ENV")
-    ENV.delete("RACK_ENV")
-    stub_api_request config, "auth"
-  end
 
   def run
     run_within_dir project_fixture_path
@@ -25,8 +19,6 @@ def run_within_dir(chdir)
   end
 
   context "without configuration" do
-    let(:config) { Appsignal::Config.new("development", tmp_dir) }
-
     it "returns an error" do
       expect { run_within_dir tmp_dir }.to raise_error(SystemExit)
 
@@ -35,32 +27,20 @@ def run_within_dir(chdir)
   end
 
   context "with configuration" do
-    let(:config) { project_fixture_config }
     before do
       # Ignore sleeps to speed up the test
       allow(Appsignal::Demo).to receive(:sleep)
     end
+    let(:options) { { :environment => "development" } }
 
-    context "without environment" do
-      it "returns an error" do
-        expect { run_within_dir tmp_dir }.to raise_error(SystemExit)
-
-        expect(output).to include("Error: Unable to start the AppSignal agent")
-      end
+    it "calls Appsignal::Demo transmitter" do
+      expect(Appsignal::Demo).to receive(:transmit).and_return(true)
+      run
     end
 
-    context "with environment" do
-      let(:options) { { :environment => "development" } }
-
-      it "calls Appsignal::Demo transmitter" do
-        expect(Appsignal::Demo).to receive(:transmit).and_return(true)
-        run
-      end
-
-      it "outputs message" do
-        run
-        expect(output).to include("Demonstration sample data sent!")
-      end
+    it "outputs message" do
+      run
+      expect(output).to include("Demonstration sample data sent!")
     end
   end
 end

spec/lib/appsignal/config_spec.rb

@@ -538,7 +538,7 @@ def on_load
           File.join(File.dirname(__FILE__), "../../support/fixtures/projects/broken")
         )
       end
-      let(:config) { Appsignal::Config.new(config_path, "foo") }
+      let(:config) { described_class.new(config_path, "production") }
 
       it "does not start AppSignal, logs & prints an error" do
         stdout = std_stream
@@ -602,19 +602,17 @@ def on_load
 
     context "without the selected env" do
       let(:config) { project_fixture_config("nonsense") }
-      let(:log_stream) { std_stream }
-      let(:log) { log_contents(log_stream) }
 
       it "is not valid or active" do
         expect(config.valid?).to be_falsy
         expect(config.active?).to be_falsy
       end
 
       it "logs an error" do
-        use_logger_with(log_stream) { config }
-        expect(log)
+        logs = capture_logs { config }
+        expect(logs)
           .to contains_log(:error, "Not loading from config file: config for 'nonsense' not found")
-        expect(log)
+        expect(logs)
           .to contains_log(:error, "Push API key not set after loading config")
       end
     end
@@ -624,9 +622,7 @@ def on_load
     let(:config) do
       described_class.new(
         "non-existing-path",
-        "production",
-        :running_in_container => true,
-        :debug => true
+        "production"
       )
     end
     let(:working_directory_path) { File.join(tmp_dir, "test_working_directory_path") }
@@ -789,12 +785,12 @@ def on_load
 
     context "with mixed case `true` env variables values" do
       before do
-        ENV["APPSIGNAL_DEBUG"] = "TRUE"
+        ENV["APPSIGNAL_ENABLE_RAKE_PERFORMANCE_INSTRUMENTATION"] = "TRUE"
         ENV["APPSIGNAL_INSTRUMENT_SEQUEL"] = "True"
       end
 
       it "accepts mixed case `true` values" do
-        expect(config[:debug]).to eq(true)
+        expect(config[:enable_rake_performance_instrumentation]).to eq(true)
         expect(config[:instrument_sequel]).to eq(true)
       end
     end
@@ -805,11 +801,8 @@ def on_load
   end
 
   describe "with config based on overrides" do
-    let(:log_stream) { StringIO.new }
-    let(:logger) { test_logger(log_stream) }
-    let(:logs) { log_contents(log_stream) }
     let(:config) do
-      described_class.new(Dir.pwd, "production", config_options, logger)
+      described_class.new(Dir.pwd, "production", config_options)
     end
 
     if DependencyHelper.rails_present?
@@ -1425,9 +1418,10 @@ def log_file_path
       end
 
       it "logs a deprecation warning" do
-        logs = capture_logs do
-          silence { dsl.app_path = "foo" }
-        end
+        logs =
+          capture_logs do
+            silence { dsl.app_path = "foo" }
+          end
 
         expect(logs).to contains_log(
           :warn,

spec/lib/appsignal/hooks/activejob_spec.rb

@@ -34,7 +34,6 @@
     let(:time) { Time.parse("2001-01-01 10:00:00UTC") }
     let(:namespace) { Appsignal::Transaction::BACKGROUND_JOB }
     let(:queue) { "default" }
-    let(:log) { StringIO.new }
     let(:parameterized_given_args) do
       {
         :foo => "Foo",
@@ -77,7 +76,6 @@
       ActiveJob::Base.queue_adapter = :inline
 
       start_agent(:options => options)
-      Appsignal.internal_logger = test_logger(log)
       class ActiveJobTestJob < ActiveJob::Base
         def perform(*_args)
         end

spec/lib/appsignal/integrations/sidekiq_spec.rb

@@ -173,7 +173,7 @@ def expect_no_error_on_transaction
   end
 end
 
-describe Appsignal::Integrations::SidekiqMiddleware, :with_yaml_parse_error => false do
+describe Appsignal::Integrations::SidekiqMiddleware do
   class DelayedTestClass; end
 
   let(:namespace) { Appsignal::Transaction::BACKGROUND_JOB }
@@ -214,15 +214,14 @@ class DelayedTestClass; end
     }
   end
   let(:plugin) { Appsignal::Integrations::SidekiqMiddleware.new }
-  let(:log) { StringIO.new }
   let(:options) { {} }
   before do
     start_agent(:options => options)
-    Appsignal.internal_logger = test_logger(log)
   end
   around { |example| keep_transactions { example.run } }
-  after :with_yaml_parse_error => false do
-    expect(log_contents(log)).to_not contains_log(:warn, "Unable to load YAML")
+
+  def expect_no_yaml_parse_error(logs)
+    expect(logs).to_not contains_log(:warn, "Unable to load YAML")
   end
 
   describe "internal Sidekiq job values" do
@@ -290,15 +289,15 @@ class DelayedTestClass; end
       expect(transaction).to include_params(["bar" => "baz"])
     end
 
-    context "when job arguments is a malformed YAML object", :with_yaml_parse_error => true do
+    context "when job arguments is a malformed YAML object" do
       before { item["args"] = [] }
 
       it "logs a warning and uses the default argument" do
-        perform_sidekiq_job
+        logs = capture_logs { perform_sidekiq_job }
 
         expect(transaction).to have_action("Sidekiq::Extensions::DelayedClass#perform")
         expect(transaction).to include_params([])
-        expect(log_contents(log)).to contains_log(:warn, "Unable to load YAML")
+        expect(logs).to contains_log(:warn, "Unable to load YAML")
       end
     end
   end
@@ -326,15 +325,15 @@ class DelayedTestClass; end
       expect(transaction).to include_params(["bar" => "baz"])
     end
 
-    context "when job arguments is a malformed YAML object", :with_yaml_parse_error => true do
+    context "when job arguments is a malformed YAML object" do
       before { item["args"] = [] }
 
       it "logs a warning and uses the default argument" do
-        perform_sidekiq_job
+        logs = capture_logs { perform_sidekiq_job }
 
         expect(transaction).to have_action("Sidekiq::Extensions::DelayedModel#perform")
         expect(transaction).to include_params([])
-        expect(log_contents(log)).to contains_log(:warn, "Unable to load YAML")
+        expect(logs).to contains_log(:warn, "Unable to load YAML")
       end
     end
   end

spec/lib/appsignal/rack/event_handler_spec.rb

@@ -13,7 +13,7 @@
   let(:request) { Rack::Request.new(env) }
   let(:response) { nil }
   let(:log_stream) { StringIO.new }
-  let(:log) { log_contents(log_stream) }
+  let(:logs) { log_contents(log_stream) }
   let(:event_handler_instance) { described_class.new }
   let(:appsignal_env) { :default }
   before do
@@ -96,7 +96,7 @@ def on_error(error)
       end
 
       it "logs an error" do
-        expect(log).to contains_log(
+        expect(logs).to contains_log(
           :error,
           "Error occurred in Appsignal::Rack::EventHandler's after_reply: " \
             "ExampleStandardError: oh no"
@@ -113,7 +113,7 @@ def on_error(error)
       callback = request.env[Appsignal::Rack::RACK_AFTER_REPLY].first
       callback.call
 
-      expect(log).to contains_log(
+      expect(logs).to contains_log(
         :error,
         "Error occurred in Appsignal::Rack::EventHandler's after_reply: ExampleStandardError: oh no"
       )
@@ -125,7 +125,7 @@ def on_error(error)
 
       on_start
 
-      expect(log).to contains_log(
+      expect(logs).to contains_log(
         :error,
         "Error occurred in Appsignal::Rack::EventHandler#on_start: ExampleStandardError: oh no"
       )
@@ -168,7 +168,7 @@ def on_error(error)
 
       on_error(ExampleStandardError.new("the error"))
 
-      expect(log).to contains_log(
+      expect(logs).to contains_log(
         :error,
         "Error occurred in Appsignal::Rack::EventHandler#on_error: ExampleStandardError: oh no"
       )
@@ -354,8 +354,10 @@ def on_finish(given_request = request, given_response = response)
       end
 
       it "logs an error" do
-        expect(log).to contains_log(:error,
-          "Error occurred in Appsignal::Rack::EventHandler#on_finish: ExampleStandardError: oh no")
+        expect(logs).to contains_log(
+          :error,
+          "Error occurred in Appsignal::Rack::EventHandler#on_finish: ExampleStandardError: oh no"
+        )
       end
     end
 
@@ -430,7 +432,7 @@ def on_finish(given_request = request, given_response = response)
       on_start
       on_finish
 
-      expect(log).to contains_log(
+      expect(logs).to contains_log(
         :error,
         "Error occurred in Appsignal::Rack::EventHandler#on_finish: ExampleStandardError: oh no"
       )

spec/lib/appsignal/rack/rails_instrumentation_spec.rb

@@ -2,7 +2,6 @@
   describe Appsignal::Rack::RailsInstrumentation do
     class MockController; end
 
-    let(:log) { StringIO.new }
     let(:transaction) { new_transaction }
     let(:app) { DummyApp.new }
     let(:params) do
@@ -30,7 +29,6 @@ class MockController; end
     around { |example| keep_transactions { example.run } }
     before do
       start_agent
-      Appsignal.internal_logger = test_logger(log)
       env[Appsignal::Rack::APPSIGNAL_TRANSACTION] = transaction
     end
 
@@ -105,10 +103,10 @@ def make_request_with_error(error_class, error_message)
       it "does not store the invalid HTTP request method" do
         env[:request_method] = "FOO"
         env["REQUEST_METHOD"] = "FOO"
-        make_request
+        logs = capture_logs { make_request }
 
         expect(last_transaction).to_not include_metadata("method" => anything)
-        expect(log_contents(log)).to contains_log(
+        expect(logs).to contains_log(
           :error,
           "Exception while fetching the HTTP request method: "
         )

spec/support/helpers/log_helpers.rb

@@ -8,6 +8,7 @@ def capture_logs(&block)
   def use_logger_with(log)
     Appsignal.internal_logger = test_logger(log)
     yield
+  ensure
     Appsignal.internal_logger = nil
   end