From f5873420bd633873563b1209046cf42a0332f63b Mon Sep 17 00:00:00 2001 From: Samuel Cochran Date: Sun, 8 Aug 2021 20:18:04 +1000 Subject: [PATCH 1/4] Test on rails 6.1 --- .github/workflows/ci.yml | 2 +- Gemfile | 2 +- README.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b73fd0a..f82b7f2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,7 +10,7 @@ jobs: fail-fast: false matrix: ruby-version: ["2.6", "2.7", "3.0"] - rails-version: ["5.2.0", "6.0.0"] + rails-version: ["5.2.0", "6.0.0", "6.1.0"] exclude: # Rails 5.2 does not support Ruby 3.0 - {ruby-version: "3.0", rails-version: "5.2.0"} diff --git a/Gemfile b/Gemfile index 628fa9b..cafd801 100644 --- a/Gemfile +++ b/Gemfile @@ -2,7 +2,7 @@ source "https://rubygems.org" gemspec -rails_version = ENV["RAILS_VERSION"] || "6.0.0" +rails_version = ENV["RAILS_VERSION"] || "6.1.0" if rails_version == "main" gem "rails", github: "rails/rails" else diff --git a/README.md b/README.md index 2327459..db2e04c 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ This gem was created at 37signals. You can read more about how we use it [on our blog](http://37signals.com/svn/posts/3130-tech-note-mysql-query-comments-in-rails). This has been tested and used in production with the mysql2 and pg gems, and is -tested on Rails 5.2 through 6.0, and Ruby 2.6 through 3.0. It is also tested +tested on Rails 5.2 through 6.1, and Ruby 2.6 through 3.0. It is also tested for sqlite3. Rails version support will follow supported versions in the [Ruby on Rails maintenance policy](https://guides.rubyonrails.org/maintenance_policy.html) From 32ed891848c106f8e5c8e90a1f992f98d66bd9ae Mon Sep 17 00:00:00 2001 From: Samuel Cochran Date: Sun, 8 Aug 2021 20:34:20 +1000 Subject: [PATCH 2/4] Use native ruby String#start_with? Rails 6.1 removes the String#starts_with? shim from ActiveSupport. --- lib/marginalia/comment.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/marginalia/comment.rb b/lib/marginalia/comment.rb index a598513..7673174 100644 --- a/lib/marginalia/comment.rb +++ b/lib/marginalia/comment.rb @@ -123,7 +123,7 @@ def self.line else "" end - if last_line.starts_with? root + if last_line.start_with? root last_line = last_line[root.length..-1] end last_line From ddd21f7e5e6d192efa773e5d99bf2edb48155826 Mon Sep 17 00:00:00 2001 From: Samuel Cochran Date: Thu, 15 Aug 2019 16:55:28 +1000 Subject: [PATCH 3/4] Add support for rails 6.1+ ActiveRecord connection pooling was re-jigged in rails/rails#36371 and landed in v6.1.0. Connection pools quack a little differently. They might be a NullPool instead of a nil. And they have a #db_config, not a #spec. --- lib/marginalia/comment.rb | 14 +++++++++++--- test/query_comments_test.rb | 32 ++++++++++++++++++++++++-------- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/lib/marginalia/comment.rb b/lib/marginalia/comment.rb index 7673174..f4a77de 100644 --- a/lib/marginalia/comment.rb +++ b/lib/marginalia/comment.rb @@ -162,9 +162,17 @@ def self.database end end - def self.connection_config - return if marginalia_adapter.pool.nil? - marginalia_adapter.pool.spec.config + if Gem::Version.new(ActiveRecord::VERSION::STRING) < Gem::Version.new('6.1') + def self.connection_config + return if marginalia_adapter.pool.nil? + marginalia_adapter.pool.spec.config + end + else + def self.connection_config + # `pool` might be a NullPool which has no db_config + return unless marginalia_adapter.pool.respond_to?(:db_config) + marginalia_adapter.pool.db_config.configuration_hash + end end def self.inline_annotations diff --git a/test/query_comments_test.rb b/test/query_comments_test.rb index bc72920..487d733 100644 --- a/test/query_comments_test.rb +++ b/test/query_comments_test.rb @@ -5,6 +5,10 @@ def using_rails_api? ENV["TEST_RAILS_API"] == true end +def pool_db_config? + Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new('6.1') +end + require "minitest/autorun" require "mocha/minitest" require 'logger' @@ -230,14 +234,26 @@ def test_database assert_match %r{/\*database:marginalia_test}, @queries.first end - def test_socket - # setting socket in configuration would break some connections - mock it instead - pool = ActiveRecord::Base.connection_pool - pool.spec.stubs(:config).returns({:socket => "marginalia_socket"}) - Marginalia::Comment.components = [:socket] - API::V1::PostsController.action(:driver_only).call(@env) - assert_match %r{/\*socket:marginalia_socket}, @queries.first - pool.spec.unstub(:config) + if pool_db_config? + def test_socket + # setting socket in configuration would break some connections - mock it instead + pool = ActiveRecord::Base.connection_pool + pool.db_config.stubs(:configuration_hash).returns({:socket => "marginalia_socket"}) + Marginalia::Comment.components = [:socket] + API::V1::PostsController.action(:driver_only).call(@env) + assert_match %r{/\*socket:marginalia_socket}, @queries.first + pool.db_config.unstub(:configuration_hash) + end + else + def test_socket + # setting socket in configuration would break some connections - mock it instead + pool = ActiveRecord::Base.connection_pool + pool.spec.stubs(:config).returns({:socket => "marginalia_socket"}) + Marginalia::Comment.components = [:socket] + API::V1::PostsController.action(:driver_only).call(@env) + assert_match %r{/\*socket:marginalia_socket}, @queries.first + pool.spec.unstub(:config) + end end def test_request_id From 0afefbca0d55128c52818b8a604d852d8cceb15f Mon Sep 17 00:00:00 2001 From: Samuel Cochran Date: Sun, 8 Aug 2021 21:14:11 +1000 Subject: [PATCH 4/4] Avoid spurious queries during testing The order seems to have changed in rails 6.1 --- test/query_comments_test.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/query_comments_test.rb b/test/query_comments_test.rb index 487d733..a5cb9c9 100644 --- a/test/query_comments_test.rb +++ b/test/query_comments_test.rb @@ -101,6 +101,9 @@ def driver_only class MarginaliaTest < MiniTest::Test def setup + # Touch the model to avoid spurious schema queries + Post.first + @queries = [] ActiveSupport::Notifications.subscribe "sql.active_record" do |*args| @queries << args.last[:sql]