Skip to content
This repository has been archived by the owner on Apr 1, 2019. It is now read-only.

Rename chromedriver so it does not conflict with system installed #58

Merged
merged 1 commit into from Sep 15, 2018

Conversation

mfazekas
Copy link
Contributor

@mfazekas mfazekas commented Aug 14, 2018

Fixes: #57
Rename the bin shim from chromedriver to chromedriver-helper so it doesn't shadows system installed one (which can break projects not using chromedriver-helper)

Seems to work fine with a new default rails 5.2.1 project. Assumes the project is using Selenium::WebDriver and that it's required before (we could be issuing and user friendly message if this is not the case)

@flavorjones
Copy link
Owner

Hi,

Thanks for submitting this proposed change.

This is a breaking change. Part of the value of chromedriver-helper is that it shadows the system-installed version (and calls it if it's present). I'm not sure I want to do it, but surely if we do it we'll need documentation updated among other things.

I'm assuming this is related to the discussion happening at #57 and so I'd like to continue the conversation there about possible ways to address that underlying problem, before we consider making a significant breaking change of this nature.

Hope that all makes sense!

@mfazekas
Copy link
Contributor Author

@flavorjones sure this is a possible and working solution to the problem, and sure it's a somewhat breaking change so increasing major version would be suggested.

@flavorjones
Copy link
Owner

Reopened because of conversation at #57. Will re-examine my assumptions.

@flavorjones flavorjones reopened this Sep 15, 2018
@flavorjones
Copy link
Owner

I'd like to play with some upgrade scenarios before merging. Should be able to do that today.

@flavorjones
Copy link
Owner

flavorjones commented Sep 15, 2018

Scenarios to test:

  • clean install of post-patch
  • upgrade from pre-patch to post-patch
  • two projects, one with pre-patch installation, one with post-patch installation

@flavorjones
Copy link
Owner

flavorjones commented Sep 15, 2018

Then actions to take:

  • update README with a section that explains the problem and instructs users to upgrade
  • update README to reflect the new implementation

(I considered renaming chromedriver-update as chromedriver-helper-update but couldn't come up with a compelling reason to do so.)

@flavorjones flavorjones merged commit 7e90604 into flavorjones:master Sep 15, 2018
@flavorjones flavorjones added this to the v2.0.0 milestone Sep 15, 2018
@flavorjones
Copy link
Owner

This will be in a v2.0.0 release shortly. I've bumped the major version because of the backwards incompatibility (Some people may have scripts? I honestly don't know.)

y-yagi added a commit to y-yagi/rails that referenced this pull request Sep 16, 2018
The bin shim provides by `chromedriver-helper` gem has renamed to
`chromedriver-helper` since 2.0.
flavorjones/chromedriver-helper#58

Since bin of new name is set to driver path in
`lib/chromedriver-helper.rb`, need to load it.
mcary added a commit to mcary/chromedriver-helper that referenced this pull request Sep 19, 2018
I'm seeing this error:

    unable to connect to chromedriver 127.0.0.1:9515 (Selenium::WebDriver::Error::WebDriverError)

PR flavorjones#58 changes the "chromedriver" executable file name to "chromedriver-helper", but I found that Selenium::WebDriver::Chrome.driver_path was nil after trying to visit a page (after rescuing from the failure). 

The problem is that I was never loading the new chromedriver-helper code to set the custom driver_path.  I added this to features/support/env.rb and my Cucumber scenario started working:

    require 'chromedriver-helper'

I'm using this outside of Rails in a standalone test suite to test an app that is developed separately.  (Rails may be requiring the gem behind the scenes.)

Mention the possible need to require 'chromedriver-helper' in the Standalone section of the README, where people will be looking for installation instructions.
mcary added a commit to mcary/chromedriver-helper that referenced this pull request Sep 19, 2018
I'm seeing this error:

    unable to connect to chromedriver 127.0.0.1:9515
      (Selenium::WebDriver::Error::WebDriverError)

PR flavorjones#58 changes the "chromedriver" executable file name to
"chromedriver-helper", but I found that
Selenium::WebDriver::Chrome.driver_path was nil after trying to visit a
page (after rescuing from the failure).

The problem is that I was never loading the new chromedriver-helper code
to set the custom driver_path.  I added this to features/support/env.rb
and my Cucumber scenario started working:

    require "chromedriver-helper"

I'm using this outside of Rails in a standalone test suite to test an
app that is developed separately.  (Rails may be requiring the gem
behind the scenes.)

Mention the possible need to require "chromedriver-helper" in the
Standalone section of the README, where people will be looking for
installation instructions.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants