ronantakizawa/codereview-bench huggingface dataset:
Repository: rails/rails
File: activesupport/lib/active_support/continuous_integration.rb (Ruby)

Code before:
    def echo(text, type:)
      puts colorize(text, type)
    end

    # :nodoc:
    def report(title, &block)
      Signal.trap("INT") { abort colorize(:error, "\n❌ #{title} interrupted") }

      ci = self.class.new
      elapsed = timing { ci.instance_eval(&block) }

      if ci.success?
        echo "\n✅ #{title} passed in #{elapsed}", type: :success
      else
        echo "\n❌ #{title} failed in #{elapsed}", type: :error
        ci.failure_summary(title)
      end

      results.concat ci.results
    ensure
      Signal.trap("INT", "-")
    end

    # :nodoc:
    def failure_summary(title)
      return if success? || results.size < 2

      results.each do |success, title|
        unless success
          echo "   ↳ #{title} failed", type: :error
        end
      end
    end

    private
      def timing
        started_at = Time.now.to_f
        yield
        min, sec = (Time.now.to_f - started_at).divmod(60)
        "#{"#{min}m" if min > 0}%.2fs" % sec
      end

      def colorize(text, type)
        "#{COLORS.fetch(type)}#{text}\033[0m"
      end
  end
end


Review comment (bug):
We're already on the failure branch of the former conditional, so think we can drop `success?` and I also think we should probably extract `results.size < 2` as `multiple_steps?` and put that into the caller, so it's `ci.failure_summary(title) if multiple_steps?`, so it's clear where the presentation is being made whether it needs a summary. 

Code after:
    def echo(text, type:)
      puts colorize(text, type)
    end

    # :nodoc:
    def report(title, &block)
      Signal.trap("INT") { abort colorize(:error, "\n❌ #{title} interrupted") }

      ci = self.class.new
      elapsed = timing { ci.instance_eval(&block) }

      if ci.success?
        echo "\n✅ #{title} passed in #{elapsed}", type: :success
      else
        echo "\n❌ #{title} failed in #{elapsed}", type: :error

        if ci.multiple_results?
          ci.failures.each do |success, title|
            unless success
              echo "   ↳ #{title} failed", type: :error
            end
          end
        end
      end

      results.concat ci.results
    ensure
      Signal.trap("INT", "-")
    end

    # :nodoc:
    def failures
      results.reject(&:first)
    end

    # :nodoc:
    def multiple_results?
      results.size > 1
    end

    private
      def timing
        started_at = Time.now.to_f
        yield
        min, sec = (Time.now.to_f - started_at).divmod(60)
        "#{"#{min}m" if min > 0}%.2fs" % sec
      end

      def colorize(text, type)
        "#{COLORS.fetch(type)}#{text}\033[0m"
      end
Repository: rails/rails
File: activejob/lib/active_job/serializers.rb (Ruby)

Code before:
        def index_serializers(new_serializers)
          new_serializers.each do |s|
            if s.respond_to?(:klass)
              @serializers_index[s.klass] = s
            elsif s.respond_to?(:klass, true)
              klass = s.send(:klass)
              ActiveJob.deprecator.warn(<<~MSG.squish)
                #{s.class.name}#klass method should be public.
                This will raise an error in Rails 8.2.
              MSG
              @serializers_index[klass] = s
            else
              ActiveJob.deprecator.warn(
                <<~MSG.squish
                  #{s.class.name} should implement a public #klass method.
                  This will raise an error in Rails 8.2.
                MSG
              )
            end
          end
        end
    end

    private
      # :nodoc:
      OBJECT_SERIALIZER_KEY = "_aj_serialized"

      add_serializers SymbolSerializer.instance,
        DurationSerializer.instance,
        DateTimeSerializer.instance,
        DateSerializer.instance,
        TimeWithZoneSerializer.instance,
        TimeSerializer.instance,
        ModuleSerializer.instance,
        RangeSerializer.instance,
        BigDecimalSerializer.instance
  end
end


Review comment (suggestion):
This isn't private, so why move here? `private` doesn't change the constant visibility. Also, neither do anything with `add_serializers` 

Code after:
        def index_serializers(new_serializers)
          new_serializers.each do |s|
            if s.respond_to?(:klass)
              @serializers_index[s.klass] = s
            elsif s.respond_to?(:klass, true)
              klass = s.send(:klass)
              ActiveJob.deprecator.warn(<<~MSG.squish)
                #{s.class.name}#klass method should be public.
                This will raise an error in Rails 8.2.
              MSG
              @serializers_index[klass] = s
            else
              ActiveJob.deprecator.warn(
                <<~MSG.squish
                  #{s.class.name} should implement a public #klass method.
                  This will raise an error in Rails 8.2.
                MSG
              )
            end
          end
        end
    end

    # :nodoc:
    OBJECT_SERIALIZER_KEY = "_aj_serialized"

    add_serializers SymbolSerializer.instance,
      DurationSerializer.instance,
      DateTimeSerializer.instance,
      DateSerializer.instance,
      TimeWithZoneSerializer.instance,
      TimeSerializer.instance,
      ModuleSerializer.instance,
      RangeSerializer.instance,
      BigDecimalSerializer.instance
  end
end
Repository: rails/rails
File: railties/test/application/active_job_railtie_test.rb (Ruby)

Code before:
# frozen_string_literal: true

require "isolation/abstract_unit"

module ApplicationTests
  class ActiveJobRailtieTest < ActiveSupport::TestCase
    include ActiveSupport::Testing::Isolation

    setup :build_app
    teardown :teardown_app

    test "jobs can set 'enqueue_after_transaction_commit' when eager_load is true" do
      add_to_env_config "development", "config.eager_load = true"

      app_file "app/jobs/foo_job.rb", <<-RUBY
        class FooJob < ActiveJob::Base
          self.enqueue_after_transaction_commit = false
        end
      RUBY

      assert_equal "false", rails("runner", "p FooJob.enqueue_after_transaction_commit").strip
    end

    test "custom serializers are loaded for Arguments#serialize" do
      app_file "config/initializers/custom_serializers.rb", <<-RUBY
      class Money; end

      class MoneySerializer < ActiveJob::Serializers::ObjectSerializer
        def klass = Money
        def serialize(money) = {}
        def deserialize(hash) = Money.new
      end

      Rails.configuration.active_job.custom_serializers << MoneySerializer
      RUBY

      app "development"

      assert_equal([{}], ActiveJob::Arguments.serialize([Money.new]))
    end
  end
end


Review comment (suggestion):
Super small nitpick but maybe `<<~RUBY` instead? Would allow for indentation of the code for easier reading

Code after:
# frozen_string_literal: true

require "isolation/abstract_unit"

module ApplicationTests
  class ActiveJobRailtieTest < ActiveSupport::TestCase
    include ActiveSupport::Testing::Isolation

    setup :build_app
    teardown :teardown_app

    test "jobs can set 'enqueue_after_transaction_commit' when eager_load is true" do
      add_to_env_config "development", "config.eager_load = true"

      app_file "app/jobs/foo_job.rb", <<-RUBY
        class FooJob < ActiveJob::Base
          self.enqueue_after_transaction_commit = false
        end
      RUBY

      assert_equal "false", rails("runner", "p FooJob.enqueue_after_transaction_commit").strip
    end

    test "custom serializers are loaded for Arguments#serialize" do
      app_file "config/initializers/custom_serializers.rb", <<~RUBY
        class Money; end

        class MoneySerializer < ActiveJob::Serializers::ObjectSerializer
          def klass = Money
          def serialize(money) = {}
          def deserialize(hash) = Money.new
        end

        Rails.configuration.active_job.custom_serializers << MoneySerializer
      RUBY

      app "development"

      assert_equal([{}], ActiveJob::Arguments.serialize([Money.new]))
    end
  end
end
Repository: rails/rails
File: activerecord/lib/active_record/connection_adapters/column.rb (Ruby)

Code before:
          collation == other.collation &&
          comment == other.comment
      end
      alias :eql? :==

      def hash
        Column.hash ^
          name.hash ^
          name.encoding.hash ^
          default.hash ^
          sql_type_metadata.hash ^
          null.hash ^
          default_function.hash ^
          collation.hash ^
          comment.hash
      end

      def virtual?
        false
      end

      private
        def deduplicated
          @name = -name
          @sql_type_metadata = sql_type_metadata.deduplicate if sql_type_metadata
          @default = -default if default
          @default_function = -default_function if default_function
          @collation = -collation if collation
          @comment = -comment if comment
          super
        end
    end

    class NullColumn < Column
      def initialize(name, **)
        super(name, nil)
      end
    end
  end
  # :startdoc:
end


Review comment (suggestion):
```suggestion
          @default = String === default ? -default : default
```

This is a bit defensive, but I think we should consider `default` could be anything.

Code after:
          collation == other.collation &&
          comment == other.comment
      end
      alias :eql? :==

      def hash
        Column.hash ^
          name.hash ^
          name.encoding.hash ^
          default.hash ^
          sql_type_metadata.hash ^
          null.hash ^
          default_function.hash ^
          collation.hash ^
          comment.hash
      end

      def virtual?
        false
      end

      private
        def deduplicated
          @name = -name
          @sql_type_metadata = sql_type_metadata.deduplicate if sql_type_metadata
          @default = -default if String === default
          @default_function = -default_function if default_function
          @collation = -collation if collation
          @comment = -comment if comment
          super
        end
    end

    class NullColumn < Column
      def initialize(name, **)
        super(name, nil)
      end
    end
  end
  # :startdoc:
end
Repository: rails/rails
File: activesupport/lib/active_support/core_ext/string/strip.rb (Ruby)

Code before:
  #     puts <<-USAGE.strip_heredoc
  #       This command does such and such.
  #
  #       Supported options are:
  #         -h         This message
  #         ...
  #     USAGE
  #   end
  #
  # the user would see the usage message aligned against the left margin.
  #
  # Technically, it looks for the least indented non-empty line
  # in the whole string, and removes that amount of leading whitespace.
  def strip_heredoc
    scanner = StringScanner.new(self)
    min_indent_length = Float::INFINITY

    until scanner.eos?
      indent_length = scanner.skip(/[ \t]*/)

      min_indent_length = [min_indent_length, indent_length].min if scanner.match?(/[^\r\n]/)

      scanner.skip_until(/\n/) || scanner.terminate
    end

    return self if min_indent_length.zero? || min_indent_length.infinite?

    scanner.reset
    new_string = +""
    indent_pattern = /[ \t]{#{min_indent_length}}/

    until scanner.eos?
      scanner.skip(indent_pattern)
      line_rest = scanner.scan_until(/\n/)
      new_string << (line_rest || scanner.rest)

      scanner.terminate if line_rest.nil?
    end

    new_string.tap { |stripped| stripped.freeze if frozen? }
  end
end


Review comment (suggestion):
```suggestion
    if min_indent_length.zero? || min_indent_length.infinite?
      return frozen? ? self : dup
    end
```

The current method always return a new string, even if unchanged, we should keep that behavior at least for mutable strings.

Code after:
  #     puts <<-USAGE.strip_heredoc
  #       This command does such and such.
  #
  #       Supported options are:
  #         -h         This message
  #         ...
  #     USAGE
  #   end
  #
  # the user would see the usage message aligned against the left margin.
  #
  # Technically, it looks for the least indented non-empty line
  # in the whole string, and removes that amount of leading whitespace.
  def strip_heredoc
    scanner = StringScanner.new("")

    min_indent_len = nil
    lines = self.lines

    lines.each do |line|
      scanner.string = line
      indent_len = scanner.skip(/[ \t]*/)

      next unless scanner.match?(/[^\r\n]/)

      min_indent_len = indent_len if indent_len < (min_indent_len || Float::INFINITY)
    end


    if min_indent_len.nil? || min_indent_len.zero?
      return frozen? ? self : dup
    end

    lines.each do |line|
      line[0, min_indent_len] = "" unless line == "\n"
    end

    result = lines.join

    result.freeze if frozen?
    result
  end
end
Repository: rails/rails
File: activemodel/test/cases/validations_test.rb (Ruby)

Code before:

    errors = r.errors.to_a.inject([]) { |result, error| result + [error] }

    assert_equal ["is invalid"], r.errors[:base]

    assert_includes errors, "Title is Empty"
    assert_includes errors, "is invalid"

    assert_equal 2, r.errors.count
  end

  def test_errors_on_custom_attribute
    r = Reply.new

    r.errors.add(:foo_bar, 'is invalid')

    assert_equal r.errors.full_messages, ['Foo bar is invalid']
  end

  def test_errors_on_custom_attribute_with_symbol_message
    r = Reply.new

    r.errors.add(:foo_bar, :invalid)

    assert_equal r.errors.full_messages, ['Foo bar is invalid']
  end

  def test_errors_empty_after_errors_on_check
    t = Topic.new
    assert_empty t.errors[:id]
    assert_empty t.errors
  end

  def test_validates_each
    hits = 0
    Topic.validates_each(:title, :content, [:title, :content]) do |record, attr|
      record.errors.add attr, "gotcha"
      hits += 1
    end
    t = Topic.new("title" => "valid", "content" => "whatever")
    assert_predicate t, :invalid?
    assert_equal 4, hits
    assert_equal %w(gotcha gotcha), t.errors[:title]
    assert_equal %w(gotcha gotcha), t.errors[:content]
  end

  def test_validates_each_custom_reader
    hits = 0
    CustomReader.validates_each(:title, :content, [:title, :content]) do |record, attr|
      record.errors.add attr, "gotcha"
      hits += 1

Review comment (bug):
`test_errors_on_custom_attribute` passes on main, while `test_errors_on_custom_attribute_with_symbol_message` fails with `NoMethodError`.

Code after:

    errors = r.errors.to_a.inject([]) { |result, error| result + [error] }

    assert_equal ["is invalid"], r.errors[:base]

    assert_includes errors, "Title is Empty"
    assert_includes errors, "is invalid"

    assert_equal 2, r.errors.count
  end

  def test_errors_on_custom_attribute
    r = Reply.new

    r.errors.add(:foo_bar, "is invalid")

    assert_equal ["Foo bar is invalid"], r.errors.full_messages
  end

  def test_errors_on_custom_attribute_with_symbol_message
    r = Reply.new

    r.errors.add(:foo_bar, :invalid)

    assert_equal ["Foo bar is invalid"], r.errors.full_messages
  end

  def test_errors_empty_after_errors_on_check
    t = Topic.new
    assert_empty t.errors[:id]
    assert_empty t.errors
  end

  def test_validates_each
    hits = 0
    Topic.validates_each(:title, :content, [:title, :content]) do |record, attr|
      record.errors.add attr, "gotcha"
      hits += 1
    end
    t = Topic.new("title" => "valid", "content" => "whatever")
    assert_predicate t, :invalid?
    assert_equal 4, hits
    assert_equal %w(gotcha gotcha), t.errors[:title]
    assert_equal %w(gotcha gotcha), t.errors[:content]
  end

  def test_validates_each_custom_reader
    hits = 0
    CustomReader.validates_each(:title, :content, [:title, :content]) do |record, attr|
      record.errors.add attr, "gotcha"
      hits += 1
Repository: rails/rails
File: railties/lib/rails/devtools_controller.rb (Ruby)

Code before:
# frozen_string_literal: true

class Rails::DevtoolsController < ActionController::Base # :nodoc:
  def show
    require "digest"
    root_path = Rails.root.to_s
    hash = Digest::SHA1.hexdigest(root_path)
    uuid = "#{hash[0..7]}-#{hash[8..11]}-#{hash[12..15]}-#{hash[16..19]}-#{hash[20..31]}"

    render json: {
      "workspace": {
        "root": root_path,
        "uuid": uuid
      }
    }
  end
end


Review comment (suggestion):
`require` shouldn't be called from controller actions.


Code after:
# frozen_string_literal: true

class Rails::DevtoolsController < ActionController::Base # :nodoc:
  def show
    root_path = Rails.root.to_s
    hash = Digest::SHA1.hexdigest(root_path)
    uuid = "#{hash[0..7]}-#{hash[8..11]}-#{hash[12..15]}-#{hash[16..19]}-#{hash[20..31]}"

    render json: {
      "workspace": {
        "root": root_path,
        "uuid": uuid
      }
    }
  end
end
Repository: rails/rails
File: railties/lib/rails.rb (Ruby)

Code before:
require "rails/application"
require "rails/backtrace_cleaner"

require "active_support/railtie"
require "action_dispatch/railtie"

# UTF-8 is the default internal and external encoding.
silence_warnings do
  Encoding.default_external = Encoding::UTF_8
  Encoding.default_internal = Encoding::UTF_8
end

# :include: ../README.rdoc
module Rails
  extend ActiveSupport::Autoload
  extend ActiveSupport::Benchmarkable

  autoload :Info
  autoload :InfoController
  autoload :MailersController
  autoload :WelcomeController

  eager_autoload do
    autoload :HealthController
    autoload :PwaController
    autoload :DevtoolsController
  end

  class << self
    @application = @app_class = nil

    attr_writer :application
    attr_accessor :app_class, :cache, :logger
    def application
      @application ||= (app_class.instance if app_class)
    end

    delegate :initialize!, :initialized?, to: :application

    # The Configuration instance used to configure the \Rails environment
    def configuration
      application.config
    end

    def backtrace_cleaner
      @backtrace_cleaner ||= Rails::BacktraceCleaner.new
    end

    # Returns a Pathname object of the current \Rails project,
    # otherwise it returns +nil+ if there is no project:
    #

Review comment (suggestion):
This doesn't need to be in `eager_autoload` as it won't be used in production.

Code after:
require "rails/application"
require "rails/backtrace_cleaner"

require "active_support/railtie"
require "action_dispatch/railtie"

# UTF-8 is the default internal and external encoding.
silence_warnings do
  Encoding.default_external = Encoding::UTF_8
  Encoding.default_internal = Encoding::UTF_8
end

# :include: ../README.rdoc
module Rails
  extend ActiveSupport::Autoload
  extend ActiveSupport::Benchmarkable

  autoload :Info
  autoload :InfoController
  autoload :MailersController
  autoload :WelcomeController
  autoload :DevtoolsController

  eager_autoload do
    autoload :HealthController
    autoload :PwaController
  end

  class << self
    @application = @app_class = nil

    attr_writer :application
    attr_accessor :app_class, :cache, :logger
    def application
      @application ||= (app_class.instance if app_class)
    end

    delegate :initialize!, :initialized?, to: :application

    # The Configuration instance used to configure the \Rails environment
    def configuration
      application.config
    end

    def backtrace_cleaner
      @backtrace_cleaner ||= Rails::BacktraceCleaner.new
    end

    # Returns a Pathname object of the current \Rails project,
    # otherwise it returns +nil+ if there is no project:
    #
Repository: rails/rails
File: guides/bug_report_templates/action_view.rb (Ruby)

Code before:
class TestApp < Rails::Application
  config.load_defaults Rails::VERSION::STRING.to_f
  config.eager_load       = false
  config.logger           = Logger.new($stdout)
  config.secret_key_base  = "secret_key_base"
end

Rails.application.initialize!

class BugTest < ActionView::TestCase
  setup do
    @view_path = Pathname.new(Dir.mktmpdir)
    ActionController::Base.prepend_view_path @view_path
  end

  teardown do
    @view_path.rmtree
  end

  helper do
    def titleize(string)
      string.titleize
    end
  end

  test "Action View template rendering" do
    view_file "posts/index.html.erb", <<~ERB
      <h1>Posts</h1>

      <% posts.each do |post| %>
        <%= render partial: "posts/post", locals: { post: post } %>
      <% end %>
    ERB

    view_file "posts/_post.html.erb", <<~ERB
      <p><%= titleize(post) %></p>
    ERB

    render template: "posts/index", locals: { posts: ["hello world"] }

    assert_equal "Hello World", rendered.html.at("p").text
  end


  test "Action View inline rendering" do
    render inline: <<~ERB, locals: { key: "value" }
      <p><%= titleize(key) %></p>
    ERB

    element = rendered.html.at("p")


Review comment (suggestion):
```suggestion
  def test_action_view_render_template
```

Code after:
class TestApp < Rails::Application
  config.load_defaults Rails::VERSION::STRING.to_f
  config.eager_load = false
  config.logger = Logger.new($stdout)
  config.secret_key_base = "secret_key_base"
end
Rails.application.initialize!

class BugTest < ActionView::TestCase
  setup do
    @view_path = Pathname.new(Dir.mktmpdir)
    ActionController::Base.prepend_view_path @view_path
  end

  teardown do
    @view_path.rmtree
  end

  helper do
    def upcase(value)
      value.upcase
    end
  end

  def test_action_view_render_template
    view_file "posts/index.html.erb", <<~ERB
      <h1>Posts</h1>

      <% posts.each do |post| %>
        <%= render partial: "posts/post", locals: { post: post } %>
      <% end %>
    ERB

    view_file "posts/_post.html.erb", <<~ERB
      <p><%= upcase(post) %></p>
    ERB

    render template: "posts/index", locals: { posts: ["hello world"] }

    assert_equal "HELLO WORLD", rendered.html.at("p").text
  end

  def test_action_view_render_inline
    render inline: <<~ERB, locals: { key: "value" }
      <p><%= upcase(key) %></p>
    ERB

    element = rendered.html.at("p")

    assert_equal element.text, "VALUE"
  end
Repository: rails/rails
File: guides/bug_report_templates/action_view.rb (Ruby)

Code before:
  helper do
    def titleize(string)
      string.titleize
    end
  end

  test "Action View template rendering" do
    view_file "posts/index.html.erb", <<~ERB
      <h1>Posts</h1>

      <% posts.each do |post| %>
        <%= render partial: "posts/post", locals: { post: post } %>
      <% end %>
    ERB

    view_file "posts/_post.html.erb", <<~ERB
      <p><%= titleize(post) %></p>
    ERB

    render template: "posts/index", locals: { posts: ["hello world"] }

    assert_equal "Hello World", rendered.html.at("p").text
  end


  test "Action View inline rendering" do
    render inline: <<~ERB, locals: { key: "value" }
      <p><%= titleize(key) %></p>
    ERB

    element = rendered.html.at("p")

    assert_equal element.text, "Value"
  end

  private
    def view_file(filename, contents)
      pathname = @view_path.join(filename)
      pathname.dirname.mkpath
      pathname.write(contents.chomp)
    end
end


Review comment (suggestion):
```suggestion
  def test_action_view_render_inline
```

Code after:
    def upcase(value)
      value.upcase
    end
  end

  def test_action_view_render_template
    view_file "posts/index.html.erb", <<~ERB
      <h1>Posts</h1>

      <% posts.each do |post| %>
        <%= render partial: "posts/post", locals: { post: post } %>
      <% end %>
    ERB

    view_file "posts/_post.html.erb", <<~ERB
      <p><%= upcase(post) %></p>
    ERB

    render template: "posts/index", locals: { posts: ["hello world"] }

    assert_equal "HELLO WORLD", rendered.html.at("p").text
  end

  def test_action_view_render_inline
    render inline: <<~ERB, locals: { key: "value" }
      <p><%= upcase(key) %></p>
    ERB

    element = rendered.html.at("p")

    assert_equal element.text, "VALUE"
  end

  private
    def view_file(filename, contents)
      pathname = @view_path.join(filename)
      pathname.dirname.mkpath
      pathname.write(contents.chomp)
    end
end