Merge pull request #4579 from DataDog/dry-webrick

DEBUG-3700 DRY webrick usage in test suite

Author: TonyCTHsu

spec/datadog/core/crashtracking/component_spec.rb

@@ -4,6 +4,9 @@
 require 'webrick'
 require 'fiddle'
 
+# https://github.com/rubocop/rubocop-rspec/issues/2078
+# rubocop:disable RSpec/ScatteredLet
+
 RSpec.describe Datadog::Core::Crashtracking::Component, skip: !CrashtrackingHelpers.supported? do
   let(:logger) { Logger.new($stdout) }
 
@@ -165,18 +168,10 @@
 
     context 'integration testing' do
       shared_context 'HTTP server' do
-        let(:server) do
-          WEBrick::HTTPServer.new(
-            Port: 0,
-            Logger: log,
-            AccessLog: access_log,
-            StartCallback: -> { init_signal.push(1) }
-          )
+        http_server do |http_server|
+          http_server.mount_proc('/', &server_proc)
         end
         let(:hostname) { '127.0.0.1' }
-        let(:log) { WEBrick::Log.new(StringIO.new, WEBrick::Log::WARN) }
-        let(:access_log_buffer) { StringIO.new }
-        let(:access_log) { [[access_log_buffer, WEBrick::AccessLog::COMBINED_LOG_FORMAT]] }
         let(:server_proc) do
           proc do |req, res|
             messages << req.tap { req.body } # Read body, store message before socket closes.
@@ -186,30 +181,13 @@
         let(:init_signal) { Queue.new }
 
         let(:messages) { [] }
-
-        before do
-          server.mount_proc('/', &server_proc)
-          @server_thread = Thread.new { server.start }
-          init_signal.pop
-        end
-
-        after do
-          unless RSpec.current_example.skipped?
-            # When the test is skipped, server has not been initialized and @server_thread would be nil; thus we only
-            # want to touch them when the test actually run, otherwise we would cause the server to start (incorrectly)
-            # and join to be called on a nil @server_thread
-            server.shutdown
-            @server_thread.join
-          end
-        end
       end
 
       include_context 'HTTP server'
 
       let(:request) { messages.first }
-      let(:port) { server[:Port] }
 
-      let(:agent_base_url) { "http://#{hostname}:#{port}" }
+      let(:agent_base_url) { "http://#{hostname}:#{http_server_port}" }
 
       [:fiddle, :signal].each do |trigger|
         it "reports crashes via http when app crashes with #{trigger}" do
@@ -251,15 +229,14 @@
         let(:temporary_directory) { Dir.mktmpdir }
         let(:socket_path) { "#{temporary_directory}/rspec_unix_domain_socket" }
         let(:unix_domain_socket) { UNIXServer.new(socket_path) } # Closing the socket is handled by webrick
-        let(:server) do
-          server = WEBrick::HTTPServer.new(
+        define_http_server do |http_server|
+          http_server.listeners << unix_domain_socket
+          http_server.mount_proc('/', &server_proc)
+        end
+        let(:http_server_options) do
+          {
             DoNotListen: true,
-            Logger: log,
-            AccessLog: access_log,
-            StartCallback: -> { init_signal.push(1) }
-          )
-          server.listeners << unix_domain_socket
-          server
+          }
         end
         let(:agent_base_url) { "unix://#{socket_path}" }
 
@@ -340,3 +317,5 @@ def tear_down!
     described_class._native_stop
   end
 end
+
+# rubocop:enable RSpec/ScatteredLet

spec/datadog/di/integration/probe_notifier_worker_spec.rb

@@ -1,6 +1,5 @@
 require "datadog/di/spec_helper"
 require 'datadog/di'
-require 'webrick'
 
 # standard tries to wreck regular expressions in this fiel
 # rubocop:disable Style/PercentLiteralDelimiters
@@ -23,55 +22,32 @@
   let(:settings) do
     Datadog::Core::Configuration::Settings.new.tap do |settings|
       settings.agent.host = 'localhost'
-      settings.agent.port = test_port
+      settings.agent.port = http_server_port
       settings.agent.use_ssl = false
       settings.agent.timeout_seconds = 1
     end
   end
 
   let(:agent_settings) { Datadog::Core::Configuration::AgentSettingsResolver.call(settings, logger: nil) }
 
-  let(:test_port) { 48485 }
-
   di_logger_double
 
   let(:diagnostics_payloads) { [] }
   let(:input_payloads) { [] }
 
-  let(:server) do
-    WEBrick::HTTPServer.new(
-      Port: test_port,
-    ).tap do |server|
-      @received_snapshot_count = 0
-      @received_snapshot_bytes = 0
-
-      server.mount_proc('/debugger/v1/diagnostics') do |req, res|
-        # This request is a multipart form post
-        expect(req.content_type).to match(%r,^multipart/form-data;,)
-        diagnostics_payloads << req.body
-      end
-
-      server.mount_proc('/debugger/v1/input') do |req, res|
-        payload = JSON.parse(req.body)
-        input_payloads << payload
-      end
-    end
-  end
+  http_server do |http_server|
+    @received_snapshot_count = 0
+    @received_snapshot_bytes = 0
 
-  around do |example|
-    @server_thread = Thread.new do
-      server.start
+    http_server.mount_proc('/debugger/v1/diagnostics') do |req, res|
+      # This request is a multipart form post
+      expect(req.content_type).to match(%r,^multipart/form-data;,)
+      diagnostics_payloads << req.body
     end
-    loop do
-      break if server.status == :Running || !@server_thread.alive?
-      sleep 0.5
-    end
-    expect(@server_thread).to be_alive
-    example.run
-    @server_thread.kill
-    loop do
-      break unless @server_thread.alive?
-      sleep 0.5
+
+    http_server.mount_proc('/debugger/v1/input') do |req, res|
+      payload = JSON.parse(req.body)
+      input_payloads << payload
     end
   end
 

spec/datadog/profiling/http_transport_spec.rb

@@ -7,6 +7,9 @@
 require "socket"
 require "webrick"
 
+# https://github.com/rubocop/rubocop-rspec/issues/2078
+# rubocop:disable RSpec/ScatteredLet
+
 # Design note for this class's specs: from the Ruby code side, we're treating the `_native_` methods as an API
 # between the Ruby code and the native methods, and thus in this class we have a bunch of tests to make sure the
 # native methods are invoked correctly.
@@ -282,51 +285,26 @@
 
   context "integration testing" do
     shared_context "HTTP server" do
-      let(:server) do
-        WEBrick::HTTPServer.new(
-          Port: 0,
-          Logger: log,
-          AccessLog: access_log,
-          StartCallback: -> { init_signal.push(1) }
-        )
+      http_server do |http_server|
+        http_server.mount_proc('/', &server_proc)
       end
       let(:hostname) { "127.0.0.1" }
-      let(:log) { WEBrick::Log.new($stderr, WEBrick::Log::WARN) }
-      let(:access_log_buffer) { StringIO.new }
-      let(:access_log) { [[access_log_buffer, WEBrick::AccessLog::COMBINED_LOG_FORMAT]] }
       let(:server_proc) do
         proc do |req, res|
           messages << req.tap { req.body } # Read body, store message before socket closes.
           res.body = "{}"
         end
       end
-      let(:init_signal) { Queue.new }
 
       let(:messages) { [] }
-
-      before do
-        server.mount_proc("/", &server_proc)
-        @server_thread = Thread.new { server.start }
-        init_signal.pop
-      end
-
-      after do
-        unless RSpec.current_example.skipped?
-          # When the test is skipped, server has not been initialized and @server_thread would be nil; thus we only
-          # want to touch them when the test actually run, otherwise we would cause the server to start (incorrectly)
-          # and join to be called on a nil @server_thread
-          server.shutdown
-          @server_thread.join
-        end
-      end
     end
 
     include_context "HTTP server"
 
     let(:request) { messages.first }
 
     let(:hostname) { "127.0.0.1" }
-    let(:port) { server[:Port] }
+    let(:port) { http_server_port }
 
     let!(:encoded_profile_bytes) { encoded_profile._native_bytes }
 
@@ -416,15 +394,14 @@
       let(:temporary_directory) { Dir.mktmpdir }
       let(:socket_path) { "#{temporary_directory}/rspec_unix_domain_socket" }
       let(:unix_domain_socket) { UNIXServer.new(socket_path) } # Closing the socket is handled by webrick
-      let(:server) do
-        server = WEBrick::HTTPServer.new(
+      define_http_server do |http_server|
+        http_server.listeners << unix_domain_socket
+        http_server.mount_proc('/', &server_proc)
+      end
+      let(:http_server_options) do
+        {
           DoNotListen: true,
-          Logger: log,
-          AccessLog: access_log,
-          StartCallback: -> { init_signal.push(1) }
-        )
-        server.listeners << unix_domain_socket
-        server
+        }
       end
       let(:adapter) { Datadog::Core::Transport::Ext::UnixSocket::ADAPTER }
       let(:uds_path) { socket_path }
@@ -440,7 +417,7 @@
 
     context "when agent is down" do
       before do
-        server.shutdown
+        http_server.shutdown
         @server_thread.join
       end
 
@@ -549,3 +526,5 @@
     end
   end
 end
+
+# rubocop:enable RSpec/ScatteredLet

spec/datadog/tracing/contrib/ethon/integration_context.rb

@@ -2,21 +2,8 @@
 require 'spec/support/thread_helpers'
 
 RSpec.shared_context 'integration context' do
-  before(:all) do
-    # TODO: Consolidate mock webserver code
-    @log_buffer = StringIO.new # set to $stderr to debug
-    log = WEBrick::Log.new(@log_buffer, WEBrick::Log::DEBUG)
-    access_log = [[@log_buffer, WEBrick::AccessLog::COMBINED_LOG_FORMAT]]
-
-    init_signal = Queue.new
-    server = WEBrick::HTTPServer.new(
-      Port: 0,
-      Logger: log,
-      AccessLog: access_log,
-      StartCallback: -> { init_signal.push(1) }
-    )
-
-    server.mount_proc '/' do |req, res|
+  http_server do |http_server|
+    http_server.mount_proc '/' do |req, res|
       sleep(0.001) if req.query['simulate_timeout']
       res.status = (req.query['status'] || req.body['status']).to_i
       if req.query['return_headers']
@@ -29,26 +16,12 @@
         res.body = 'response'
       end
     end
-
-    ThreadHelpers.with_leaky_thread_creation(:ethon_test_server) do
-      @thread = Thread.new { server.start }
-    end
-
-    init_signal.pop
-
-    @server = server
-    @port = server[:Port]
-  end
-
-  after(:all) do
-    @server.shutdown
-    @thread.join
   end
 
   let(:host) { 'localhost' }
   let(:status) { '200' }
   let(:path) { '/sample/path' }
-  let(:port) { @port }
+  let(:port) { http_server_port }
   let(:method) { 'GET' }
   let(:simulate_timeout) { false }
   let(:timeout) { 5 }
@@ -59,7 +32,7 @@
     query[:simulate_timeout] = 'true' if simulate_timeout
   end
   let(:url) do
-    url = "http://#{host}:#{@port}#{path}?"
+    url = "http://#{host}:#{http_server_port}#{path}?"
     url += "status=#{status}&" if status
     url += 'return_headers=true&' if return_headers
     url += 'simulate_timeout=true' if simulate_timeout

spec/datadog/tracing/contrib/ethon/integration_test_spec.rb

@@ -1,7 +1,6 @@
 require 'json'
 require 'stringio'
 require 'typhoeus'
-require 'webrick'
 
 require 'datadog/tracing/trace_digest'
 require 'datadog/tracing/contrib/ethon/easy_patch'

spec/datadog/tracing/contrib/ethon/typhoeus_integration_spec.rb

@@ -42,8 +42,8 @@
   context 'with concurrent Hydra requests' do
     include_context 'integration context'
 
-    let(:url_1) { "http://#{host}:#{@port}#{path}?status=200&simulate_timeout=true" }
-    let(:url_2) { "http://#{host}:#{@port}#{path}" }
+    let(:url_1) { "http://#{host}:#{http_server_port}#{path}?status=200&simulate_timeout=true" }
+    let(:url_2) { "http://#{host}:#{http_server_port}#{path}" }
     let(:request_1) { Typhoeus::Request.new(url_1, timeout: 0.001) }
     let(:request_2) { Typhoeus::Request.new(url_2, method: :post, timeout: timeout, body: { status: 404 }) }
 

spec/datadog/tracing/contrib/httpclient/instrumentation_spec.rb

@@ -20,14 +20,8 @@
 require 'spec/support/thread_helpers'
 
 RSpec.describe Datadog::Tracing::Contrib::Httpclient::Instrumentation do
-  before(:all) do
-    # TODO: Consolidate mock webserver code
-    @log_buffer = StringIO.new # set to $stderr to debug
-    log = WEBrick::Log.new(@log_buffer, WEBrick::Log::DEBUG)
-    access_log = [[@log_buffer, WEBrick::AccessLog::COMBINED_LOG_FORMAT]]
-
-    server = WEBrick::HTTPServer.new(Port: 0, Logger: log, AccessLog: access_log)
-    server.mount_proc '/' do |req, res|
+  http_server do |http_server|
+    http_server.mount_proc '/' do |req, res|
       body = JSON.parse(req.body)
       res.status = body['code'].to_i
 
@@ -39,18 +33,6 @@
 
       res.body = req.body
     end
-
-    ThreadHelpers.with_leaky_thread_creation(:httpclient_test_server) do
-      @thread = Thread.new { server.start }
-    end
-
-    @server = server
-    @port = server[:Port]
-  end
-
-  after(:all) do
-    @server.shutdown
-    @thread.join
   end
 
   let(:configuration_options) { {} }
@@ -73,8 +55,8 @@
     let(:host) { 'localhost' }
     let(:message) { 'OK' }
     let(:path) { '/sample/path' }
-    let(:port) { @port }
-    let(:url) { "http://#{host}:#{@port}#{path}" }
+    let(:port) { http_server_port }
+    let(:url) { "http://#{host}:#{http_server_port}#{path}" }
     let(:body) { { 'message' => message, 'code' => code } }
     let(:headers) { { accept: 'application/json' } }
     let(:client) { HTTPClient.new }