Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Ruby 3.0 compatibility #622

Closed
v-kolesnikov opened this issue Jan 5, 2021 · 3 comments · Fixed by #623
Closed

Fix Ruby 3.0 compatibility #622

v-kolesnikov opened this issue Jan 5, 2021 · 3 comments · Fixed by #623
Assignees
Labels
Milestone

Comments

@v-kolesnikov
Copy link
Collaborator

Describe the bug

Tests don't pass on Ruby 3.0

To Reproduce

$ rvm use 3.0
$ rake spec

Expected behavior

Tests passed


Objective

ROM configuration is broken on Ruby 3.0 because of Separation of positional and keyword arguments in Ruby 3.0. It is mostly on rom-core side:
https://github.com/rom-rb/rom/blob/v5.2.5/core/lib/rom/environment.rb#L16-L53

In general all def method(*args) should be replaced to def method(*args, **kwargs), but this is not so simple in some cases (such above).

UPD:

I prepared a small proof to demonstrate the bug:

# frozen_string_literal: true

require 'bundler/inline'

gemfile(_install = true) do
  source 'https://rubygems.org'
  gem 'rom-core', '~> 5.2', '>= 5.2.4'
  gem 'rspec'
end

module MyAdapter
  class Gateway
    extend ROM::Initializer

    param :param
    option :option
  end
end

ROM.register_adapter(:my_adapter, MyAdapter)

require 'rspec/autorun'

RSpec.describe MyAdapter do
  specify do
    gw = MyAdapter::Gateway.new(:param, option: :option)
    expect(gw.param).to eq :param
    expect(gw.option).to eq :option
  end

  specify do
    conf = ROM::Configuration.new(:my_adapter, :param, option: :option)
    container = ROM.container(conf)
    gw = container.gateways[:default]
    expect(gw.param).to eq :param
    expect(gw.option).to eq :option
  end
end

On Ruby 2.7:

~/.rvm/gems/ruby-2.7.1/gems/rom-core-5.2.4/lib/rom/environment.rb:53: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
~/.rvm/gems/ruby-2.7.1/gems/rom-core-5.2.4/lib/rom/gateway.rb:83: warning: The called method `setup' is defined here
.

Finished in 0.00322 seconds (files took 0.07245 seconds to load)
2 examples, 0 failures

On Ruby 3.0:

.F

Failures:

  1) MyAdapter
     Failure/Error: conf = ROM::Configuration.new(:my_adapter, :param, option: :option)

     KeyError:
       MyAdapter::Gateway: option 'option' is required
     # (eval):6:in `block in __dry_initializer_initialize__'
     # (eval):6:in `fetch'
     # (eval):6:in `__dry_initializer_initialize__'
     # /Users/b.white/.rvm/gems/ruby-3.0.0-rc1/gems/dry-initializer-3.0.4/lib/dry/initializer/mixin/root.rb:7:in `initialize'
     # /Users/b.white/.rvm/gems/ruby-3.0.0-rc1/gems/rom-core-5.2.4/lib/rom/gateway.rb:96:in `new'
     # /Users/b.white/.rvm/gems/ruby-3.0.0-rc1/gems/rom-core-5.2.4/lib/rom/gateway.rb:96:in `setup'
     # /Users/b.white/.rvm/gems/ruby-3.0.0-rc1/gems/rom-core-5.2.4/lib/rom/environment.rb:53:in `block in normalize_gateways'
     # /Users/b.white/.rvm/gems/ruby-3.0.0-rc1/gems/rom-core-5.2.4/lib/rom/environment.rb:47:in `each'
     # /Users/b.white/.rvm/gems/ruby-3.0.0-rc1/gems/rom-core-5.2.4/lib/rom/environment.rb:47:in `each_with_object'
     # /Users/b.white/.rvm/gems/ruby-3.0.0-rc1/gems/rom-core-5.2.4/lib/rom/environment.rb:47:in `normalize_gateways'
     # /Users/b.white/.rvm/gems/ruby-3.0.0-rc1/gems/rom-core-5.2.4/lib/rom/environment.rb:28:in `configure_gateways'
     # /Users/b.white/.rvm/gems/ruby-3.0.0-rc1/gems/rom-core-5.2.4/lib/rom/environment.rb:20:in `initialize'
     # /Users/b.white/.rvm/gems/ruby-3.0.0-rc1/gems/rom-core-5.2.4/lib/rom/configuration.rb:53:in `new'
     # /Users/b.white/.rvm/gems/ruby-3.0.0-rc1/gems/rom-core-5.2.4/lib/rom/configuration.rb:53:in `initialize'
     # rom_ruby3.rb:32:in `new'
     # rom_ruby3.rb:32:in `block (2 levels) in <main>'

Finished in 0.00311 seconds (files took 0.08745 seconds to load)
2 examples, 1 failure

Failed examples:

rspec rom_ruby3.rb:31 # MyAdapter
@v-kolesnikov v-kolesnikov self-assigned this Jan 5, 2021
@v-kolesnikov
Copy link
Collaborator Author

@solnic Now I think this issue should be addressed to rom-core.

@v-kolesnikov v-kolesnikov transferred this issue from rom-rb/rom-elasticsearch Jan 12, 2021
@solnic solnic added this to the 5.2.6 milestone Jan 13, 2021
@solnic
Copy link
Member

solnic commented Jan 13, 2021

@v-kolesnikov makes sense, I'm gonna cherry-pick the fix into release-5.2 btw (once you're done with your PR 😃)

v-kolesnikov added a commit that referenced this issue Jan 13, 2021
solnic added a commit that referenced this issue Jan 16, 2021
Fix gateway configuration options

[changelog]

fixed: "Setup works under MRI 3.0.0 (issue #622 fixed via #623) (@v-kolesnikov)"
solnic pushed a commit that referenced this issue Jan 16, 2021
@solnic
Copy link
Member

solnic commented Jan 16, 2021

@v-kolesnikov I cherry-picked it into release-5.2 and pushed rom 5.2.6 with rom-core bumped to 5.2.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants