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 matrix as a dependency for Ruby 3.1 compatibility #2468

Merged
merged 1 commit into from
May 29, 2021

Conversation

casperisfine
Copy link

@coveralls
Copy link

Pull Request Test Coverage Report for Build 7830

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage remained the same at 97.19%

Files with Coverage Reduction New Missed Lines %
lib/capybara/queries/text_query.rb 1 98.44%
lib/capybara/selenium/patches/logs.rb 1 72.22%
Totals Coverage Status
Change from base Build 7818: 0%
Covered Lines: 14282
Relevant Lines: 14695

💛 - Coveralls

casperisfine pushed a commit to Shopify/rails that referenced this pull request May 28, 2021
@casperisfine
Copy link
Author

Actually this cause double loading warnings for ruby 3.0 and older. I'm not too sure yet how best to fix this.

@twalpole
Copy link
Member

I haven’t looked in detail yet, but I can’t think of anywhere that Capybara uses matrix itself. That would imply it’s actually a requirement of a dependency and should be fixed there rather than in Capybara

@casperisfine
Copy link
Author

I can’t think of anywhere that Capybara uses matrix itself.

Right here:

@casperisfine
Copy link
Author

That being said I don't see it used, it was introduced in b0e3b93 but I see no reference to the Matrix class. So maybe it was a mistake and we can just remove the require?

@twalpole
Copy link
Member

Ah, yes -- we use Vector which is provided by the matrix library

@twalpole
Copy link
Member

If this is causing double loading warnings on released versions of Ruby then I'm not going to merge it in to fix a dev version of Ruby. For now you'll just need to specify the dependency in your own gemfile

@casperisfine
Copy link
Author

If this is causing double loading warnings on released versions of Ruby then I'm not going to merge it

In the end it doesn't, the problem is only with specific gems that bundler use like digest, net/protocol. See: https://bugs.ruby-lang.org/issues/17873

I got this issue with the mail gem, so I posted here quickly too to make sure this wouldn't be merged before I investigate, but I'm confident this can be merged safely now.

For now you'll just need to specify the dependency in your own gemfile

Yeah it's already worked around on our side, but I'm running ruby-head CI specifically to catch this kind of things early so that upgrading to Ruby 3.1 will as smooth as possible in January. So IMHO if this is included early it's better.

@twalpole twalpole merged commit 08e7aec into teamcapybara:master May 29, 2021
gabebw added a commit to hotline-webring/hotline-webring that referenced this pull request Feb 20, 2022
Heroku supports [Ruby 3.1.1][0] but CircleCI only supports [up to version 3.1.0][1].

[0]: https://devcenter.heroku.com/articles/ruby-support#ruby-versions
[1]: https://circleci.com/developer/images/image/cimg/ruby

Update gems:

* Rails >7.1 due to rails/rails#43998
* Capybara >=3.36.0 because the `matrix` gem is no longer built in to
  Ruby: teamcapybara/capybara#2468
* Timecop >= 0.9.4 for 3.1 support ("<internal:timev>:310:in
  `initialize': no implicit conversion of Hash into Integer
  (TypeError)")
Earlopain added a commit to Earlopain/good_job that referenced this pull request Oct 10, 2024
* matrix: teamcapybara/capybara#2468
* `nokogiri 🤷 but there's no version constraint anyways
* `net-*`: mikel/mail#1472

It's been quite a while so these seem safe to drop
Earlopain added a commit to Earlopain/good_job that referenced this pull request Oct 10, 2024
* matrix: teamcapybara/capybara#2468
* `nokogiri 🤷 but there's no version constraint anyways
* `net-*`: mikel/mail#1472

It's been quite a while so these seem safe to drop
bensheldon pushed a commit to bensheldon/good_job that referenced this pull request Oct 10, 2024
* Remove some explicit dependencies from the gemfile

* matrix: teamcapybara/capybara#2468
* `nokogiri 🤷 but there's no version constraint anyways
* `net-*`: mikel/mail#1472

It's been quite a while so these seem safe to drop

* Run different rails versions tests in parallel

Ditch appraisals while we're at it
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.

4 participants