Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 12 commits into from
Oct 11, 2023
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -253,6 +256,7 @@ jobs:
POSTGRES_PASSWORD: password
SERIALIZE: 1
COVERAGE: true
CI_FOR_PR: true

- name: Annotate errors
if: ${{ failure() }}
Expand Down Expand Up @@ -306,6 +310,7 @@ jobs:
VERBOSE_TEST_OUTPUT: true
SERIALIZE: 1
COVERAGE: true
CI_FOR_PR: true

- name: Annotate errors
if: ${{ failure() }}
Expand Down
16 changes: 14 additions & 2 deletions lib/new_relic/control/frameworks/rails.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
28 changes: 28 additions & 0 deletions test/multiverse/lib/multiverse/envfile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very helpful comment! Cool we get to test with the latest now

# 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)
Expand Down
2 changes: 2 additions & 0 deletions test/multiverse/suites/active_record_pg/Envfile
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
18 changes: 10 additions & 8 deletions test/multiverse/suites/active_record_pg/active_record_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions test/multiverse/suites/rails/Envfile
Original file line number Diff line number Diff line change
Expand Up @@ -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') ||
Expand Down
8 changes: 8 additions & 0 deletions test/multiverse/suites/rails/action_cable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
require './app'

if defined?(ActionController::Live)

class UndeadController < ApplicationController
RESPONSE_BODY = '<html><head></head><body>Brains!</body></html>'

Expand Down Expand Up @@ -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
Expand Down
12 changes: 11 additions & 1 deletion test/multiverse/suites/rails/activejob_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 23 additions & 0 deletions test/multiverse/suites/rails/before_suite.rb
Original file line number Diff line number Diff line change
@@ -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
fallwith marked this conversation as resolved.
Show resolved Hide resolved

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
fallwith marked this conversation as resolved.
Show resolved Hide resolved
self
end
end
end
126 changes: 5 additions & 121 deletions test/multiverse/suites/rails/rails3_app/app_rails3_plus.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Loading