Skip to content

Commit

Permalink
Refactor/rubocop autofixes on selenium rake (SeleniumHQ#7671)
Browse files Browse the repository at this point in the history
* Autofixes for rake_tasks folder
* Massively cull the unused keys of the Browsers hash
* Slight drying up of checks class
* Dry up checks to only retain the checker methods we use
* Avoid frozenstringliteral issues by using squiggly heredocs
*Also tidies up the very systemic creation of starting points of c++/java files
* Dry up class usages by using Decorator approach and pass class instance directly in (By default for detonatinghandler)
* Fix up a couple of areas that would complain due to frozen_string failures
* Reduce complexity of crazy-fun methodology, and tidy up some of the formatter tabbing
* Slightly tidy up checks for present? to be a bit more comprehensible
* Begin to remove un-required code
* c_checks no longer required
* BROWSERS props not consumed
  • Loading branch information
luke-hill authored and shs96c committed Oct 16, 2019
1 parent 925fee2 commit 7ac1b7c
Show file tree
Hide file tree
Showing 11 changed files with 110 additions and 235 deletions.
5 changes: 2 additions & 3 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ require 'rake_tasks/crazy_fun/mappings/rename'
require 'rake_tasks/crazy_fun/mappings/ruby'

# Location of all new methods
require 'rake_tasks/selenium_rake/c_tasks'
require 'rake_tasks/selenium_rake/checks'
require 'rake_tasks/selenium_rake/ie_code_generator'
require 'rake_tasks/selenium_rake/java_formatter'
Expand Down Expand Up @@ -213,8 +212,8 @@ task :test_java_webdriver => [
]

task :test_java_webdriver => [:test_ie] if SeleniumRake::Checks.windows?
task :test_java_webdriver => [:test_chrome] if SeleniumRake::Checks.present?("chromedriver")
task :test_java_webdriver => [:test_edge] if SeleniumRake::Checks.present?("msedgedriver")
task :test_java_webdriver => [:test_chrome] if SeleniumRake::Checks.chrome?
task :test_java_webdriver => [:test_edge] if SeleniumRake::Checks.edge?
task :test_java_webdriver => [:test_opera] if SeleniumRake::Checks.opera?

task :test_java => [
Expand Down
4 changes: 4 additions & 0 deletions rake_tasks/selenium_rake/base_generator.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module SeleniumRake
class BaseGenerator
def create_deps_(out, args)
Expand All @@ -14,6 +16,8 @@ def create_deps_(out, args)
t.out = out
end

private

def add_deps_(task_name, srcs)
return if srcs.nil?

Expand Down
73 changes: 11 additions & 62 deletions rake_tasks/selenium_rake/browsers.rb
Original file line number Diff line number Diff line change
@@ -1,73 +1,30 @@
# frozen_string_literal: true

require 'rake_tasks/selenium_rake/checks'

module SeleniumRake
class Browsers
BROWSERS = {
'ff' => {
python: {
driver: 'Marionette'
},
java: {
class: 'org.openqa.selenium.firefox.SynthesizedFirefoxDriver',
deps: ['//java/client/test/org/openqa/selenium/testing/drivers']
},
browser_name: 'firefox'
python: { driver: 'Marionette' },
},
'marionette' => {
python: {
driver: 'Marionette'
},
java: {
class: 'org.openqa.selenium.firefox.SynthesizedFirefoxDriver',
deps: ['//java/client/test/org/openqa/selenium/testing/drivers']
},
browser_name: 'firefox'
python: { driver: 'Marionette' },
},
'ie' => {
python: {
driver: 'Ie'
},
java: {
class: 'org.openqa.selenium.ie.InternetExplorerDriver',
deps: ['//java/client/src/org/openqa/selenium/ie:ie', '//cpp/iedriverserver:win32']
},
browser_name: 'internet explorer',
available: SeleniumRake::Checks.windows?
python: { driver: 'Ie' },
},
'edge' => {
python: {
driver: 'Edge'
},
browser_name: 'MicrosoftEdge',
available: SeleniumRake::Checks.windows?
python: { driver: 'Edge' },
},
'chrome' => {
python: {
driver: 'Chrome'
},
java: {
class: 'org.openqa.selenium.chrome.ChromeDriver',
deps: ['//java/client/src/org/openqa/selenium/chrome:chrome']
},
browser_name: 'chrome',
available: SeleniumRake::Checks.chrome?
python: { driver: 'Chrome' },
},
'chromiumedge' => {
python: {
driver: 'ChromiumEdge'
},
java: {
class: 'org.openqa.selenium.edge.EdgeDriver',
deps: ['//java/client/src/org/openqa/selenium/edge:edge']
},
browser_name: 'MicrosoftEdge',
available: SeleniumRake::Checks.edge?
python: { driver: 'ChromiumEdge' },
},
'blackberry' => {
python: {
driver: 'BlackBerry'
},
browser_name: 'blackberry'
python: { driver: 'BlackBerry' },
},
'remote_firefox' => {
python: {
Expand All @@ -80,16 +37,8 @@ class Browsers
}
},
'safari' => {
python: {
driver: 'Safari'
},
java: {
class: 'org.openqa.selenium.safari.SafariDriver',
deps: ['//java/client/src/org/openqa/selenium/safari:safari']
},
browser_name: 'safari',
available: SeleniumRake::Checks.mac?
python: { driver: 'Safari' },
}
}
}.freeze
end
end
91 changes: 0 additions & 91 deletions rake_tasks/selenium_rake/c_tasks.rb

This file was deleted.

78 changes: 32 additions & 46 deletions rake_tasks/selenium_rake/checks.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
# frozen_string_literal: true

require 'rbconfig'

module SeleniumRake
class Checks
class << self
PRESENT_CACHE = {}

def windows?
(RbConfig::CONFIG['host_os'] =~ /mswin|msys|mingw32/) != nil
end
Expand All @@ -11,28 +15,12 @@ def mac?
(RbConfig::CONFIG['host_os'] =~ /darwin|mac os/) != nil
end

def linux?
(RbConfig::CONFIG['host_os'] =~ /linux/) != nil
end

def cygwin?
RUBY_PLATFORM.downcase.include?('cygwin')
end

def dir_separator
File::ALT_SEPARATOR || File::SEPARATOR
end

def env_separator
File::PATH_SEPARATOR
end

def jruby?
RUBY_PLATFORM =~ /java/
end

def path_for(path)
windows? ? path.gsub("/", File::ALT_SEPARATOR || File::SEPARATOR) : path
windows? ? path.gsub('/', dir_separator) : path
end

def classpath_separator?
Expand All @@ -43,26 +31,6 @@ def classpath_separator?
end
end

PRESENT_CACHE = {}

# Checking for particular applications
# This "I believe" can be made private - Luke - Sep 2019
def present?(arg)
return PRESENT_CACHE[arg] if PRESENT_CACHE.key?(arg)

prefixes = ENV['PATH'].split(File::PATH_SEPARATOR)

bool = prefixes.any? do |prefix|
File.exist?(prefix + File::SEPARATOR + arg)
end

bool = File.exist?("/Applications/#{arg}.app") if !bool && mac?

PRESENT_CACHE[arg] = bool

bool
end

def chrome?
present?('chromedriver') || present?('chromedriver.exe')
end
Expand All @@ -71,22 +39,40 @@ def edge?
present?('msedgedriver') || present?('msedgedriver.exe')
end

# Think of the confusion if we called this "g++"
def gcc?
linux? && present?('g++')
end

def msbuild_installed?
windows? && present?('msbuild.exe')
end

def opera?
present?('opera') || present?('Opera')
end

def python?
present?('python') || present?('python.exe')
end

private

def cygwin?
RUBY_PLATFORM.downcase.include?('cygwin')
end

def present?(arg)
return PRESENT_CACHE[arg] if PRESENT_CACHE.key?(arg)
return PRESENT_CACHE[arg] = true if exist_on_non_mac?(arg)
return PRESENT_CACHE[arg] = exist_on_mac?(arg) if mac?
PRESENT_CACHE[arg] = false
end

def exist_on_non_mac?(arg)
prefixes.any? do |prefix|
File.exist?("#{prefix}#{File::SEPARATOR}#{arg}")
end
end

def exist_on_mac?(arg)
File.exist?("/Applications/#{arg}.app")
end

def prefixes
ENV['PATH'].split(File::PATH_SEPARATOR)
end
end
end
end
13 changes: 8 additions & 5 deletions rake_tasks/selenium_rake/cpp_formatter.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
# frozen_string_literal: true

# This file auto-generates code for the IE driver. To make different language
# bindings easier to maintain, it generates code for mapping numeric return
# type identifiers (returned from wdGetScriptResultType) to a meaningful
# string identifier.
module SeleniumRake
class CppFormatter
def generate_file_header
out_str = "/* AUTO GENERATED - Do not edit by hand. */\n"
out_str += "/* See rake-tasks/selenium_rake/cpp_formatter.rb instead. */\n"
out_str += "#ifndef __IE_RETURN_TYPES_H_\n"
out_str += "#define __IE_RETURN_TYPES_H_\n"
out_str
<<~HEREDOC
/* AUTO GENERATED - Do not edit by hand. */
/* See rake-tasks/selenium_rake/cpp_formatter.rb instead. */
#ifndef __IE_RETURN_TYPES_H_
#define __IE_RETURN_TYPES_H_
HEREDOC
end

def generate_file_footer
Expand Down
Loading

0 comments on commit 7ac1b7c

Please sign in to comment.