From fdbc55b9d55ae9d2b5b39be3ef4af5f60f6954a3 Mon Sep 17 00:00:00 2001 From: Yasuo Honda Date: Mon, 22 Jun 2020 21:19:21 +0900 Subject: [PATCH] Use Array(ActiveSupport::ToJsonWithActiveSupportEncoder)#to_json and Hash(ActiveSupport::ToJsonWithActiveSupportEncoder)#to_json for Ruby 2.8.0 This pull request addresses failures at https://buildkite.com/rails/rails/builds/70219#79d96882-6c51-4854-8cab-28f50ac8bca1 According to https://bugs.ruby-lang.org/issues/16973 This is an expected change in Ruby. These failures has been addressed by changing the order of prepend as suggested. ```diff % git diff diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb index 30a3b8e5a0..1328041bf7 100644 --- a/activesupport/test/json/encoding_test.rb +++ b/activesupport/test/json/encoding_test.rb @@ -186,6 +186,8 @@ def test_hash_should_pass_encoding_options_to_children_in_to_json country: "UK" } } + p person.method(:to_json) + pp person.class.ancestors json = person.to_json only: [:address, :city] assert_equal(%({"address":{"city":"London"}}), json) @@ -287,6 +289,8 @@ def test_array_to_json_should_not_keep_options_around f.bar = "world" array = [f, { "foo" => "other_foo", "test" => "other_test" }] + p array.method(:to_json) + pp array.class.ancestors assert_equal([{ "foo" => "hello", "bar" => "world" }, { "foo" => "other_foo", "test" => "other_test" }], ActiveSupport::JSON.decode(array.to_json)) end % ``` * Ruby 2.8.0 without this fix uses `Array(JSON::Ext::Generator::GeneratorMethods::Array)#to_json`, which should use `Array(ActiveSupport::ToJsonWithActiveSupportEncoder)#to_json` ``` % bin/test test/json/encoding_test.rb -n test_array_to_json_should_not_keep_options_around Run options: -n test_array_to_json_should_not_keep_options_around --seed 33311 [Array, JSON::Ext::Generator::GeneratorMethods::Array, ActiveSupport::ToJsonWithActiveSupportEncoder, Enumerable, ActiveSupport::ToJsonWithActiveSupportEncoder, Object, JSON::Ext::Generator::GeneratorMethods::Object, ActiveSupport::Tryable, Kernel, BasicObject] F Failure: TestJSONEncoding#test_array_to_json_should_not_keep_options_around [/Users/yahonda/src/github.com/rails/rails/activesupport/test/json/encoding_test.rb:294]: --- expected +++ actual @@ -1 +1 @@ -[{"foo"=>"hello", "bar"=>"world"}, {"foo"=>"other_foo", "test"=>"other_test"}] +["#", {"foo"=>"other_foo", "test"=>"other_test"}] bin/test test/json/encoding_test.rb:286 Finished in 0.015486s, 64.5745 runs/s, 64.5745 assertions/s. 1 runs, 1 assertions, 1 failures, 0 errors, 0 skips % ``` * Ruby 2.8.0 with this fix uses `Array(ActiveSupport::ToJsonWithActiveSupportEncoder)#to_json` ``` % bin/test test/json/encoding_test.rb -n test_array_to_json_should_not_keep_options_around Run options: -n test_array_to_json_should_not_keep_options_around --seed 12193 [ActiveSupport::ToJsonWithActiveSupportEncoder, Array, JSON::Ext::Generator::GeneratorMethods::Array, ActiveSupport::ToJsonWithActiveSupportEncoder, Enumerable, ActiveSupport::ToJsonWithActiveSupportEncoder, Object, JSON::Ext::Generator::GeneratorMethods::Object, ActiveSupport::Tryable, Kernel, BasicObject] . Finished in 0.008070s, 123.9157 runs/s, 123.9157 assertions/s. 1 runs, 1 assertions, 0 failures, 0 errors, 0 skips % ``` * Ruby 2.8.0 without this fix uses `Hash(JSON::Ext::Generator::GeneratorMethods::Hash)#to_json`, which should use `Hash(ActiveSupport::ToJsonWithActiveSupportEncoder)#to_json` ``` % bin/test test/json/encoding_test.rb -n test_hash_should_pass_encoding_options_to_children_in_to_json Run options: -n test_hash_should_pass_encoding_options_to_children_in_to_json --seed 18064 [Hash, JSON::Ext::Generator::GeneratorMethods::Hash, ActiveSupport::ToJsonWithActiveSupportEncoder, Enumerable, ActiveSupport::ToJsonWithActiveSupportEncoder, Object, JSON::Ext::Generator::GeneratorMethods::Object, ActiveSupport::Tryable, Kernel, BasicObject] F Failure: TestJSONEncoding#test_hash_should_pass_encoding_options_to_children_in_to_json [/Users/yahonda/src/github.com/rails/rails/activesupport/test/json/encoding_test.rb:193]: --- expected +++ actual @@ -1 +1 @@ -"{\"address\":{\"city\":\"London\"}}" +"{\"name\":\"John\",\"address\":{\"city\":\"London\",\"country\":\"UK\"}}" bin/test test/json/encoding_test.rb:181 Finished in 0.015009s, 66.6267 runs/s, 66.6267 assertions/s. 1 runs, 1 assertions, 1 failures, 0 errors, 0 skips % ``` * Ruby 2.8.0 with this fix uses `Hash(ActiveSupport::ToJsonWithActiveSupportEncoder)#to_json` ``` % bin/test test/json/encoding_test.rb -n test_hash_should_pass_encoding_options_to_children_in_to_json Run options: -n test_hash_should_pass_encoding_options_to_children_in_to_json --seed 56794 [ActiveSupport::ToJsonWithActiveSupportEncoder, Hash, JSON::Ext::Generator::GeneratorMethods::Hash, ActiveSupport::ToJsonWithActiveSupportEncoder, Enumerable, ActiveSupport::ToJsonWithActiveSupportEncoder, Object, JSON::Ext::Generator::GeneratorMethods::Object, ActiveSupport::Tryable, Kernel, BasicObject] . Finished in 0.007434s, 134.5171 runs/s, 134.5171 assertions/s. 1 runs, 1 assertions, 0 failures, 0 errors, 0 skips % ``` Refer https://github.com/ruby/ruby/pull/3181 https://github.com/ruby/ruby/pull/2936 https://bugs.ruby-lang.org/issues/9573 https://github.com/rails/rails/pull/19413 --- activesupport/lib/active_support/core_ext/object/json.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/core_ext/object/json.rb b/activesupport/lib/active_support/core_ext/object/json.rb index d92af19137..dcfbe8b40f 100644 --- a/activesupport/lib/active_support/core_ext/object/json.rb +++ b/activesupport/lib/active_support/core_ext/object/json.rb @@ -45,7 +45,7 @@ module ActiveSupport end end -[Object, Array, FalseClass, Float, Hash, Integer, NilClass, String, TrueClass, Enumerable].reverse_each do |klass| +[Enumerable, Object, Array, FalseClass, Float, Hash, Integer, NilClass, String, TrueClass].reverse_each do |klass| klass.prepend(ActiveSupport::ToJsonWithActiveSupportEncoder) end