feat: raise its own defined errors (#1035)

* feat: raise an error when no driver instances were created in some cases

* raise Appium::ArgumentError error

* use ArgumentError

Author: KazuCocoa

lib/appium_lib/android/common/helper.rb

@@ -150,7 +150,7 @@ def ele_index(class_name, index)
         index = results.length
         index -= 1 if index >= 0
       else
-        raise 'Index must be >= 1' unless index >= 1
+        raise ArgumentError, 'Index must be >= 1' unless index >= 1
 
         index -= 1 if index >= 1
       end

lib/appium_lib/android/element/button.rb

@@ -29,7 +29,7 @@ def button(value)
       # Android needs to combine button and image button to match iOS.
       if value.is_a? Numeric
         index = value
-        raise "#{index} is not a valid index. Must be >= 1" if index <= 0
+        raise ArgumentError, "#{index} is not a valid index. Must be >= 1" if index <= 0
 
         # 1 indexed
         return find_element :uiautomator, _button_visible_selectors(index: index)

lib/appium_lib/android/espresso/element/button.rb

@@ -25,7 +25,7 @@ def button(value)
           # Android needs to combine button and image button to match iOS.
           if value.is_a? Numeric
             index = value
-            raise "#{index} is not a valid index. Must be >= 1" if index <= 0
+            raise ArgumentError, "#{index} is not a valid index. Must be >= 1" if index <= 0
 
             # zero index
             _button_visible_selectors_xpath(index: index - 1)

lib/appium_lib/android/uiautomator2/element/button.rb

@@ -25,7 +25,7 @@ def button(value)
           # Android needs to combine button and image button to match iOS.
           if value.is_a? Numeric
             index = value
-            raise "#{index} is not a valid index. Must be >= 1" if index <= 0
+            raise ArgumentError, "#{index} is not a valid index. Must be >= 1" if index <= 0
 
             result = find_elements :uiautomator, _button_visible_selectors(index: index)
             raise _no_such_element if result.empty?

lib/appium_lib/appium.rb

@@ -56,11 +56,11 @@ class << self
     # @param opts [Hash] file: '/path/to/appium.txt', verbose: true
     # @return [hash] the symbolized hash with updated :app and :require keys
     def load_settings(opts = {})
-      raise 'opts must be a hash' unless opts.is_a? Hash
-      raise 'opts must not be empty' if opts.empty?
+      raise ArgumentError, 'opts must be a hash' unless opts.is_a? Hash
+      raise ArgumentError, 'opts must not be empty' if opts.empty?
 
       toml = opts[:file]
-      raise 'Must pass a capability file which has [caps] and [appium_lib]' unless toml
+      raise ArgumentError, 'Must pass a capability file which has [caps] and [appium_lib]' unless toml
 
       verbose = opts.fetch :verbose, false
 
@@ -69,7 +69,7 @@ def load_settings(opts = {})
       toml_exists = File.exist? toml
       Appium::Logger.info "Exists? #{toml_exists}" if verbose
 
-      raise "toml doesn't exist #{toml}" unless toml_exists
+      raise ArgumentError, "toml doesn't exist #{toml}" unless toml_exists
 
       require 'tomlrb'
       Appium::Logger.info "Loading #{toml}" if verbose
@@ -151,7 +151,7 @@ def expand_required_files(base_dir, file_paths)
     # @param [Array<Module>] modules An array of modules
     # @param [Driver] driver A driver to extend for
     def promote_singleton_appium_methods(modules, driver = $driver)
-      raise 'Global $driver is nil' if driver.nil?
+      raise ArgumentError, 'Global $driver is nil' if driver.nil?
 
       target_modules = []
 
@@ -160,7 +160,7 @@ def promote_singleton_appium_methods(modules, driver = $driver)
           target_modules << modules.const_get(sub_module)
         end
       else
-        raise 'modules must be a module or an array' unless modules.is_a? Array
+        raise ArgumentError, 'modules must be a module or an array' unless modules.is_a? Array
 
         target_modules = modules
       end
@@ -205,7 +205,7 @@ def promote_singleton_appium_methods(modules, driver = $driver)
     #   Appium.promote_appium_methods Minitest::Spec
     #
     def promote_appium_methods(class_array, driver = $driver)
-      raise 'Driver is nil' if driver.nil?
+      raise ArgumentError, 'Driver is nil' if driver.nil?
 
       # Wrap single class into an array
       class_array = [class_array] unless class_array.instance_of? Array

lib/appium_lib/driver.rb

@@ -156,7 +156,7 @@ def initialize(opts = {}, global_driver = false)
 
       $driver&.driver_quit if global_driver
 
-      raise 'opts must be a hash' unless opts.is_a? Hash
+      raise ArgumentError, 'opts must be a hash' unless opts.is_a? Hash
 
       @core = ::Appium::Core.for(opts)
       extend ::Appium::Core::Device
@@ -329,7 +329,7 @@ def automation_name_is_xcuitest?
     #     action.click(element).perform # The `click` is a part of `PointerActions`
     #
     def action
-      @driver.action
+      @driver&.action
     end
 
     # Returns the server's version info
@@ -384,12 +384,12 @@ def appium_client_version
     #
     # @return [String] APP_PATH as an absolute path
     def self.absolute_app_path(opts)
-      raise 'opts must be a hash' unless opts.is_a? Hash
+      raise ArgumentError, 'opts must be a hash' unless opts.is_a? Hash
 
       # FIXME: 'caps' and 'app' will be correct
       caps            = opts[:caps] || opts['caps'] || {}
       app_path        = caps[:app] || caps['app']
-      raise 'absolute_app_path invoked and app is not set!' if app_path.nil? || app_path.empty?
+      raise ArgumentError, 'absolute_app_path invoked and app is not set!' if app_path.nil? || app_path.empty?
       # Sauce storage API. http://saucelabs.com/docs/rest#storage
       return app_path if app_path.start_with? 'sauce-storage:'
       return app_path if app_path =~ URI::DEFAULT_PARSER.make_regexp # public URL for Sauce
@@ -431,7 +431,7 @@ def restart
     # @param png_save_path [String] the full path to save the png
     # @return [File]
     def screenshot(png_save_path)
-      @driver.save_screenshot png_save_path
+      @driver&.save_screenshot png_save_path
     end
 
     # Takes a png screenshot of particular element's area
@@ -445,7 +445,7 @@ def screenshot(png_save_path)
     # @param [String] png_save_path the full path to save the png
     # @return [File]
     def element_screenshot(element, png_save_path)
-      @driver.take_element_screenshot element, png_save_path
+      @driver&.take_element_screenshot element, png_save_path
       nil
     end
 
@@ -469,6 +469,9 @@ def driver_quit
     #   size.height #=> Integer
     #
     def window_size
+      # maybe exception is expected as no driver created
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.window_size
     end
 
@@ -484,6 +487,8 @@ def window_size
     #   size.y #=> Integer
     #
     def window_rect
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.window_rect
     end
 
@@ -555,7 +560,7 @@ def set_implicit_wait(wait)
 
     # Set implicit wait to zero.
     def no_wait
-      @driver.manage.timeouts.implicit_wait = 0
+      @driver&.manage&.timeouts&.implicit_wait = 0
     end
 
     # Set implicit wait. Default to @default_wait.
@@ -570,7 +575,7 @@ def no_wait
     # @return [void]
     def set_wait(timeout = nil)
       timeout = @default_wait if timeout.nil?
-      @driver.manage.timeouts.implicit_wait = timeout
+      @driver&.manage&.timeouts&.implicit_wait = timeout
     end
 
     # Returns existence of element.
@@ -589,9 +594,9 @@ def exists(pre_check = 0, post_check = @default_wait)
       # do not uset set_wait here.
       # it will cause problems with other methods reading the default_wait of 0
       # which then gets converted to a 1 second wait.
-      @driver.manage.timeouts.implicit_wait = pre_check
+      @driver&.manage&.timeouts&.implicit_wait = pre_check
       # the element exists unless an error is raised.
-      exists                                = true
+      exists = true
 
       begin
         yield # search for element
@@ -600,7 +605,7 @@ def exists(pre_check = 0, post_check = @default_wait)
       end
 
       # restore wait
-      @driver.manage.timeouts.implicit_wait = post_check if post_check != pre_check
+      @driver&.manage&.timeouts&.implicit_wait = post_check if post_check != pre_check
 
       exists
     end
@@ -610,6 +615,8 @@ def exists(pre_check = 0, post_check = @default_wait)
     # @param [*args] args The args to pass to the script
     # @return [Object]
     def execute_script(script, *args)
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.execute_script script, *args
     end
 
@@ -618,6 +625,8 @@ def execute_script(script, *args)
     ###
     # Get the window handles of open browser windows
     def execute_async_script(script, *args)
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.execute_async_script script, *args
     end
 
@@ -650,41 +659,59 @@ def execute_async_script(script, *args)
     #      r.logs   #=> The `logs` key part as `{'log' => [], 'warn' => [], 'error' => []}`
     #
     def execute_driver(script: '', type: 'webdriverio', timeout_ms: nil)
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.execute_driver(script: script, type: type, timeout_ms: timeout_ms)
     end
 
     def window_handles
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.window_handles
     end
 
     # Get the current window handle
     def window_handle
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.window_handle
     end
 
     def navigate
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.navigate
     end
 
     def manage
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.manage
     end
 
     def get(url)
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.get(url)
     end
 
     def current_url
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.current_url
     end
 
     def title
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.title
     end
 
     # @return [TargetLocator]
     # @see TargetLocator
     def switch_to
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.switch_to
     end
     ###
@@ -712,6 +739,8 @@ def switch_to
     # @param [*args] args The args to use
     # @return [Array<Element>] Array is empty when no elements are found.
     def find_elements(*args)
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.find_elements(*args)
     end
 
@@ -728,6 +757,8 @@ def find_elements(*args)
     # @param [*args] args The args to use
     # @return [Element]
     def find_element(*args)
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.find_element(*args)
     end
 
@@ -743,6 +774,8 @@ def find_element(*args)
     #     @driver.find_element_by_image './test/functional/data/test_element_image.png'
     #
     def find_element_by_image(png_img_path)
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.find_element_by_image(png_img_path)
     end
 
@@ -758,6 +791,8 @@ def find_element_by_image(png_img_path)
     #     @driver.find_elements_by_image ['./test/functional/data/test_element_image.png']
     #
     def find_elements_by_image(png_img_paths)
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.find_elements_by_image(png_img_paths)
     end
 
@@ -771,6 +806,8 @@ def find_elements_by_image(png_img_paths)
     # @option opts [Float] :altitude the altitude, defaulting to 75
     # @return [Selenium::WebDriver::Location] the location constructed by the selenium webdriver
     def set_location(opts = {})
+      raise NoDriverInstanceError if @driver.nil?
+
       latitude = opts.fetch(:latitude)
       longitude = opts.fetch(:longitude)
       altitude = opts.fetch(:altitude, 75)
@@ -794,10 +831,13 @@ def set_location(opts = {})
     #              #          'appium:anotherEvent' => 1572959315}
     #
     def log_event(vendor:, event:)
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.logs.event vendor: vendor, event: event
     end
 
     def log_event=(log_event)
+      raise if @driver.nil?
       unless log_event.is_a?(Hash)
         raise ::Appium::Core::Error::ArgumentError('log_event should be Hash like { vendor: "appium", event: "funEvent"}')
       end
@@ -817,6 +857,8 @@ def log_event=(log_event)
     #   log_events #=> {'commands' => [{'cmd' => 123455, ....}], 'startTime' => 1572954894127, }
     #
     def log_events(type = nil)
+      raise NoDriverInstanceError if @driver.nil?
+
       @driver.logs.events(type)
     end
 

lib/appium_lib/error.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+module Appium
+  class Error < StandardError; end
+
+  # Driver instance hasn't been created yet.
+  class NoDriverInstanceError < Appium::Error; end
+
+  class ArgumentError < ArgumentError; end
+end