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

specify minimum capybara version for system tests #30959

Merged
merged 1 commit into from
Oct 23, 2017

Conversation

lostapathy
Copy link
Contributor

Resolves #30952

Upgraded rails applications may have a Gemfile without a new enough capybara to run system tests. Setting a version here gives the user a more direct error message than they get otherwise.

Upgraded rails applications may have a Gemfile without a new enough
capybara to run system tests. Setting a version here gives the user a
more direct error message than they get otherwise.  Resolves rails#30952
@rafaelfranca rafaelfranca merged commit c085a2a into rails:master Oct 23, 2017
rafaelfranca added a commit that referenced this pull request Oct 23, 2017
specify minimum capybara version for system tests
@@ -1,5 +1,7 @@
# frozen_string_literal: true

gem "capybara", "~> 2.13"
Copy link
Contributor

@twalpole twalpole Oct 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has a side effect of users not being able to update to Capybara 3 when it releases, since the requirement is in non user modifiable source. Additionally master now depends on ~> 2.15. Wouldn't it be better as gem "capybara", ">= 2.15" in this file? (~> 2.15 makes sense in the apps generated Gemfile though)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I submitted it this way as that's what was requested on #30952. I'm assuming the preferred convention here is to match rubygem's preference that open-ended dependencies are to be avoided.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fine to not allow update to capybara 3 since rails may not work with that version, but yeah, it should be 2.15. I'll update it.

Copy link
Contributor

@twalpole twalpole Oct 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rafaelfranca It will work with 3 since 3 is just going to be a remove deprecations and drop ruby versions release. However, this is adding a requirement just for a warning in a non user editable file - The more restrictive requirement in the Gemfile generated for an app is correct, and where the actual restriction should be (so users can modify that when Capybara 3 is released, if desired). I really feel this should be >= 2.15 in this file.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@twalpole so I should be good to use gem 'capybara', '~> 3.0.2', '< 4.0' in my Rails 5.1 app? I'm experiencing #30952

Copy link
Contributor

@twalpole twalpole Apr 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MrHubble If you're using Rails 5.1 you should be able to use gem 'capybara', '~> 2.18' in your gem file since Rails 5.1 won't allow Capybara 3+ due to this change (which IMHO was bad and has been corrected in 5.2) if you're using system tests. With Rails 5.2 you should be good to use gem 'capybara', '~> 3.0' (note that specifying the <4.0 is unnecessary due to the the way ~> works

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. Thanks so much for taking the time to clarify.

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.

None yet

4 participants