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.

Sign up for GitHub

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

Jump to bottom

specify minimum capybara version for system tests #30959

Merged
merged 1 commit into from
Oct 23, 2017
Merged

specify minimum capybara version for system tests #30959

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions actionpack/lib/action_dispatch/system_test_case.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

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

@twalpole twalpole Oct 23, 2017 •
edited

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 •
edited

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.

Copy link

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 •
edited

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

Copy link

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.


require "capybara/dsl"
require "capybara/minitest"
require "action_controller"
Expand Down