Add support for proxy servers #47
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Cruikshanks
force-pushed
the
feature/use-proxy-with-drivers
branch
from
981b0a3 to
92903d0
Compare
Cruikshanks
changed the title
Cruikshanks
force-pushed
the
feature/use-proxy-with-drivers
branch
from
92903d0 to
bfdeeb7
Compare
This change adds support to **Quke** to specify a proxy server when using the **PhantomJS**, **Firefox**, and **Chrome** drivers. The reasoning is that a connection to the internet may be been setup to go via a proxy server for security purposes in some build environments. With no way currently to tell **Quke**, it would not be able to access the site under test. To add this support the following changes have been made - `Quke::Configuration` will now accept a new section in the `.config.yml` called `proxy`, containing the values `host` and `port` - `Quke::Configuration` has a new helper method which returns whether proxy settings need to be applied to the driver - `Quke::DriverRegistration` now contains logic specific to each driver for specifying proxy details (if required) - The methods which returned config options for **Poltergeist** and **PhantomJS** have been moved to `Quke::DriverRegistration` (more details below) Though it was envisaged that users could set the options for each driver by simply adding some key value pairs into the `.config.yml`, adding support for using a proxy has shown in practise this is not always the case. For example we need to instantiate a `Selenium::WebDriver::Firefox::Profile.new` in order to support setting the proxy for the Firefox driver. Therefore these place holders for passing in options have been removed, and the driver configuration for poltergeist and phantomjs has been moved out of `Quke::Configuration` into `Quke::DriverRegistration`. This brings it inline with the other drivers and now means `Quke::Configuration` is responsible solely for what is (or can be) specified in the config yml file.
Cruikshanks
force-pushed
the
feature/use-proxy-with-drivers
branch
from
bfdeeb7 to
a192b06
Compare
Completing the changes for proxy server support ( 62d33f9 ) meant adding more configuration logic to the existing `Quke::DriverRegistration` class. This put it over the line of being **monolithic**, and felt like we were mixing two different domains; the driver registration logic, and the logic for determining each driver's configuration settings. So this change moves the driver configuration logic into a new class called `Quke::DriverConfiguration`. This has led to knock on changes of how we initialise each of the objects, and how each driver is registered. The benefits are that both classes are now much easier to maintain, and more of our code has test coverage (something that helped spot some omissions in setting up the **Browserstack** capabilities).
Found that these files did not have execute permissions when we attempted to use them so this change fixes that. It also adds `require`'s for all the **Quke** classes, which should make working with them in IRB a little easier.
Cruikshanks
force-pushed
the
feature/use-proxy-with-drivers
branch
from
a192b06 to
c8b86f0
Compare
|
This will resolve #46 when its merged. |
This was referenced Nov 29, 2016
Cruikshanks
added a commit
that referenced
this pull request
Dec 1, 2016
Having previously thought we had done enough to support using **Quke** in an environment where a proxy server is in use (PR #47), we found that is not the case. The problem is that a common configuration is to only use the proxy server when making external connections, but not for local ones. So without the ability to allow **Quke** to configure the drivers in this was, when the **selenium** based drivers attempt to make local connections to the selenium server **Quke** falls over. This change will add support for specifying this option. In the same manner as PR #47 the user simply has to provide the details in the config file, **Quke** will handle how to configure the drivers with this information when registering them with [Capybara](https://github.com/teamcapybara/capybara).
Cruikshanks
added a commit
that referenced
this pull request
Dec 2, 2016
Having previously thought we had done enough to support using **Quke** in an environment where a proxy server is in use (PR #47), we found that is not the case. The problem is that a common configuration is to only use the proxy server when making external connections, but not for local ones. So without the ability to allow **Quke** to configure the drivers in this was, when the **selenium** based drivers attempt to make local connections to the selenium server **Quke** falls over. This change will add support for specifying this option. In the same manner as PR #47 the user simply has to provide the details in the config file, **Quke** will handle how to configure the drivers with this information when registering them with [Capybara](https://github.com/teamcapybara/capybara).
Cruikshanks
added a commit
that referenced
this pull request
Dec 2, 2016
Having previously thought we had done enough to support using **Quke** in an environment where a proxy server is in use (PR #47), we found that is not the case. The problem is that a common configuration is to only use the proxy server when making external connections, but not for local ones. So without the ability to allow **Quke** to configure the drivers in this was, when the **selenium** based drivers attempt to make local connections to the selenium server **Quke** falls over. This change will add support for specifying this option. In the same manner as PR #47 the user simply has to provide the details in the config file, **Quke** will handle how to configure the drivers with this information when registering them with [Capybara](https://github.com/teamcapybara/capybara).
Cruikshanks
added a commit
that referenced
this pull request
Dec 2, 2016
Having previously thought we had done enough to support using **Quke** in an environment where a proxy server is in use (PR #47), we found that is not the case. The problem is that a common configuration is to only use the proxy server when making external connections, but not for local ones. So without the ability to allow **Quke** to configure the drivers in this was, when the **selenium** based drivers attempt to make local connections to the selenium server **Quke** falls over. This change will add support for specifying this option. In the same manner as PR #47 the user simply has to provide the details in the config file, **Quke** will handle how to configure the drivers with this information when registering them with [Capybara](https://github.com/teamcapybara/capybara).
Cruikshanks
added a commit
that referenced
this pull request
Dec 13, 2016
Having previously thought we had done enough to support using **Quke** in an environment where a proxy server is in use (PR #47), we found that is not the case. The problem is that a common configuration is to only use the proxy server when making external connections, but not for local ones. So without the ability to allow **Quke** to configure the drivers in this was, when the **selenium** based drivers attempt to make local connections to the selenium server **Quke** falls over. This change will add support for specifying this option. In the same manner as PR #47 the user simply has to provide the details in the config file, **Quke** will handle how to configure the drivers with this information when registering them with [Capybara](https://github.com/teamcapybara/capybara).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change adds support to Quke to specify a proxy server when using the PhantomJS, Firefox, and Chrome drivers.
The reasoning is that a connection to the internet may be been setup to go via a proxy server for security purposes in some build environments. With no way currently to tell Quke, it would not be able to access the site under test.
To add this support the following changes have been made
Quke::Configurationwill now accept a new section in the.config.ymlcalledproxy, containing the valueshostandportQuke::Configurationhas a new helper method which returns whether proxy settings need to be applied to the driverQuke::DriverRegistrationnow contains logic specific to each driver for specifying proxy details (if required)Quke::DriverRegistration(more details below)Though it was envisaged that users could set the options for each driver by simply adding some key value pairs into the
.config.yml, adding support for using a proxy has shown in practise this is not always the case. For example we need to instantiate aSelenium::WebDriver::Firefox::Profile.newin order to support setting the proxy for the Firefox driver.Therefore these place holders for passing in options have been removed, and the driver configuration for poltergeist and phantomjs has been moved out of
Quke::ConfigurationintoQuke::DriverRegistration. This brings it inline with the other drivers and now meansQuke::Configurationis responsible solely for what is (or can be) specified in the config yml file.