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

Add Ruby 3 and Mongo 5 to CI #14

Merged
merged 11 commits into from
Mar 22, 2022

Conversation

crazyoptimist
Copy link
Collaborator

@crazyoptimist crazyoptimist commented Mar 22, 2022

  • Add Ruby 3.1.1, Mongo 5.0 to Mongoid test
  • Add Ruby 3.1.1 to ActiveRecord test
  • Modify database names to more meaningful ones
  • Fix Danger failure

@crazyoptimist crazyoptimist force-pushed the chore/ruby3-mongo5 branch 2 times, most recently from 1baa505 to f30985f Compare March 22, 2022 07:48
@crazyoptimist
Copy link
Collaborator Author

@dblock

  • Failed to figure out Ruby 3 with postgresql, guess it has something to do with passing yaml file differently according to ruby3 upgrade, but couldn't figure it out so far.
  • Failed to fix Danger failure, please check the workflow file if I'm missing something.

@dblock
Copy link
Contributor

dblock commented Mar 22, 2022

@dblock

  • Failed to figure out Ruby 3 with postgresql, guess it has something to do with passing yaml file differently according to ruby3 upgrade, but couldn't figure it out so far.

The interface for YAML.safe_load is different in whatever ships with Ruby 3.1.1, copy-paste https://github.com/slack-ruby/slack-ruby-bot-server/blob/7623dcd9176ac8a85fb3154378baa298f45c40ac/spec/database_adapters/activerecord/activerecord.rb.

  • Failed to fix Danger failure, please check the workflow file if I'm missing something.

Not sure what its problem is, I gave it its own Gemfile like in slack-ruby-bot-server and it worked, #15.

@crazyoptimist
Copy link
Collaborator Author

The interface for YAML.safe_load is different in whatever ships with Ruby 3.1.1, copy-paste https://github.com/slack-ruby/slack-ruby-bot-server/blob/7623dcd9176ac8a85fb3154378baa298f45c40ac/spec/database_adapters/activerecord/activerecord.rb.

I tried it already and failed. But let me try again.

@dangerpr-bot
Copy link

dangerpr-bot commented Mar 22, 2022

1 Message
📖 We really appreciate pull requests that demonstrate issues, even without a fix. That said, the next step is to try and fix the failing tests!

Generated by 🚫 Danger

@dblock
Copy link
Contributor

dblock commented Mar 22, 2022

The interface for YAML.safe_load is different in whatever ships with Ruby 3.1.1, copy-paste https://github.com/slack-ruby/slack-ruby-bot-server/blob/7623dcd9176ac8a85fb3154378baa298f45c40ac/spec/database_adapters/activerecord/activerecord.rb.

I tried it already and failed. But let me try again.

Show me a failure if you're still having trouble.

Add a CHANGELOG saying "Added support for Ruby 3.1." when both mongodb/postgresql work.

@crazyoptimist
Copy link
Collaborator Author

An error occurred while loading ./spec/slack-ruby-bot-server-events/version_spec.rb.
Failure/Error: ActiveRecord::Tasks::DatabaseTasks.create(db_config)

ArgumentError:
  wrong number of arguments (given 3, expected 2)

@crazyoptimist
Copy link
Collaborator Author

db_config is parsed correctly as I tried to print out:

{"adapter"=>"postgresql", "pool"=>10, "timeout"=>5000, "encoding"=>"unicode", "database"=>"slack_ruby_bot_server_events_test", "url"=>"postgresql://test:password@localhost/slack_ruby_bot_server_events_test"}

@crazyoptimist
Copy link
Collaborator Author

crazyoptimist commented Mar 22, 2022

Something to do with this one maybe?
https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/

But curious I just copied it from slack-ruby-bot-server which works without issues

@dblock
Copy link
Contributor

dblock commented Mar 22, 2022

Something to do with this one maybe? https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/

But curious I just copied it from slack-ruby-bot-server which works without issues

If you look at the call stack of that error you'll notice that it's bringing in ActiveRecord 5 and errors like so.

An error occurred while loading ./spec/slack-ruby-bot-server-events/api/endpoints/slack/actions_endpoint_spec.rb.
Failure/Error: ActiveRecord::Tasks::DatabaseTasks.create(db_config)

ArgumentError:
  wrong number of arguments (given 3, expected 2)
# /Users/dblock/.rvm/gems/ruby-3.1.1/gems/activerecord-5.0.7.2/lib/active_record/type/adapter_specific_registry.rb:7:in `add_modifier'
# /Users/dblock/.rvm/gems/ruby-3.1.1/gems/activerecord-5.0.7.2/lib/active_record/type.rb:22:in `add_modifier'

ActiveRecord 5 is just not compatible with this version of Ruby anymore, so we should just let bundler pick compatible versions. Then, once you move it up to 6, there are more problems that can be fixed the same way as slack-ruby-bot-server. Change this in Gemfile:

when 'activerecord' then
  gem 'activerecord'
  gem 'otr-activerecord'
  gem 'virtus'
  gem 'cursor_pagination', github: 'dblock/cursor_pagination', branch: 'misc' # rubocop:disable Bundler/OrderedGems
  gem 'pg'

@crazyoptimist
Copy link
Collaborator Author

crazyoptimist commented Mar 22, 2022

I didn't know bundler was smart to pick compatible versions. Thanks!!

@crazyoptimist
Copy link
Collaborator Author

Danger yells again this time, everything else is passing!
@dblock

@crazyoptimist
Copy link
Collaborator Author

CI log doesn't give any clue for why danger failed :(

@dblock
Copy link
Contributor

dblock commented Mar 22, 2022

It does, see above: #14 (comment)

I am guessing those dates are wrong (should be year/...)? Not sure why it didn't complain before, probably because some dependent gem got updated and it's now seeing these problems?

Change #### 0.3.1 (2/4/2021) to #### 0.3.1 (2021/04/02)

@crazyoptimist crazyoptimist requested a review from dblock March 22, 2022 21:13
@dblock dblock merged commit 8314b4d into slack-ruby:master Mar 22, 2022
@dblock
Copy link
Contributor

dblock commented Mar 26, 2022

@crazyoptimist If you have time, take a look at slack-ruby/slack-ruby-bot-server#148

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

Successfully merging this pull request may close these issues.

3 participants