Merge pull request #2248 from newrelic/pumpkin_spiced_rails_tests

Rails v7.1 driven 'rails' / 'rails_prepend' suite updates

.github/workflows/ci.yml

@@ -25,6 +25,7 @@ jobs:
         image: mysql:5.7
         env:
           MYSQL_ALLOW_EMPTY_PASSWORD: yes
+          CI_FOR_PR: true
         options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
         ports:
           - "3306:3306"
@@ -83,6 +84,7 @@ jobs:
         env:
           RUBY_VERSION: ${{ matrix.ruby-version }}
           RAILS_VERSION: ${{ env.rails }}
+          CI_FOR_PR: true
 
       - name: Run Unit Tests
         uses: nick-fields/retry@943e742917ac94714d2f408a0e8320f2d1fcafcd # tag v2.8.3
@@ -94,6 +96,7 @@ jobs:
           DB_PORT: ${{ job.services.mysql.ports[3306] }}
           VERBOSE_TEST_OUTPUT: true
           COVERAGE: true
+          CI_FOR_PR: true
 
       - name: Save coverage results
         uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # tag v3.1.2
@@ -253,6 +256,7 @@ jobs:
           POSTGRES_PASSWORD: password
           SERIALIZE: 1
           COVERAGE: true
+          CI_FOR_PR: true
 
       - name: Annotate errors
         if: ${{ failure() }}
@@ -306,6 +310,7 @@ jobs:
           VERBOSE_TEST_OUTPUT: true
           SERIALIZE: 1
           COVERAGE: true
+          CI_FOR_PR: true
 
       - name: Annotate errors
         if: ${{ failure() }}

lib/new_relic/control/frameworks/rails.rb

@@ -10,6 +10,9 @@ module Frameworks
       # Rails specific configuration, instrumentation, environment values,
       # etc.
       class Rails < NewRelic::Control::Frameworks::Ruby
+        BROWSER_MONITORING_INSTALLED_SINGLETON = NewRelic::Agent.config
+        BROWSER_MONITORING_INSTALLED_VARIABLE = :@browser_monitoring_installed
+
         def env
           @env ||= (ENV['NEW_RELIC_ENV'] || RAILS_ENV.dup)
         end
@@ -97,9 +100,9 @@ def install_agent_hooks(config)
 
         def install_browser_monitoring(config)
           @install_lock.synchronize do
-            return if defined?(@browser_monitoring_installed) && @browser_monitoring_installed
+            return if browser_agent_already_installed?
 
-            @browser_monitoring_installed = true
+            mark_browser_agent_as_installed
             return if config.nil? || !config.respond_to?(:middleware) || !Agent.config[:'browser_monitoring.auto_instrument']
 
             begin
@@ -112,6 +115,15 @@ def install_browser_monitoring(config)
           end
         end
 
+        def browser_agent_already_installed?
+          BROWSER_MONITORING_INSTALLED_SINGLETON.instance_variable_defined?(BROWSER_MONITORING_INSTALLED_VARIABLE) &&
+            BROWSER_MONITORING_INSTALLED_SINGLETON.instance_variable_get(BROWSER_MONITORING_INSTALLED_VARIABLE)
+        end
+
+        def mark_browser_agent_as_installed
+          BROWSER_MONITORING_INSTALLED_SINGLETON.instance_variable_set(BROWSER_MONITORING_INSTALLED_VARIABLE, true)
+        end
+
         def rails_version
           @rails_version ||= Gem::Version.new(::Rails::VERSION::STRING)
         end

test/multiverse/lib/multiverse/envfile.rb

@@ -133,6 +133,34 @@ def serialize?
       @serialize
     end
 
+    # add Rails Edge to the beginning of the array of gem versions for testing,
+    # unless we're operating in a PR workflow context
+    def unshift_rails_edge(gem_version_array = [])
+      return if ci_for_pr?
+
+      # Unshift Rails Edge (representing the latest GitHub primary branch
+      # commit for https://github.com/rails/rails) onto the front of the
+      # gem version array. This produces the following line in the generated
+      # Gemfile file:
+      #
+      #   gem 'rails', github: 'rails'
+      #
+      # NOTE: Individually distributed Rails gems such as Active Record are each
+      #       contained within the same 'rails' GitHub repo. For now we are not
+      #       too concerned with cloning the entire Rails repo despite only
+      #       wanting to test one gem.
+      #
+      # NOTE: The Rails Edge version is not tested unless the Ruby version in
+      #       play is greater than or equal to (>=) the version number at the
+      #       end of the unshifted inner array
+      gem_version_array.unshift(["github: 'rails'", 3.0])
+    end
+
+    # are we running in a CI context intended for PR approvals?
+    def ci_for_pr?
+      ENV.key?('CI_FOR_PR')
+    end
+
     private
 
     def last_supported_ruby_version?(last_supported_ruby_version)

test/multiverse/suites/active_record_pg/Envfile

@@ -21,6 +21,8 @@ ACTIVERECORD_VERSIONS = [
   ['5.0.0', 2.4, 2.7]
 ]
 
+unshift_rails_edge(ACTIVERECORD_VERSIONS)
+
 def gem_list(activerecord_version = nil)
   <<~RB
     gem 'activerecord'#{activerecord_version}

test/multiverse/suites/active_record_pg/active_record_test.rb

@@ -66,17 +66,19 @@ def test_metrics_for_pluck
     end
   end
 
-  if active_record_version >= Gem::Version.new('4.0.0')
-    def test_metrics_for_ids
-      in_web_transaction do
-        Order.ids
-      end
+  def test_metrics_for_ids
+    in_web_transaction do
+      Order.ids
+    end
 
-      if active_record_major_version >= 7
-        assert_activerecord_metrics(Order, 'pluck')
+    if active_record_major_version >= 7
+      if active_record_minor_version >= 1
+        assert_activerecord_metrics(Order, 'ids')
       else
-        assert_activerecord_metrics(Order, 'select')
+        assert_activerecord_metrics(Order, 'pluck')
       end
+    else
+      assert_activerecord_metrics(Order, 'select')
     end
   end
 

test/multiverse/suites/rails/Envfile

@@ -13,6 +13,8 @@ RAILS_VERSIONS = [
   ['4.2.11', 2.4, 2.4]
 ]
 
+unshift_rails_edge(RAILS_VERSIONS)
+
 def haml_rails(rails_version = nil)
   if rails_version && (
     rails_version.include?('4.0.13') ||

test/multiverse/suites/rails/action_cable_test.rb

@@ -25,6 +25,14 @@ def initialize
         @logger = Logger.new(StringIO.new)
       end
 
+      # In Rails itself, `#config` is delegated via a stub.
+      # See https://github.com/rails/rails/commit/8fff6d609cec2d20972235d3c2cf7d004e2d6983
+      # But seeing as that stub is not distributed in the ActionCable gem, we
+      # use this workaround.
+      def config
+        Rails.application.config
+      end
+
       def transmit(data)
         @transmissions << data
       end

test/multiverse/suites/rails/action_controller_live_rum_test.rb

@@ -5,7 +5,6 @@
 require './app'
 
 if defined?(ActionController::Live)
-
   class UndeadController < ApplicationController
     RESPONSE_BODY = '<html><head></head><body>Brains!</body></html>'
 
@@ -44,6 +43,7 @@ def test_excludes_rum_instrumentation_when_streaming_with_action_controller_live
     def test_excludes_rum_instrumentation_when_streaming_with_action_stream_true
       get('/undead/brain_stream', env: {'HTTP_VERSION' => 'HTTP/1.1'})
 
+      assert_predicate(response, :ok?, 'Expected ActionController streaming response to be OK')
       assert_includes(response.body, UndeadController::RESPONSE_BODY)
       assert_not_includes(response.body, JS_LOADER)
     end

test/multiverse/suites/rails/activejob_test.rb

@@ -98,7 +98,17 @@ def test_code_information_recorded_with_new_transaction
                     namespace: 'MyJob'}
         segment = MiniTest::Mock.new
         segment.expect(:code_information=, nil, [expected])
-        segment.expect(:finish, [])
+        segment.expect(:code_information=,
+          nil,
+          [{transaction_name: 'OtherTransaction/ActiveJob::Inline/MyJob/execute'}])
+        (NewRelic::Agent::Instrumentation::ActiveJobSubscriber::PAYLOAD_KEYS.size + 1).times do
+          segment.expect(:params, {}, [])
+        end
+        3.times do
+          segment.expect(:finish, [])
+        end
+        segment.expect(:record_scoped_metric=, nil, [false])
+        segment.expect(:notice_error, nil, [])
         NewRelic::Agent::Tracer.stub(:start_segment, segment) do
           MyJob.perform_later
         end

test/multiverse/suites/rails/before_suite.rb

@@ -0,0 +1,23 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+# These are hacks to make the 'rails' multiverse test suite compatible with
+# Rails v7.1 released on 2023-10-05.
+#
+# TODO: refactor these out with non-hack replacements as time permits
+
+if Gem::Version.new(Rails.version) >= Gem::Version.new('7.1.0')
+  # NoMethodError (undefined method `to_ary' for an instance of ActionController::Streaming::Body):
+  # actionpack (7.1.0) lib/action_dispatch/http/response.rb:107:in `to_ary'
+  # actionpack (7.1.0) lib/action_dispatch/http/response.rb:509:in `to_ary'
+  # rack (3.0.8) lib/rack/body_proxy.rb:41:in `method_missing'
+  # rack (3.0.8) lib/rack/etag.rb:32:in `call'
+  # newrelic-ruby-agent/lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
+  require 'action_controller/railtie'
+  class ActionController::Streaming::Body
+    def to_ary
+      self
+    end
+  end
+end

test/multiverse/suites/rails/rails3_app/app_rails3_plus.rb

@@ -4,125 +4,9 @@
 
 require 'action_controller/railtie'
 require 'active_model'
-require 'rails/test_help'
 require 'filtering_test_app'
-
-# We define our single Rails application here, one time, upon the first inclusion
-# Tests should feel free to define their own Controllers locally, but if they
-# need anything special at the Application level, put it here
-if !defined?(MyApp)
-
-  ENV['NEW_RELIC_DISPATCHER'] = 'test'
-
-  class NamedMiddleware
-    def initialize(app, options = {})
-      @app = app
-    end
-
-    def call(env)
-      status, headers, body = @app.call(env)
-      headers['NamedMiddleware'] = '1'
-      [status, headers, body]
-    end
-  end
-
-  class InstanceMiddleware
-    attr_reader :name
-
-    def initialize
-      @app = nil
-      @name = 'InstanceMiddleware'
-    end
-
-    def new(app)
-      @app = app
-      self
-    end
-
-    def call(env)
-      status, headers, body = @app.call(env)
-      headers['InstanceMiddleware'] = '1'
-      [status, headers, body]
-    end
-  end
-
-  if defined?(Sinatra)
-    module Sinatra
-      class Application < Base
-        # Override to not accidentally start the app in at_exit handler
-        set :run, proc { false }
-      end
-    end
-
-    class SinatraTestApp < Sinatra::Base
-      get '/' do
-        raise 'Intentional error' if params['raise']
-
-        'SinatraTestApp#index'
-      end
-    end
-  end
-
-  class MyApp < Rails::Application
-    # We need a secret token for session, cookies, etc.
-    config.active_support.deprecation = :log
-    config.secret_token = '49837489qkuweoiuoqwehisuakshdjksadhaisdy78o34y138974xyqp9rmye8yrpiokeuioqwzyoiuxftoyqiuxrhm3iou1hrzmjk'
-    config.eager_load = false
-    config.filter_parameters += [:secret]
-    config.secret_key_base = fake_guid(64)
-    if Rails::VERSION::STRING >= '7.0.0'
-      config.action_controller.default_protect_from_forgery = true
-    end
-    if config.respond_to?(:hosts)
-      config.hosts << 'www.example.com'
-    end
-    initializer 'install_error_middleware' do
-      config.middleware.use(ErrorMiddleware)
-    end
-    initializer 'install_middleware_by_name' do
-      config.middleware.use(NamedMiddleware)
-    end
-    initializer 'install_middleware_instance' do
-      config.middleware.use(InstanceMiddleware.new)
-    end
-  end
-  MyApp.initialize!
-
-  MyApp.routes.draw do
-    get('/bad_route' => 'test#controller_error',
-      :constraints => lambda do |_|
-        raise ActionController::RoutingError.new('this is an uncaught routing error')
-      end)
-
-    mount SinatraTestApp, :at => '/sinatra_app' if defined?(Sinatra)
-
-    post '/filtering_test' => FilteringTestApp.new
-
-    post '/parameter_capture', :to => 'parameter_capture#create'
-
-    get '/:controller(/:action(/:id))'
-  end
-
-  class ApplicationController < ActionController::Base
-    if Rails::VERSION::STRING.to_i >= 7
-      # forgery protection explicitly prevents application/javascript content types
-      # as originating from the same origin
-      # this allows view_instrumentation_test to pass
-      skip_before_action :verify_authenticity_token, only: :js_render
-    end
-
-    # The :text option to render was deprecated in Rails 4.1 in favor of :body.
-    # With the patch below we can write our tests using render :body but have
-    # that converted to render :text for Rails versions that do not support
-    # render :body.
-    if Rails::VERSION::STRING < '4.1.0'
-      def render(*args)
-        options = args.first
-        if Hash === options && options.key?(:body)
-          options[:text] = options.delete(:body)
-        end
-        super
-      end
-    end
-  end
-end
+# NOTE: my_app should be brought in before rails/test_help,
+#       but after filtering_test_app. This is because logic to maintain
+#       the test db schema will expect a Rails app to be in play.
+require_relative 'my_app'
+require 'rails/test_help'