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

Ruby sample code for HTTP proxies section #445

Merged
merged 3 commits into from
Jun 15, 2020

Conversation

masakazutakewaka
Copy link
Contributor

@masakazutakewaka masakazutakewaka commented May 31, 2020

Description

Added Ruby code snippets for the HTTP proxy section across the locales. I tested the code snippet with WEBrick::HTTPProxyServer.

Motivation and Context

It helps users understand how to integrate the library with their http proxy servers.

Types of changes

  • Change to the site (I am attaching a screenshot showing the before and after)
  • Code example added (and I also added the example to all translated languages)
  • Improved translation
  • Added new translation (and I also added a notice to each document missing translation)

Checklist

  • I have read the contributing document.
  • I have used hugo to render the site/docs locally and I am sure it works.

before

スクリーンショット 2020-06-01 0 45 54

after

スクリーンショット 2020-06-14 23 43 08

@CLAassistant
Copy link

CLAassistant commented May 31, 2020

CLA assistant check
All committers have signed the CLA.

@masakazutakewaka
Copy link
Contributor Author

note: as shown in the checklist, I haven't rendered it on my local yet.

Copy link
Member

@harsha509 harsha509 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @masakazutakewaka,

Thank you for the PR.

As we're moving away from desired capabilities, can you please change code sample using driver specific options( like chromeoptions)

You can look into other code samples (except python) for reference.

Regards,
Harsha.

@masakazutakewaka
Copy link
Contributor Author

masakazutakewaka commented Jun 1, 2020

versions:

Ruby 2.6.3
Rails 6.0.3
selenium-webdriver 3.142.7

The proxy option is deprecated, but I believe it is the one of only options that works atm at least for Chrome except desired_capabilities.

2020-06-01 22:51:17 WARN Selenium [DEPRECATION] Selenium::WebDriver::Chrome::Driver#new with `:proxy` parameter is deprecated. Use Selenium::WebDriver::Chrome::Capabilities#proxy= instead.`

I tried to use profile like below, but I think I stumbled upon what seems to be a bug(I think I could send a patch later).

$ irb
irb(main)[01:0]> profile = Selenium::WebDriver::Chrome::Profile.new
irb(main)[02:0]> profile['network.proxy.http'] = 'localhost'
irb(main)[03:0]> profile['network.proxy.http_port'] = 8000
irb(main)[04:0]> options = Selenium::WebDriver::Chrome::Options.new
irb(main)[05:0]> options.profile = profile
irb(main)[06:0]> driver = Selenium::WebDriver.for :chrome, options: options
Traceback (most recent call last):
        1: from (irb):6
NoMethodError (undefined method `split' for :directory:Symbol)

This is caused by Chrome::Options#as_json calling a method with a Symbol argument when it only accepts String.


I also tried using Selenium::WebDriver::Chrome::Capabilities#proxy= as the deprecate warning suggests, but Selenium::WebDriver::Chrome::Capabilities actually doesn't exist.

> defined? Selenium::WebDriver::Chrome::Capabilities
=> nil

There is Selenium::WebDriver::Remote::Capabilities but as far as I know it's passed to a driver via desired_capabilities key.

capabilities option is supported by Chrome::Driver but it says Chrome::Driver is a private api.

module Selenium
  module WebDriver
    module Chrome

      #
      # Driver implementation for Chrome.
      # @api private
      #

      class Driver < WebDriver::Driver

Passing Remote::Capabilities to Chrome::Driver fails because Remote::Bridge#handshake causes an error.

# rails c

irb(main)[01:0]> proxy = Selenium::WebDriver::Proxy.new(http: 'localhost:8000')
irb(main)[02:0]> cap   = Selenium::WebDriver::Remote::Capabilities.chrome(proxy: proxy)
irb(main)[03:0]> driver = Selenium::WebDriver.for :chrome, capabilities: cap
Traceback (most recent call last):
        1: from (irb):3
ArgumentError (unknown option: {:capabilities=>#<Selenium::WebDriver::Remote::Capabilities:0x00007f98945dedf8 @capabilities={:browser_name=>"chrome", :version=>"", :platform=>:any, :javascript_enabled=>true, :css_selectors_enabled=>true, :takes_screenshot=>false, :native_events=>false, :rotatable=>false, :firefox_profile=>nil, :proxy=>#<Selenium::WebDriver::Proxy:0x00007f989314ed70 @type=:manual, @http="localhost:8000">}>})

It seems all of them are fixed in version 4 alpha, so I'm going to upgrade it and try again.

@masakazutakewaka
Copy link
Contributor Author

masakazutakewaka commented Jun 3, 2020

https://github.com/SeleniumHQ/selenium/blob/ab162925f7326431cda18d5e84b2971187f913f5/rb/CHANGES#L25-L34

Ruby:
  * Deprecated passing :desired_capabilities and :options to driver initialization
    in favor of :capabilities. They now can be combined in an array to make
    custom capabilities requirements:
      caps = Selenium::WebDriver::Remote::Capabilities.chrome
      opts = Selenium::WebDriver::Chrome::Options.new
      Selenium::WebDriver.for(:remote, capabilities: :chrome)
      Selenium::WebDriver.for(:remote, capabilities: caps)
      Selenium::WebDriver.for(:remote, capabilities: opts)
      Selenium::WebDriver.for(:remote, capabilities: [caps, opts])

just rewritten the sample codes according to what the latest change log had to say. Maybe merge this PR after version 4 is released? @harsha509

Right now, it's 3.1 on rubygems.org -> https://rubygems.org/gems/selenium-webdriver

スクリーンショット 2020-06-04 1 59 14

@harsha509
Copy link
Member

https://github.com/SeleniumHQ/selenium/blob/ab162925f7326431cda18d5e84b2971187f913f5/rb/CHANGES#L25-L34

Ruby:
  * Deprecated passing :desired_capabilities and :options to driver initialization
    in favor of :capabilities. They now can be combined in an array to make
    custom capabilities requirements:
      caps = Selenium::WebDriver::Remote::Capabilities.chrome
      opts = Selenium::WebDriver::Chrome::Options.new
      Selenium::WebDriver.for(:remote, capabilities: :chrome)
      Selenium::WebDriver.for(:remote, capabilities: caps)
      Selenium::WebDriver.for(:remote, capabilities: opts)
      Selenium::WebDriver.for(:remote, capabilities: [caps, opts])

just rewritten the sample codes according to what the latest change log had to say. Maybe merge this PR after version 4 is released? @harsha509

Right now, it's 3.1 on rubygems.org -> https://rubygems.org/gems/selenium-webdriver

スクリーンショット 2020-06-04 1 59 14

Hi @masakazutakewaka ,

All the code samples in documentation section are tested with latest Alpha versions. Please let me know if you can try it out with alpha versions.

Regards,
Harsha.

@harsha509 harsha509 added the needs-testing Code needs to be tested locally to get it merged label Jun 5, 2020
@masakazutakewaka
Copy link
Contributor Author

Hi @harsha509,

capabilities option was introduced in version 4.0.0.alpha6, which is the latest alpha version, so it doesn't work with older alpha versions(1~5). I've also confirmed it on my local. Should it suffice or need to wait till alpha gets stripped of version 4?

@diemol diemol changed the base branch from master to dev June 5, 2020 22:01
Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@masakazutakewaka we can have the code samples with the latest alpha, that is fine, this is another way to motivate people to migrate. Perhaps you can add a comment in the code snippet saying that the code was written using Selenium 4 as a dependency.

@masakazutakewaka
Copy link
Contributor Author

@diemol sure, that makes sense. I've just added comments to each locale. Would you mind checking it when you have a chance?

Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, what do you think @harsha509?

@harsha509
Copy link
Member

Hi @diemol,

PR Looks good to me now. Thanks for providing comments and resolving it. We can merge it now

Regards,
Harsha

Copy link
Member

@harsha509 harsha509 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @masakazutakewaka !

Congratulations on your first contribution ! 🥳

@harsha509 harsha509 merged commit 069131d into SeleniumHQ:dev Jun 15, 2020
selenium-ci added a commit that referenced this pull request Jun 15, 2020
Co-authored-by: Diego Molina <diemol@users.noreply.github.com>
Co-authored-by: Sri Harsha <Harsha509@users.noreply.github.com> 069131d
@masakazutakewaka
Copy link
Contributor Author

Thank you for reviewing & merging it:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants