Refactor/ruby rake tasks tidy (#7551)

* Autofixed 95% of the cop issues * Additional auto-fixes from rubocop * Final partial completed fixes of rubocop auto-correct * Undo YodaCondition in rake-tasks/checks.rb - Making further simplication easier * Fix up some string interpolation improvements and compress some lines down where appropriate * String interpolation fixes * CR: More tidy-up: Remove un-required syntax
SeleniumHQ · Sep 11, 2019 · 3f0dd43 · 3f0dd43
1 parent 798123f
commit 3f0dd43
Show file tree

Hide file tree

Showing 10 changed files with 195 additions and 214 deletions.
diff --git a/AUTHORS b/AUTHORS
@@ -239,6 +239,7 @@ lhuang <lhuang@example.com>
 lsowen <lowen@misoenergy.org>
 Lucas Tierney <lucast1533@gmail.com>
 Luke Inman-Semerau <luke.semerau@gmail.com>
+Luke Hill <lukehill_uk@hotmail.com>
 lukec <lukec@example.com>
 Lukáš Linhart <lukas.linhart.1981@gmail.com>
 Magnus E. Halvorsen <mhalv@nets.eu>

diff --git a/rake-tasks/browsers.rb b/rake-tasks/browsers.rb
@@ -3,91 +3,91 @@
 #:available => whether this browser is available on this computer. Defaults to true.
 
 BROWSERS = {
-  "ff" => {
-    :python => {
-      :driver => "Marionette",
+  'ff' => {
+    python: {
+      driver: 'Marionette'
     },
-    :java => {
-      :class => "org.openqa.selenium.firefox.SynthesizedFirefoxDriver",
-      :deps => [ "//java/client/test/org/openqa/selenium/testing/drivers" ]
+    java: {
+      class: 'org.openqa.selenium.firefox.SynthesizedFirefoxDriver',
+      deps: ['//java/client/test/org/openqa/selenium/testing/drivers']
     },
-    :browser_name => "firefox",
+    browser_name: 'firefox'
   },
-  "marionette" => {
-    :python => {
-      :driver => "Marionette",
+  'marionette' => {
+    python: {
+      driver: 'Marionette'
     },
-    :java => {
-      :class => "org.openqa.selenium.firefox.SynthesizedFirefoxDriver",
-      :deps => [ "//java/client/test/org/openqa/selenium/testing/drivers" ]
+    java: {
+      class: 'org.openqa.selenium.firefox.SynthesizedFirefoxDriver',
+      deps: ['//java/client/test/org/openqa/selenium/testing/drivers']
     },
-    :browser_name => "firefox",
+    browser_name: 'firefox'
   },
-  "ie" => {
-    :python => {
-      :driver => "Ie",
+  'ie' => {
+    python: {
+      driver: 'Ie'
     },
-    :java => {
-      :class => "org.openqa.selenium.ie.InternetExplorerDriver",
-      :deps => [ "//java/client/src/org/openqa/selenium/ie:ie", "//cpp/iedriverserver:win32" ]
+    java: {
+      class: 'org.openqa.selenium.ie.InternetExplorerDriver',
+      deps: ['//java/client/src/org/openqa/selenium/ie:ie', '//cpp/iedriverserver:win32']
     },
-    :browser_name => "internet explorer",
-    :available => windows?
+    browser_name: 'internet explorer',
+    available: windows?
   },
-  "edge" => {
-    :python => {
-      :driver => "Edge",
+  'edge' => {
+    python: {
+      driver: 'Edge'
     },
-    :browser_name => "MicrosoftEdge",
-    :available => windows?
+    browser_name: 'MicrosoftEdge',
+    available: windows?
   },
-  "chrome" => {
-    :python => {
-      :driver => "Chrome",
+  'chrome' => {
+    python: {
+      driver: 'Chrome'
     },
-    :java => {
-      :class => "org.openqa.selenium.chrome.ChromeDriver",
-      :deps => [ "//java/client/src/org/openqa/selenium/chrome:chrome" ]
+    java: {
+      class: 'org.openqa.selenium.chrome.ChromeDriver',
+      deps: ['//java/client/src/org/openqa/selenium/chrome:chrome']
     },
-    :browser_name => "chrome",
-    :available => chrome?
+    browser_name: 'chrome',
+    available: chrome?
   },
-  "chromiumedge" => {
-    :python => {
-      :driver => "ChromiumEdge",
+  'chromiumedge' => {
+    python: {
+      driver: 'ChromiumEdge'
     },
-    :java => {
-      :class => "org.openqa.selenium.edge.EdgeDriver",
-      :deps => [ "//java/client/src/org/openqa/selenium/edge:edge" ]
+    java: {
+      class: 'org.openqa.selenium.edge.EdgeDriver',
+      deps: ['//java/client/src/org/openqa/selenium/edge:edge']
     },
-    :browser_name => "MicrosoftEdge",
-    :available => edge?
+    browser_name: 'MicrosoftEdge',
+    available: edge?
   },
-  "blackberry" => {
-    :python => {
-      :driver => "BlackBerry",
+  'blackberry' => {
+    python: {
+      driver: 'BlackBerry'
     },
-    :browser_name => "blackberry"
+    browser_name: 'blackberry'
   },
-  "remote_firefox" => {
-    :python => {
-      :driver => "Remote",
-      :deps => [
+  'remote_firefox' => {
+    python: {
+      driver: 'Remote',
+      deps: [
         :remote_client,
         :'selenium-server-standalone',
         '//java/server/test/org/openqa/selenium/remote/server/auth:server'
-      ],
+      ]
     }
   },
-  "safari" => {
-    :python => {
-      :driver => "Safari",
+  'safari' => {
+    python: {
+      driver: 'Safari'
     },
-    :java => {
-      :class => "org.openqa.selenium.safari.SafariDriver",
-      :deps => [ "//java/client/src/org/openqa/selenium/safari:safari" ]
+    java: {
+      class: 'org.openqa.selenium.safari.SafariDriver',
+      deps: ['//java/client/src/org/openqa/selenium/safari:safari']
     },
-    :browser_name => "safari",
-    :available => mac?
+    browser_name: 'safari',
+    available: mac?
   }
 }
diff --git a/rake-tasks/c.rb b/rake-tasks/c.rb
@@ -1,37 +1,37 @@
 # C related tasks
 
-require "rake-tasks/files.rb"
+require 'rake-tasks/files.rb'
 
 def dll(args)
   deps = build_deps_(args[:deps])
-  
+
   Array(args[:out]).each do |result|
     out = "build/#{result}"
-  
+
     file out => build_deps_(args[:src]) + deps do
-      if (out =~ /\.dll$/)
+      if out =~ /\.dll$/
         msbuild_legacy(args[:solution], out, args[:prebuilt])
-      elsif (out =~ /\.so$/) 
-        is_32_bit = "amd64" != args[:arch]
+      elsif out =~ /\.so$/
+        is_32_bit = args[:arch] != 'amd64'
         gcc(args[:src], out, args[:args], args[:link_args], is_32_bit, args[:prebuilt])
       else
         puts "Cannot compile #{args[:out]}"
         exit -1
       end
     end
-    
+
     # TODO(simon): Yuck. Not Good Enough
-    task "#{args[:name]}" => out
+    task args[:name].to_s => out
     task args[:out] => out
-    Rake::Task[args[:name]].out = "#{out}"
+    Rake::Task[args[:name]].out = out.to_s
   end
 end
 
 def msbuild_legacy(solution, out, prebuilt)
   if msbuild_installed?
-    if (!File.exists? out) then
-      sh "MSBuild.exe #{solution} /verbosity:q /target:Rebuild /property:Configuration=Release /property:Platform=x64", :verbose => false
-      sh "MSBuild.exe #{solution} /verbosity:q /target:Rebuild /property:Configuration=Release /property:Platform=Win32", :verbose => false
+    unless File.exist? out
+      sh "MSBuild.exe #{solution} /verbosity:q /target:Rebuild /property:Configuration=Release /property:Platform=x64", verbose: false
+      sh "MSBuild.exe #{solution} /verbosity:q /target:Rebuild /property:Configuration=Release /property:Platform=Win32", verbose: false
       copy_to_prebuilt(out, prebuilt)
     end
   else
@@ -40,39 +40,36 @@ def msbuild_legacy(solution, out, prebuilt)
 end
 
 def gcc(srcs, out, args, link_args, is_32_bit, prebuilt)
-  if !gcc?
+  unless gcc?
     copy_prebuilt(prebuilt, out)
     return
   end
 
-  obj_dir = "#{out}_temp/obj" + (is_32_bit ? "32" : "64")
+  bitness = is_32_bit ? '32' : '64'
+  obj_dir = "#{out}_temp/obj#{bitness}"
 
   mkdir_p obj_dir
 
   is_cpp_code = false
   srcs.each do |src|
     ok = gccbuild_c(src, obj_dir, args, is_32_bit)
-    if (!ok)
+    unless ok
       copy_prebuilt(prebuilt, out)
       return
     end
-    if (src =~ /\.cpp$/)
-      is_cpp_code = true
-    end
+    is_cpp_code = true if src =~ /\.cpp$/
   end
 
-  flags = "-Wall -shared  -fPIC -Os -fshort-wchar "
-  flags += (is_32_bit ? "-m32 " : "-m64 ")
-  flags += " " + link_args + " " if link_args
+  flags = '-Wall -shared  -fPIC -Os -fshort-wchar '
+  flags += "-m#{bitness} "
+  flags += " #{link_args} " if link_args
 
-  # if we've made it this far, try to link. If link fails,
-  # copy from prebuilt.
-  linker = is_cpp_code ? "g++" : "gcc"
+  # if we've made it this far, try to link. If link fails, copy from prebuilt.
+  linker = is_cpp_code ? 'g++' : 'gcc'
   linker_cmd = "#{linker} -o #{out} #{obj_dir}/*.o #{flags}"
-  sh linker_cmd do |link_ok, res|
-    if (!link_ok)
-      copy_prebuilt(prebuilt, out)
-      return
+  sh linker_cmd do |link_ok, _res|
+    unless link_ok
+      return copy_prebuilt(prebuilt, out)
     end
   end
 
@@ -82,17 +79,14 @@ def gcc(srcs, out, args, link_args, is_32_bit, prebuilt)
 end
 
 def gccbuild_c(src_file, obj_dir, args, is_32_bit)
-  compiler = src_file =~ /\.c$/ ? "gcc" : "g++"
-  objname = src_file.split('/')[-1].sub(/\.c[p{2}]*?$/, ".o")
+  bitness = is_32_bit ? '32' : '64'
+  compiler = src_file =~ /\.c$/ ? 'gcc' : 'g++'
+  objname = src_file.split('/')[-1].sub(/\.c[p{2}]*?$/, '.o')
   cmd = "#{compiler} #{src_file} -Wall -c -fshort-wchar -fPIC -o #{obj_dir}/#{objname} "
-  cmd += (is_32_bit ? " -m32" : " -m64")
+  cmd += " -m#{bitness}"
   cmd += args if args
-  sh cmd do |ok, res|
-    if !ok
-      puts "Unable to build. Aborting compilation"
-      return false
-    end
+  sh cmd do |ok, _res|
+    return puts 'Unable to build. Aborting compilation' unless ok
   end
   true
 end
-
diff --git a/rake-tasks/checks.rb b/rake-tasks/checks.rb
@@ -3,28 +3,28 @@
 # Platform checks
 
 def windows?
-  (/mswin|msys|mingw32/ =~ RbConfig::CONFIG['host_os']) != nil
+  (RbConfig::CONFIG['host_os'] =~ /mswin|msys|mingw32/) != nil
 end
 
 def mac?
-  (/darwin|mac os/ =~ RbConfig::CONFIG['host_os']) != nil
+  (RbConfig::CONFIG['host_os'] =~ /darwin|mac os/) != nil
 end
 
 def linux?
-  (/linux/ =~ RbConfig::CONFIG['host_os']) != nil
+  (RbConfig::CONFIG['host_os'] =~ /linux/) != nil
 end
 
 def cygwin?
-  RUBY_PLATFORM.downcase.include?("cygwin")
+  RUBY_PLATFORM.downcase.include?('cygwin')
 end
 
 def unix?
-  linux? or mac?
+  linux? || mac?
 end
 
 def classpath_separator?
-  if cygwin? then
-    ";"
+  if cygwin?
+    ';'
   else
     File::PATH_SEPARATOR
   end
@@ -38,60 +38,56 @@ def all?
 
 # Checking for particular applications
 def present?(arg)
-  if PRESENT_CACHE.has_key?(arg)
-    return PRESENT_CACHE[arg]
-  end
+  return PRESENT_CACHE[arg] if PRESENT_CACHE.key?(arg)
 
   prefixes = ENV['PATH'].split(File::PATH_SEPARATOR)
 
   bool = prefixes.any? do |prefix|
-    File.exists?(prefix + File::SEPARATOR + arg)
+    File.exist?(prefix + File::SEPARATOR + arg)
   end
 
-  if !bool && mac?
-    bool = File.exists?("/Applications/#{arg}.app")
-  end
+  bool = File.exist?("/Applications/#{arg}.app") if !bool && mac?
 
   PRESENT_CACHE[arg] = bool
 
   bool
 end
 
 def chrome?
-  present?("chromedriver") || present?("chromedriver.exe")
+  present?('chromedriver') || present?('chromedriver.exe')
 end
 
 def edge?
-  present?("msedgedriver") || present?("msedgedriver.exe")
+  present?('msedgedriver') || present?('msedgedriver.exe')
 end
 
 def opera?
-  present?("opera") || present?("Opera")
+  present?('opera') || present?('Opera')
 end
 
 def java?
-  present?("java") || present?("java.exe")
+  present?('java') || present?('java.exe')
 end
 
 def javac?
-  present?("javac") || present?("javac.exe")
+  present?('javac') || present?('javac.exe')
 end
 
 def jar?
-  present?("jar") || present?("jar.exe")
+  present?('jar') || present?('jar.exe')
 end
 
 # Think of the confusion if we called this "g++"
 def gcc?
-  linux? && present?("g++")
+  linux? && present?('g++')
 end
 
 def python?
-  present?("python") || present?("python.exe")
+  present?('python') || present?('python.exe')
 end
 
 def msbuild_installed?
-  windows? && present?("msbuild.exe")
+  windows? && present?('msbuild.exe')
 end
 
 def vcs_revision