ronantakizawa/codereview-bench huggingface dataset:
Repository: rails/rails
File: activerecord/test/cases/query_cache_test.rb (Ruby)

Code before:

          middleware {
            assert_cache :clean

            Task.find 1
            assert_cache :dirty

            started.set
            checked.wait

            ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
          }.call({})

          assert_cache :off, thread_2_connection
        }

        started.wait

        thread_1_connection = ActiveRecord::Base.lease_connection
        assert_not_equal thread_1_connection, thread_2_connection
        checked.set
        thread.join
      }.call({})

      ActiveRecord::Base.connection_pool.connections.each do |conn|
        assert_cache :off, conn
      end
    end
  end

  def test_middleware_delegates
    called = false
    mw = middleware { |env|
      called = true
      [200, {}, nil]
    }
    mw.call({})
    assert called, "middleware should delegate"
  end

  def test_middleware_caches
    mw = middleware { |env|
      Task.find 1
      Task.find 1
      query_cache = ActiveRecord::Base.connection_pool.query_cache
      assert_equal 1, query_cache.size, query_cache.inspect
      [200, {}, nil]
    }
    mw.call({})
  end


Review comment (suggestion):
Since the new behavior only applies to pinned connections, we can actually restore this test to what it was and it should still pass.

Though I guess this is also fine.

Code after:
          assert_cache :off

          middleware {
            assert_cache :clean

            Task.find 1
            assert_cache :dirty

            started.set
            checked.wait

            ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
          }.call({})
        }

        started.wait

        thread_1_connection = ActiveRecord::Base.lease_connection
        assert_not_equal thread_1_connection, thread_2_connection
        assert_cache :dirty, thread_2_connection
        checked.set
        thread.join

        assert_cache :off, thread_2_connection
      }.call({})

      ActiveRecord::Base.connection_pool.connections.each do |conn|
        assert_cache :off, conn
      end
    end
  end

  def test_middleware_delegates
    called = false
    mw = middleware { |env|
      called = true
      [200, {}, nil]
    }
    mw.call({})
    assert called, "middleware should delegate"
  end

  def test_middleware_caches
    mw = middleware { |env|
      Task.find 1
      Task.find 1
      query_cache = ActiveRecord::Base.connection_pool.query_cache
      assert_equal 1, query_cache.size, query_cache.inspect
      [200, {}, nil]
    }
    mw.call({})
Repository: rails/rails
File: activesupport/lib/active_support/core_ext/date_and_time/calculations.rb (Ruby)

Code before:
    end
    alias :prev_day? :yesterday?

    # Returns true if the date/time is in the past.
    def past?
      self < self.class.current
    end

    # Returns true if the date/time is in the future.
    def future?
      self > self.class.current
    end

    # Returns true if the date/time falls on a Saturday or Sunday.
    def on_weekend?
      WEEKEND_DAYS.include?(wday)
    end

    # Returns true if the date/time does not fall on a Saturday or Sunday.
    def on_weekday?
      !WEEKEND_DAYS.include?(wday)
    end

    # Returns true if the date/time falls within the current week.
    def this_week?
      ::Date.current.all_week.include?(to_date)
    end
    alias_method :in_this_week?, :this_week?

    # Returns true if the date/time falls within the current month.
    def this_month?
      ::Date.current.all_month.include?(to_date)
    end
    alias_method :in_this_month?, :this_month?

    # Returns true if the date/time falls within the current year.
    def this_year?
      ::Date.current.all_year.include?(to_date)
    end
    alias_method :in_this_year?, :this_year?

    # Returns true if the date/time falls before <tt>date_or_time</tt>.
    def before?(date_or_time)
      self < date_or_time
    end

    # Returns true if the date/time falls after <tt>date_or_time</tt>.
    def after?(date_or_time)
      self > date_or_time
    end


Review comment (suggestion):
I thought comparing as date made sense for these units. Unless we plan to add `current_hour?` too, which then it would be best to convert `to_time` instead.

Code after:
    end
    alias :prev_day? :yesterday?

    # Returns true if the date/time is in the past.
    def past?
      self < self.class.current
    end

    # Returns true if the date/time is in the future.
    def future?
      self > self.class.current
    end

    # Returns true if the date/time falls on a Saturday or Sunday.
    def on_weekend?
      WEEKEND_DAYS.include?(wday)
    end

    # Returns true if the date/time does not fall on a Saturday or Sunday.
    def on_weekday?
      !WEEKEND_DAYS.include?(wday)
    end

    # Returns true if the date/time falls within the current week.
    def this_week?
      ::Date.current.all_week.include?(to_date)
    end

    # Returns true if the date/time falls within the current month.
    def this_month?
      ::Date.current.all_month.include?(to_date)
    end

    # Returns true if the date/time falls within the current year.
    def this_year?
      ::Date.current.all_year.include?(to_date)
    end

    # Returns true if the date/time falls before <tt>date_or_time</tt>.
    def before?(date_or_time)
      self < date_or_time
    end

    # Returns true if the date/time falls after <tt>date_or_time</tt>.
    def after?(date_or_time)
      self > date_or_time
    end

    # Returns a new date/time the specified number of days ago.
    def days_ago(days)
      advance(days: -days)
Repository: rails/rails
File: railties/lib/rails/engine.rb (Ruby)

Code before:
            if Rake::Task.task_defined?("railties:install:migrations")
              Rake::Task["railties:install:migrations"].invoke
            else
              Rake::Task["app:railties:install:migrations"].invoke
            end
          end
        end
      end
    end

    def routes? # :nodoc:
      @routes
    end

    protected
      def run_tasks_blocks(*) # :nodoc:
        super
        paths["lib/tasks"].existent.sort.each { |ext| load(ext) }
      end

    private
      def missing_environment_file; end

      def any_environment_files?
        []
      end

      def load_config_initializer(initializer) # :doc:
        ActiveSupport::Notifications.instrument("load_config_initializer.railties", initializer: initializer) do
          load(initializer)
        end
      end

      def has_migrations?
        paths["db/migrate"].existent.any?
      end

      def self.find_root_with_flag(flag, root_path, default = nil) # :nodoc:
        while root_path && File.directory?(root_path) && !File.exist?("#{root_path}/#{flag}")
          parent = File.dirname(root_path)
          root_path = parent != root_path && parent
        end

        root = File.exist?("#{root_path}/#{flag}") ? root_path : default
        raise "Could not find root path for #{self}" unless root

        Pathname.new File.realpath root
      end

      def default_middleware_stack
        ActionDispatch::MiddlewareStack.new

Review comment (suggestion):
```suggestion
      def any_environment_files?
        false
      end
```

Code after:
            if Rake::Task.task_defined?("railties:install:migrations")
              Rake::Task["railties:install:migrations"].invoke
            else
              Rake::Task["app:railties:install:migrations"].invoke
            end
          end
        end
      end
    end

    def routes? # :nodoc:
      @routes
    end

    protected
      def run_tasks_blocks(*) # :nodoc:
        super
        paths["lib/tasks"].existent.sort.each { |ext| load(ext) }
      end

    private
      def missing_environment_file; end

      def any_environment_files?
        false
      end

      def load_config_initializer(initializer) # :doc:
        ActiveSupport::Notifications.instrument("load_config_initializer.railties", initializer: initializer) do
          load(initializer)
        end
      end

      def has_migrations?
        paths["db/migrate"].existent.any?
      end

      def self.find_root_with_flag(flag, root_path, default = nil) # :nodoc:
        while root_path && File.directory?(root_path) && !File.exist?("#{root_path}/#{flag}")
          parent = File.dirname(root_path)
          root_path = parent != root_path && parent
        end

        root = File.exist?("#{root_path}/#{flag}") ? root_path : default
        raise "Could not find root path for #{self}" unless root

        Pathname.new File.realpath root
      end

      def default_middleware_stack
        ActionDispatch::MiddlewareStack.new
Repository: rails/rails
File: actionpack/lib/action_controller/metal/live.rb (Ruby)

Code before:

      def initialize(stream, options = {})
        @stream = stream
        @options = options
      end

      def close
        @stream.close
      end

      def write(object, options = {})
        case object
        when String
          perform_write(object, options)
        else
          perform_write(ActiveSupport::JSON.encode(object), options)
        end
      end

      private
        def perform_write(json, options)
          current_options = @options.merge(options).stringify_keys
          # Build the full SSE payload in a single mutable String so we only
          # perform one underlying @stream.write per SSE event. Use a mutable
          # String (String.new) because frozen_string_literal is enabled.
          event = String.new
          PERMITTED_OPTIONS.each do |option_name|
            if (option_value = current_options[option_name])
              event << "#{option_name}: #{option_value}\n"
            end
          end

          message = json.gsub("\n", "\ndata: ")
          event << "data: #{message}\n\n"
          @stream.write event
        end
    end

    class ClientDisconnected < RuntimeError
    end

    class Buffer < ActionDispatch::Response::Buffer # :nodoc:
      include MonitorMixin

      class << self
        attr_accessor :queue_size
      end
      @queue_size = 10

      # Ignore that the client has disconnected.
      #

Review comment (suggestion):
```suggestion
          event = +""
```
Otherwise that string will be binary encoded which may cause issues with some servers. AFAIK SSE are UTF-8.

Code after:

      def initialize(stream, options = {})
        @stream = stream
        @options = options
      end

      def close
        @stream.close
      end

      def write(object, options = {})
        case object
        when String
          perform_write(object, options)
        else
          perform_write(ActiveSupport::JSON.encode(object), options)
        end
      end

      private
        def perform_write(json, options)
          current_options = @options.merge(options).stringify_keys
          event = +""
          PERMITTED_OPTIONS.each do |option_name|
            if (option_value = current_options[option_name])
              event << "#{option_name}: #{option_value}\n"
            end
          end

          message = json.gsub("\n", "\ndata: ")
          event << "data: #{message}\n\n"
          @stream.write event
        end
    end

    class ClientDisconnected < RuntimeError
    end

    class Buffer < ActionDispatch::Response::Buffer # :nodoc:
      include MonitorMixin

      class << self
        attr_accessor :queue_size
      end
      @queue_size = 10

      # Ignore that the client has disconnected.
      #
      # If this value is `true`, calling `write` after the client disconnects will
      # result in the written content being silently discarded. If this value is
      # `false` (the default), a ClientDisconnected exception will be raised.
Repository: rails/rails
File: actionpack/lib/action_controller/metal/live.rb (Ruby)

Code before:
      PERMITTED_OPTIONS = %w( retry event id )

      def initialize(stream, options = {})
        @stream = stream
        @options = options
      end

      def close
        @stream.close
      end

      def write(object, options = {})
        case object
        when String
          perform_write(object, options)
        else
          perform_write(ActiveSupport::JSON.encode(object), options)
        end
      end

      private
        def perform_write(json, options)
          current_options = @options.merge(options).stringify_keys
          # Build the full SSE payload in a single mutable String so we only
          # perform one underlying @stream.write per SSE event. Use a mutable
          # String (String.new) because frozen_string_literal is enabled.
          event = String.new
          PERMITTED_OPTIONS.each do |option_name|
            if (option_value = current_options[option_name])
              event << "#{option_name}: #{option_value}\n"
            end
          end

          message = json.gsub("\n", "\ndata: ")
          event << "data: #{message}\n\n"
          @stream.write event
        end
    end

    class ClientDisconnected < RuntimeError
    end

    class Buffer < ActionDispatch::Response::Buffer # :nodoc:
      include MonitorMixin

      class << self
        attr_accessor :queue_size
      end
      @queue_size = 10

      # Ignore that the client has disconnected.

Review comment (suggestion):
```suggestion
```

That comment isn't necessary.

Code after:
      PERMITTED_OPTIONS = %w( retry event id )

      def initialize(stream, options = {})
        @stream = stream
        @options = options
      end

      def close
        @stream.close
      end

      def write(object, options = {})
        case object
        when String
          perform_write(object, options)
        else
          perform_write(ActiveSupport::JSON.encode(object), options)
        end
      end

      private
        def perform_write(json, options)
          current_options = @options.merge(options).stringify_keys
          event = +""
          PERMITTED_OPTIONS.each do |option_name|
            if (option_value = current_options[option_name])
              event << "#{option_name}: #{option_value}\n"
            end
          end

          message = json.gsub("\n", "\ndata: ")
          event << "data: #{message}\n\n"
          @stream.write event
        end
    end

    class ClientDisconnected < RuntimeError
    end

    class Buffer < ActionDispatch::Response::Buffer # :nodoc:
      include MonitorMixin

      class << self
        attr_accessor :queue_size
      end
      @queue_size = 10

      # Ignore that the client has disconnected.
      #
      # If this value is `true`, calling `write` after the client disconnects will
      # result in the written content being silently discarded. If this value is
Repository: rails/rails
File: railties/test/structured_event_subscriber_test.rb (Ruby)

Code before:
# frozen_string_literal: true

require "abstract_unit"
require "active_support/testing/event_reporter_assertions"
require "rails/structured_event_subscriber"

module Rails
  class StructuredEventSubscriberTest < ActionMailer::TestCase
    include ActiveSupport::Testing::EventReporterAssertions

    def run(*)
      ActiveSupport.event_reporter.with_debug do
        super
      end
    end

    def test_deprecation_is_notified_when_behavior_is_notify
      Rails.deprecator.with(behavior: :notify) do
        event = assert_event_reported("rails.deprecation", payload: { gem_name: "Rails" }) do
          Rails.deprecator.warn("This is a deprecation warning")
        end

        assert_includes event[:payload][:message], "This is a deprecation warning"
        assert_includes event[:payload].keys, :callstack
        assert_includes event[:payload].keys, :gem_name
        assert_includes event[:payload].keys, :deprecation_horizon
      end
    end

    def test_deprecation_is_not_notified_when_behavior_is_not_notify
      Rails.deprecator.with(behavior: :stderr) do
        output = capture(:stderr) do
          assert_no_event_reported("rails.deprecation") do

Review comment (suggestion):
```suggestion
  class StructuredEventSubscriberTest < ActiveSupport::TestCase
```

Code after:
# frozen_string_literal: true

require "abstract_unit"
require "active_support/testing/event_reporter_assertions"
require "rails/structured_event_subscriber"

module Rails
  class StructuredEventSubscriberTest < ActiveSupport::TestCase
    include ActiveSupport::Testing::EventReporterAssertions

    def run(*)
      ActiveSupport.event_reporter.with_debug do
        super
      end
    end

    def test_deprecation_is_notified_when_behavior_is_notify
      Rails.deprecator.with(behavior: :notify) do
        event = assert_event_reported("rails.deprecation", payload: { gem_name: "Rails" }) do
          Rails.deprecator.warn("This is a deprecation warning")
        end

        assert_includes event[:payload][:message], "This is a deprecation warning"
        assert_includes event[:payload].keys, :callstack
        assert_includes event[:payload].keys, :gem_name
        assert_includes event[:payload].keys, :deprecation_horizon
      end
    end

    def test_deprecation_is_not_notified_when_behavior_is_not_notify
      Rails.deprecator.with(behavior: :stderr) do
        output = capture(:stderr) do
          assert_no_event_reported("rails.deprecation") do
Repository: rails/rails
File: activerecord/test/cases/insert_all_test.rb (Ruby)

Code before:
    author = Author.create!(name: "Jimmy")

    assert_difference "author.books.count", +1 do
      author.books.upsert_all([{ name: "My little book", isbn: "1974522598" }])
    end
  end

  def test_upsert_all_on_relation_precedence
    skip unless supports_insert_on_duplicate_update?

    author = Author.create!(name: "Jimmy")
    second_author = Author.create!(name: "Bob")

    assert_difference "author.books.count", +1 do
      author.books.upsert_all([{ name: "My little book", isbn: "1974522598", author_id: second_author.id }])
    end
  end

  def test_upsert_with_repeated_timstamp_columns
    records = [Book.first.attributes]

    Book.upsert_all(
      records,
      update_only: [:updated_at],
      record_timestamps: true,
    )
  end

  def test_upsert_all_resets_relation
    skip unless supports_insert_on_duplicate_update?

    audit_logs = Developer.create!(name: "Alice").audit_logs.load

    assert_changes "audit_logs.loaded?", from: true, to: false do
      audit_logs.upsert_all([{ id: 1, message: "event" }])
    end
  end

  def test_upsert_all_create_with
    skip unless supports_insert_on_duplicate_update?

    assert_difference "Book.where(format: 'X').count", +2 do
      Book.create_with(format: "X").upsert_all([ { name: "A" }, { name: "B" } ])
    end
  end

  def test_upsert_all_has_many_through
    skip unless supports_insert_on_duplicate_update?

    book = Book.first
    assert_raise(ArgumentError) { book.subscribers.upsert_all([ { nick: "Jimmy" } ]) }

Review comment (bug):
The build is failing because we require tests to have an assertion.

Code after:
    author = Author.create!(name: "Jimmy")

    assert_difference "author.books.count", +1 do
      author.books.upsert_all([{ name: "My little book", isbn: "1974522598" }])
    end
  end

  def test_upsert_all_on_relation_precedence
    skip unless supports_insert_on_duplicate_update?

    author = Author.create!(name: "Jimmy")
    second_author = Author.create!(name: "Bob")

    assert_difference "author.books.count", +1 do
      author.books.upsert_all([{ name: "My little book", isbn: "1974522598", author_id: second_author.id }])
    end
  end

  def test_upsert_with_repeated_timstamp_columns
    records = [Book.first.attributes]

    assert_no_changes(proc { Book.count }) do
      Book.upsert_all(
        records,
        update_only: [:updated_at],
        record_timestamps: true,
      )
    end
  end

  def test_upsert_all_resets_relation
    skip unless supports_insert_on_duplicate_update?

    audit_logs = Developer.create!(name: "Alice").audit_logs.load

    assert_changes "audit_logs.loaded?", from: true, to: false do
      audit_logs.upsert_all([{ id: 1, message: "event" }])
    end
  end

  def test_upsert_all_create_with
    skip unless supports_insert_on_duplicate_update?

    assert_difference "Book.where(format: 'X').count", +2 do
      Book.create_with(format: "X").upsert_all([ { name: "A" }, { name: "B" } ])
    end
  end

  def test_upsert_all_has_many_through
    skip unless supports_insert_on_duplicate_update?
Repository: rails/rails
File: activemodel/test/cases/secure_password_test.rb (Ruby)

Code before:
require "models/slow_pilot"
require "models/visitor"

class CustomAlgorithm
  def algorithm_name
    :custom_algorithm
  end

  def validate(record, attribute)
    # No-op for testing
  end

  def hash_password(unencrypted_password)
    "hashed-#{unencrypted_password}"
  end

  def verify_password(password, digest)
    digest == "hashed-#{password}"
  end

  def password_salt(digest)
    "custom-salt"
  end
end

class UserWithCustomAlgorithim < User
  has_secure_password algorithm: CustomAlgorithm.new
end

class SecurePasswordTest < ActiveModel::TestCase
  setup do
    # Used only to speed up tests
    @original_min_cost = ActiveModel::SecurePassword.min_cost
    ActiveModel::SecurePassword.min_cost = true

    @user = User.new
    @visitor = Visitor.new
    @pilot = Pilot.new
    @slow_pilot = SlowPilot.new

    # Simulate loading an existing user from the DB
    @existing_user = User.new
    @existing_user.password_digest = BCrypt::Password.create("password", cost: BCrypt::Engine::MIN_COST)
    @existing_user.changes_applied
  end

  teardown do
    ActiveModel::SecurePassword.min_cost = @original_min_cost
  end

  test "automatically include ActiveModel::Validations when validations are enabled" do

Review comment (suggestion):
```suggestion
class UserWithCustomAlgorithm < User
```

Code after:
require "models/slow_pilot"
require "models/visitor"

class CustomAlgorithm
  def algorithm_name
    :custom_algorithm
  end

  def validate(record, attribute)
    # No-op for testing
  end

  def hash_password(unencrypted_password)
    "hashed-#{unencrypted_password}"
  end

  def verify_password(password, digest)
    digest == "hashed-#{password}"
  end

  def password_salt(digest)
    "custom-salt"
  end
end

class UserWithCustomAlgorithm < User
  has_secure_password algorithm: CustomAlgorithm.new
end

class SecurePasswordTest < ActiveModel::TestCase
  setup do
    # Used only to speed up tests
    @original_min_cost = ActiveModel::SecurePassword.min_cost
    ActiveModel::SecurePassword.min_cost = true

    @user = User.new
    @visitor = Visitor.new
    @pilot = Pilot.new
    @slow_pilot = SlowPilot.new

    # Simulate loading an existing user from the DB
    @existing_user = User.new
    @existing_user.password_digest = BCrypt::Password.create("password", cost: BCrypt::Engine::MIN_COST)
    @existing_user.changes_applied
  end

  teardown do
    ActiveModel::SecurePassword.min_cost = @original_min_cost
  end

  test "automatically include ActiveModel::Validations when validations are enabled" do