Consider an optional dependency on selenium-webdriver #887

Closed
christiannelson opened this Issue Nov 25, 2012 · 22 comments

Projects

None yet

10 participants

@christiannelson

I've been sleuthing how simple rails projects with only a few explicit dependencies end up including many others (transitively) that in some cases aren't being used at all. This bloat is fuel for rails critics and wastes cycles each time Bundle.require is invoked.

We have been using capybara for years on possibly as many as 20 projects, and as far as I know we haven't used it with a selenium driver for at least 2 years. I have to believe we're not the only ones who always pair capybara with something else (e.g. headless-webkit or poltergeist). However, we end up dragging selenium-webkit (and libwebsocket, websocket, and rubyzip) along with us on every project.

Would you consider removing the dependency on selenium-webdriver and leaving that to the user to include, since they know which driver they want to use? Including rack-test provides an out of the box experience (without js support). I can imagine capybara throwing a warning/error if js is required and no driver is set.

If you think this is a bad idea, I would love to understand why. Thanks!

@abotalov
Collaborator

+1

@Odaeus

I came here to raise the same proposal for removal of the selenium-webdriver dependency, I've only ever used capybara-webkit on projects. 👍

@jnicklas
Owner

This has come up before. I'm not against the idea, but in that case I'd rather split the selenium driver off into its own gem. But then everything becomes a lot more complicated. We would have something like capybara-core and then a capybara metagem, kind of like RSpec (though I don't like the fact that they split their gems into separate repos). I've tried to do this at some point and it's a lot of work. If anyone feels up to the challenge, then please go ahead.

I would however like that the following conditions are met:

  1. One single repo
  2. It must be just as easy to run the tests
  3. It should not be significantly more difficult to build and distribute the gems
  4. The capybara gem must still depend on selenium webdriver, so that we can provide an optinal out-of-the-box experience
@wildchild

But then everything becomes a lot more complicated.

Why it's complicated? It's simple to add a line to Gemfile. Just mention it in README.

@christiannelson

The reason I haven't taken a stab at this is because I felt it does get more complicated given the requirements given by @jnicklas. I see where he's coming from, but have a preference for a different solution. This baby belongs to @jnicklas, so he gets to set the rules. :)

Using bundler as an example, I rather like how it handles the bundle viz command. It detects if the necessary gem is available and uses it if it is, or displays a message if it is not. The conditional code around whether selenium-web is available or not is easy to write, simple to understand, and pretty standard practice for ruby gems.

When someone upgrades to the new capybara-without-selenium-web, who is using selenium-web, they see a message to include one line in their Gemfile. Once done, they never think about it again. If they're not using selenium-web, then they would already have the driver they depend on in the Gemfile.

This approach would meet all requirements but 4.

@morgoth

Solution provided by @christiannelson looks perfect. Separate gem that connects capybara and selenium-webdriver.
Something like sass-rails for Sass and Rails

@christiannelson

@morgoth I think you're describing @jnicklas's solution, not mine. My recommendation does not introduce a new gem.

Ultimately, either solution would be lovely and meet the goal of not dragging in gems you don't need.

@morgoth

@christiannelson right, sorry for confusion.
@jnicklas Which solution would you accept?

  • Separate gem (something like sass-rails)
  • Wrapper for selenium still in capybara, but without requirement in gemspec also with displaying info to add selenium-webdriver when used

Both solutions require adding gem to Gemfile from user manually (capybara-selenium-webdriver or selenium-webdriver).
Solution with capybara-core and meta gem doesn't look friendly for user.

@jnicklas
Owner

I'm leaning towards the second solution, personally. Maybe make rack-test optional as well. Everyone who uses Rails has it in their Gemfile implicitly anyway, since Rails depends on it.

@jnicklas
Owner

It's about time we stop depending on selenium-webdriver. Let's do it for Capybara 2.1.

@jnicklas jnicklas pushed a commit that closed this issue Feb 24, 2013
CJ Kihlbom and Jonas Nicklas Make selenium-webdriver an optional dependency, closes #887 cd1b966
@jnicklas jnicklas closed this in cd1b966 Feb 24, 2013
@jgarber

Should it be removed from https://github.com/jnicklas/capybara/blob/master/lib/capybara.rb#L321 as well? Without selenium-webdriver in my Gemfile, I get the error just loading capybara.

@morgoth

I think this commit is responsible for that: c3e75f8
/cc @jnicklas

@olivierlacan

At the risk of piling on, I'm seeing that same issue, had to put gem 'selenium-webdriver' in my Gemfile too although I don't use it.

Here's the error:

Capybara's selenium driver is unable to load `selenium-webdriver`, please install the gem and add `gem 'selenium-webdriver'` to your Gemfile if you are using bundler. (LoadError)
@olivierlacan

My bad, this has been fixed in #1018.

@phoet

is this fix included in any version?

@jnicklas
Owner

@phoet it'll be in 2.1 which isn't released yet. It'll be released in a couple of weeks at most.

@phoet

👍 i tried upgrading from 1.x but skiped that because i did not find a compatible version of capybara and capybara-webkit that did not break all of our spec...

@rogerkk

I'm seeing the same error as @olivierlacan, even though I'm running 2.1.

The error only occurs on the first test in each file, when a spec file is run separately. When running the whole suite, there is no problem. Anyone have an idea what might be going on here?

@jnicklas
Owner

@rogerkk maybe you're not running this particular spec through bundler, or something? The error that @olivierlacan was having has been fixed, so something else is probably going on. Make sure that selenium webdriver is actually installed and in the load path.

@rogerkk

@jnicklas Thanks for pitching in! I suspect this discussion really belongs somewhere else, but:

I'm definitely running it using bundler, but I'm also trying to avoid using selenium. Thus had removed selenium-webdriver from my Gemfile, included capybara-webkit, and put "Capybara.javascript_driver = :webkit" in the before block of my spec_helper.rb.

I just seem to have resolved the problem by moving the "Capybara.javascript_driver = :webkit" out of the before block. I obviously don't know what I'm doing. ;)

@jnicklas
Owner

@rogerkk alright, yeah, you probably want to configure that globally, since it's a global setting, unlike Capybara.current_driver which should be local to one particular spec. The order that before blocks run in can be a bit confusing, so the change to webkit was probably triggering too late.

@rogerkk

@jnicklas Aha, I see. Thanks for clearing that up!

@Marchino Marchino pushed a commit to Marchino/spree_variant_options that referenced this issue Feb 21, 2014
@Numerico Numerico added selenium-webkit gem as was removed in capybara a6a43bc
@satoryu satoryu pushed a commit that referenced this issue Jun 7, 2014
@seenmyfate seenmyfate Revert "🐼 857: Load tasks from the deploy.rb"
This reverts commit a5b76f7.

This commit which looked to solve #867 / #857 has caused regressions in
other areas - I'm reverting this commit because the solution to the original
issue of tasks declared in `config/deploy.rb` not appearing in the list of
tasks returned by `cap -T` is to move those tasks into
`lib/capistrano/tasks` and load them from the Capfile (as per the
generated template). Defining tasks in a configuration file may work as
far as running the tasks goes, but it's not the intended use for that
file.

This commit closes #887, and I suspect #881 as well.
1f86d32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment