Comparison of ActiveSupport::Duration objects raises unexpected error

[alexeiemam]

Summary

ActiveSupport::Duration objects created via ActiveSupport::Duration.build exhibit unexpected behaviour when instantiated with a string value.

Steps to reproduce

Rails console exemplar

## Comparing string built vs integer build durations
# setup
2.6.3 :004 > a_duration_from_string = ActiveSupport::Duration.build("23")
 => 23.0 seconds 
2.6.3 :005 > a_duration_from_integer = ActiveSupport::Duration.build(23)
 => 23.0 seconds 
# verifying correctness of parts
2.6.3 :006 > a_duration_from_string.parts
 => {:seconds=>23.0} 
2.6.3 :007 > a_duration_from_integer.parts
 => {:seconds=>23.0} 
# comparing
2.6.3 :008 > a_duration_from_string < a_duration_from_integer
Traceback (most recent call last):
        1: from (irb):8
ArgumentError (comparison of String with ActiveSupport::Duration failed)
2.6.3 :009 > a_duration_from_string == a_duration_from_integer
 => false 

## Comparing string built durations
# setup
2.6.3 :017 > small_duration_from_string = ActiveSupport::Duration.build('232')
 => 3 minutes and 52.0 seconds 
2.6.3 :019 > large_duration_from_string = ActiveSupport::Duration.build('1000000')
 => 1 week, 4 days, 13 hours, 46 minutes, and 40.0 seconds 
# verifying correctness of parts
2.6.3 :020 > small_duration_from_string.parts
 => {:minutes=>3, :seconds=>52.0} 
2.6.3 :021 > large_duration_from_string.parts
 => {:weeks=>1, :days=>4, :hours=>13, :minutes=>46, :seconds=>40.0} 
# comparing
2.6.3 :022 > large_duration_from_string > small_duration_from_string
 => false 

Minimal test cases

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "activesupport", "5.2.2.1"
end

require "active_support/duration"
require "minitest/autorun"

class ActiveSupportDurationComparabilityBugTest < Minitest::Test
  def test_less_than_comparator
    a_duration_from_string , a_duration_from_integer = durations_for_test
    string_duration_smaller_than_integer_duration = 
      begin
        a_duration_from_string < a_duration_from_integer
      rescue ArgumentError => e
        "error raised"
      end

    refute string_duration_smaller_than_integer_duration == "error raised"
    assert string_duration_smaller_than_integer_duration == false
  end

  def test_equality
    a_duration_from_string , a_duration_from_integer = durations_for_test
    string_duration_equals_integer_duration = a_duration_from_string == a_duration_from_integer

    assert string_duration_equals_integer_duration == true
  end

  def test_string_built_duration_comparisons
    small_duration_from_string = ActiveSupport::Duration.build('232')
    large_duration_from_string = ActiveSupport::Duration.build('1000000')

    assert large_duration_from_string > small_duration_from_string
  end

  private

  def durations_for_test
    the_duration_seconds = 23
    [
      build(the_duration_seconds.to_s), 
      build(the_duration_seconds)
    ]
  end

  def build(it)
    ActiveSupport::Duration.build(it)
  end
end

Expected behavior

ActiveSupport::Duration objects with identical .parts should be comparable and equivalent, regardless of how they were built:

• a_duration_from_string < a_duration_from_integer should return false
• a_duration_from_string == a_duration_from_integer should return true
• large_duration_from_string > small_duration_from_string should return true

Actual behavior

• a_duration_from_string < a_duration_from_integer raises an ArgumentError
• a_duration_from_string == a_duration_from_integer returns false
• large_duration_from_string > small_duration_from_string returns false

System configuration

Rails version: 5.2.2.1

Ruby version: 2.6.3

[alexeiemam]

Am willing to submit a corrective patch but am wondering if this behaviour is by design before I do.

If string input is unsupported by .build, would expect an argument error raised on .build
If string input is intended to be supported, would expect the test case to pass, i.e. build would instantiate a Duration containing its pre-processed value (value.to_f) rather than the raw value.

[alexeiemam]

Added an additional fail-case:
Comparing two string built durations compares based on string value (where "9" is considered larger than "8888888888888888888")