Skip to content

Commit d338de3

Browse files
titusfortnertitusfortner
committed
[rb] be more strict with options and capabilities
1 parent 8dde702 commit d338de3

13 files changed

Lines changed: 170 additions & 20 deletions

File tree

‎rb/lib/selenium/webdriver/chrome/driver.rb‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,12 @@ module Chrome
2929
#
3030

3131
class Driver < Chromium::Driver
32-
def initialize(service: nil, url: nil, **opts)
32+
def initialize(capabilities: nil, options: nil, service: nil, url: nil, **opts)
3333
raise ArgumentError, "Can't initialize #{self.class} with :url" if url
3434

35+
caps = process_options(options, capabilities)
3536
url = service_url(service || Service.chrome)
36-
super(url: url, **opts)
37+
super(caps: caps, url: url, **opts)
3738
end
3839

3940
def browser
@@ -45,6 +46,16 @@ def browser
4546
def devtools_address
4647
"http://#{capabilities['goog:chromeOptions']['debuggerAddress']}"
4748
end
49+
50+
def process_options(options, capabilities)
51+
if options && !options.is_a?(Options)
52+
raise ArgumentError, ":options must be an instance of #{Options}"
53+
elsif options.nil? && capabilities.nil?
54+
capabilities = Remote::Capabilities.chrome
55+
end
56+
57+
super(options, capabilities)
58+
end
4859
end # Driver
4960
end # Chrome
5061
end # WebDriver

‎rb/lib/selenium/webdriver/common/driver.rb‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,13 +308,21 @@ def ref
308308

309309
attr_reader :bridge
310310

311-
def create_bridge(capabilities: nil, options: nil, url: nil, http_client: nil)
311+
def create_bridge(caps:, url:, http_client: nil)
312312
Remote::Bridge.new(http_client: http_client, url: url).tap do |bridge|
313-
generated_caps = options ? options.as_json : generate_capabilities(capabilities)
314-
bridge.create_session(generated_caps)
313+
bridge.create_session(caps)
315314
end
316315
end
317316

317+
def process_options(options, capabilities)
318+
if options && capabilities
319+
msg = "Don't use both :options and :capabilities when initializing #{self.class}, prefer :options"
320+
raise ArgumentError, msg
321+
end
322+
323+
options ? options.as_json : generate_capabilities(capabilities)
324+
end
325+
318326
def generate_capabilities(capabilities)
319327
Array(capabilities).map { |cap|
320328
if cap.is_a? Symbol
@@ -324,7 +332,7 @@ def generate_capabilities(capabilities)
324332
raise ArgumentError, msg
325333
end
326334
cap.as_json
327-
}.inject(:merge) || Remote::Capabilities.send(browser || :new)
335+
}.inject(:merge)
328336
end
329337

330338
def service_url(service)

‎rb/lib/selenium/webdriver/edge/driver.rb‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,12 @@ module Edge
2929
#
3030

3131
class Driver < Chromium::Driver
32-
def initialize(service: nil, url: nil, **opts)
32+
def initialize(capabilities: nil, options: nil, service: nil, url: nil, **opts)
3333
raise ArgumentError, "Can't initialize #{self.class} with :url" if url
3434

35+
caps = process_options(options, capabilities)
3536
url = service_url(service || Service.edge)
36-
super(url: url, **opts)
37+
super(caps: caps, url: url, **opts)
3738
end
3839

3940
def browser
@@ -45,6 +46,16 @@ def browser
4546
def devtools_address
4647
"http://#{capabilities['ms:edgeOptions']['debuggerAddress']}"
4748
end
49+
50+
def process_options(options, capabilities)
51+
if options && !options.is_a?(Options)
52+
raise ArgumentError, ":options must be an instance of #{Options}"
53+
elsif options.nil? && capabilities.nil?
54+
capabilities = Remote::Capabilities.edge
55+
end
56+
57+
super(options, capabilities)
58+
end
4859
end # Driver
4960
end # Edge
5061
end # WebDriver

‎rb/lib/selenium/webdriver/firefox/driver.rb‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,12 @@ class Driver < WebDriver::Driver
3737
DriverExtensions::HasWebStorage,
3838
DriverExtensions::PrintsPage].freeze
3939

40-
def initialize(service: nil, url: nil, **opts)
40+
def initialize(capabilities: nil, options: nil, service: nil, url: nil, **opts)
4141
raise ArgumentError, "Can't initialize #{self.class} with :url" if url
4242

43+
caps = process_options(options, capabilities)
4344
url = service_url(service || Service.firefox)
44-
super(url: url, **opts)
45+
super(caps: caps, url: url, **opts)
4546
end
4647

4748
def browser
@@ -64,6 +65,16 @@ def devtools_url
6465
def devtools_version
6566
Firefox::DEVTOOLS_VERSION
6667
end
68+
69+
def process_options(options, capabilities)
70+
if options && !options.is_a?(Options)
71+
raise ArgumentError, ":options must be an instance of #{Options}"
72+
elsif options.nil? && capabilities.nil?
73+
capabilities = Remote::Capabilities.firefox
74+
end
75+
76+
super(options, capabilities)
77+
end
6778
end # Driver
6879
end # Firefox
6980
end # WebDriver

‎rb/lib/selenium/webdriver/ie/driver.rb‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,29 @@ module IE
3030
class Driver < WebDriver::Driver
3131
EXTENSIONS = [DriverExtensions::HasWebStorage].freeze
3232

33-
def initialize(service: nil, url: nil, **opts)
33+
def initialize(capabilities: nil, options: nil, service: nil, url: nil, **opts)
3434
raise ArgumentError, "Can't initialize #{self.class} with :url" if url
3535

36+
caps = process_options(options, capabilities)
3637
url = service_url(service || Service.ie)
37-
super(url: url, **opts)
38+
super(caps: caps, url: url, **opts)
3839
end
3940

4041
def browser
4142
:internet_explorer
4243
end
44+
45+
private
46+
47+
def process_options(options, capabilities)
48+
if options && !options.is_a?(Options)
49+
raise ArgumentError, ":options must be an instance of #{Options}"
50+
elsif options.nil? && capabilities.nil?
51+
capabilities = Remote::Capabilities.ie
52+
end
53+
54+
super(options, capabilities)
55+
end
4356
end # Driver
4457
end # IE
4558
end # WebDriver

‎rb/lib/selenium/webdriver/remote/driver.rb‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,12 @@ class Driver < WebDriver::Driver
3030
include DriverExtensions::UploadsFiles
3131
include DriverExtensions::HasSessionId
3232

33-
def initialize(service: nil, url: nil, **opts)
33+
def initialize(capabilities: nil, options: nil, service: nil, url: nil, **opts)
3434
raise ArgumentError, "Can not set :service object on #{self.class}" if service
3535

3636
url ||= "http://#{Platform.localhost}:4444/wd/hub"
37-
super(url: url, **opts)
37+
caps = process_options(options, capabilities)
38+
super(caps: caps, url: url, **opts)
3839
@bridge.file_detector = ->((filename, *)) { File.exist?(filename) && filename.to_s }
3940
end
4041

@@ -48,6 +49,12 @@ def devtools_version
4849
capabilities['se:cdpVersion']&.split('.')&.first ||
4950
raise(Error::WebDriverError, "DevTools is not supported by the Remote Server")
5051
end
52+
53+
def process_options(options, capabilities)
54+
raise ArgumentError, "#{self.class} needs :options to be set" if options.nil? && capabilities.nil?
55+
56+
super(options, capabilities)
57+
end
5158
end # Driver
5259
end # Remote
5360
end # WebDriver

‎rb/lib/selenium/webdriver/safari/driver.rb‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,29 @@ class Driver < WebDriver::Driver
3131
DriverExtensions::HasApplePermissions,
3232
DriverExtensions::HasWebStorage].freeze
3333

34-
def initialize(service: nil, url: nil, **opts)
34+
def initialize(capabilities: nil, options: nil, service: nil, url: nil, **opts)
3535
raise ArgumentError, "Can't initialize #{self.class} with :url" if url
3636

37+
caps = process_options(options, capabilities)
3738
url = service_url(service || Service.safari)
38-
super(url: url, **opts)
39+
super(caps: caps, url: url, **opts)
3940
end
4041

4142
def browser
4243
:safari
4344
end
45+
46+
private
47+
48+
def process_options(options, capabilities)
49+
if options && !options.is_a?(Options)
50+
raise ArgumentError, ":options must be an instance of #{Options}"
51+
elsif options.nil? && capabilities.nil?
52+
capabilities = Remote::Capabilities.safari
53+
end
54+
55+
super(options, capabilities)
56+
end
4457
end # Driver
4558
end # Safari
4659
end # WebDriver

‎rb/spec/unit/selenium/webdriver/chrome/driver_spec.rb‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,20 @@ def expect_request(body: nil, endpoint: nil)
5454
expect { Driver.new(options: Options.new(**opts)) }.not_to raise_exception
5555
end
5656

57+
it 'does not accept Options of the wrong class' do
58+
expect {
59+
Driver.new(options: Options.firefox)
60+
}.to raise_exception(ArgumentError, ':options must be an instance of Selenium::WebDriver::Chrome::Options')
61+
end
62+
63+
it 'does not allow both Options and Capabilities' do
64+
msg = "Don't use both :options and :capabilities when initializing Selenium::WebDriver::Chrome::Driver, " \
65+
"prefer :options"
66+
expect {
67+
Driver.new(options: Options.new, capabilities: Remote::Capabilities.chrome)
68+
}.to raise_exception(ArgumentError, msg)
69+
end
70+
5771
context 'with :capabilities' do
5872
it 'accepts value as a Symbol' do
5973
expect_request(body: {capabilities: {alwaysMatch: {browserName: "chrome"}}})

‎rb/spec/unit/selenium/webdriver/edge/driver_spec.rb‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,20 @@ def expect_request(body: nil, endpoint: nil)
5959
expect { Driver.new(invalid: 'foo') }.to raise_error(ArgumentError, msg)
6060
end
6161

62+
it 'does not accept Options of the wrong class' do
63+
expect {
64+
Driver.new(options: Options.chrome)
65+
}.to raise_exception(ArgumentError, ':options must be an instance of Selenium::WebDriver::Edge::Options')
66+
end
67+
68+
it 'does not allow both Options and Capabilities' do
69+
msg = "Don't use both :options and :capabilities when initializing Selenium::WebDriver::Edge::Driver, " \
70+
"prefer :options"
71+
expect {
72+
Driver.new(options: Options.new, capabilities: Remote::Capabilities.edge)
73+
}.to raise_exception(ArgumentError, msg)
74+
end
75+
6276
context 'with :capabilities' do
6377
it 'accepts value as a Symbol' do
6478
expect_request(body: {capabilities: {alwaysMatch: {browserName: "MicrosoftEdge"}}})

‎rb/spec/unit/selenium/webdriver/firefox/driver_spec.rb‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,20 @@ def expect_request(body: nil, endpoint: nil)
5656
expect { Driver.new(options: Options.new(**opts)) }.not_to raise_exception
5757
end
5858

59+
it 'does not accept Options of the wrong class' do
60+
expect {
61+
Driver.new(options: Options.chrome)
62+
}.to raise_exception(ArgumentError, ':options must be an instance of Selenium::WebDriver::Firefox::Options')
63+
end
64+
65+
it 'does not allow both Options and Capabilities' do
66+
msg = "Don't use both :options and :capabilities when initializing Selenium::WebDriver::Firefox::Driver, " \
67+
"prefer :options"
68+
expect {
69+
Driver.new(options: Options.new, capabilities: Remote::Capabilities.firefox)
70+
}.to raise_exception(ArgumentError, msg)
71+
end
72+
5973
context 'with :capabilities' do
6074
it 'accepts value as a Symbol' do
6175
expect_request(body: {capabilities: {alwaysMatch: {browserName: "firefox"}}})

0 commit comments

Comments
 (0)