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

Method "#all" collision between Capybara and Rspec #1396

Closed
mhodgson opened this issue Sep 11, 2014 · 12 comments
Closed

Method "#all" collision between Capybara and Rspec #1396

mhodgson opened this issue Sep 11, 2014 · 12 comments

Comments

@mhodgson
Copy link

Unfortunately the all method in the Capybara DSL is now conflicting with the all matcher in Rspec (https://www.relishapp.com/rspec/rspec-expectations/v/3-1/docs/built-in-matchers/all-matcher). I'm not sure what happens now (fight to the death?), but it did lead to a very confusing error message:

The given selector #<RSpec::Matchers::BuiltIn::BePredicate:0x007fd94e726760> is either invalid or does not result in a WebElement. The following error occurred:
       InvalidSelectorError: An invalid or illegal selector was specified

Thoughts?

@mhodgson
Copy link
Author

One thought, maybe rename the all method in Capybara to every. Seems to maintain the same meaning and would read ok:

expect(every('a.btn')).to all(be_visible)

@abotalov
Copy link
Collaborator

@myronmarston That's the second time when RSpec adds a method with a name that already exists in Capybara - see #1361

@abotalov
Copy link
Collaborator

IMO every is a bad name. It makes sense in your example but it doesn't make sense when you want to e.g. get second element - every('.cls')[1] or every('.cls').last

@abotalov
Copy link
Collaborator

As told in previous issue about within the easiest workaround is to invoke that method from page (aka Capybara.current_session):

page.all

@myronmarston
Copy link
Contributor

@myronmarston That's the second time when RSpec adds a method with a name that already exists in Capybara - see #1361

Sorry about that :(. I'm not sure what we could have or should have done differently, given that RSpec continues to gain new features (often in the form of new APIs) as requested by the community. Any suggestions?

@mhodgson
Copy link
Author

@abotalov I actually am having the reverse problem where I can't access Rspec's all method, not the other way around (for which your workaround would be fine). I think in the case of Rspec I need to do this:

expect(things).to Rspec.all(be_visible)

But that kind of kills readability and isn't worth it. I just looped over the elements manually instead.

@mhodgson
Copy link
Author

@abotalov maybe find_all ?

@abotalov
Copy link
Collaborator

@mhodgson RSpec's all doesn't work for you as Capybara::DSL is included after RSpec::Matchers. Adding alias inside Capybara won't resolve the issue as all will continue to exist and override RSpec's method.

You can include Capybara::DSL before including RSpec::Matchers. Then all will mean RSpec's method. And you would be able to use Capybara's method via page.all.

@abotalov
Copy link
Collaborator

@myronmarston

I'm not sure what we could have or should have done differently

It looks to me that an issue of clashing method names is an issue not only of RSpec and Capybara but of Ruby ecosystem in general. It's bad when two gems expose methods with the same name. So it would be better if there would be a general enough solution for that problem.

It's easy to find those clashing methods if one would put some care into combining a list of modules/classes with with which methods exposed by a gem may interfere. Something like:

gems_and_their_methods = [
  RSpec::Matchers,
  Capybara::DSL,
].map { |module_name| [module_name.to_s, Set.new(module_name.instance_methods(false))] }
gems_and_their_methods.combination(2) do |arr1, arr2|
  collisions = (arr1[1] & arr2[1]).to_a
  puts "Method collisions between #{arr1[0]} and #{arr2[0]} : #{collisions}" # => Method collisions between RSpec::Matchers and Capybara::DSL : [:all, :within]
end

It may be even possible to create a general gem that will contain gems_and_their_methods array not only just for testing context but also for model, controller etc. though it may be a very large task, not sure.

It's possible to incorporate such task into a build that will check for those clashes and say if there are e.g. any new.

@myronmarston
Copy link
Contributor

That's an interesting idea, @abotalov. I would say that it's really only a problem when gems choose to add methods to a namespace owned by another gem, as is the case here. RSpec "owns" the namespace for the API it exposes, and capybara chooses to import its API into that same namespace. Also, I don't want to limit new RSpec APIs to just method names that aren't already in use by an RSpec extension. It would be useful to know in advance that adding a particular API will cause a conflict with a method in a particular extension, but that wouldn't necessarily stop us from adding it. It would, however, allow us to discuss the issue before the new API is released, to have solutions ready.

If you decide to develop your conflict-detection logic further and want some input from the RSpec side, let me know.

@schnittchen
Copy link

I have a feature here where I'm not interested in accessing the Capybara method, but RSpec's builtin matcher instead. Worked around with include RSpec::Matchers.clone

rspeicher added a commit to jvanbaarsen/gitlabhq that referenced this issue Jun 1, 2015
There's a naming conflict between Capybara and rspec-matchers which both
define the `all` method.

See teamcapybara/capybara#1396
rspeicher added a commit to gitlabhq/gitlabhq that referenced this issue Jun 12, 2015
There's a naming conflict between Capybara and rspec-matchers which both
define the `all` method.

See teamcapybara/capybara#1396
@mattwynne
Copy link

Would it be possible to implement a proxy that's intelligent enough to forward methods called on the result of #all to either Capybara or RSpec depending on the context?

twalpole added a commit that referenced this issue Jul 16, 2015
add #find_all as an alias of #all - Issue #1396
@twalpole twalpole closed this as completed Nov 3, 2015
@lock lock bot locked and limited conversation to collaborators Aug 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants