From 968daae6029130eb4a1f6b7b53b849c3bd29ff56 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20Gonz=C3=A1lez=20Alonso?= <rgonalo@gmail.com>
Date: Wed, 22 Sep 2021 08:34:20 +0200
Subject: [PATCH] fix chrome options configuration from capabilities (#241)

---
 CHANGELOG.rst                      |  2 +
 docs/browser_configuration.rst     | 25 ++++++++
 toolium/config_driver.py           | 52 ++++++++++++----
 toolium/test/test_config_driver.py | 99 ++++++++++++++++++++++++++++--
 4 files changed, 159 insertions(+), 19 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 8f0bc80c..84c104af 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -7,6 +7,8 @@ v2.1.1
 *Release date: In development*
 
 - Avoid to overwrite parent in group elements when a custom parent is defined
+- Fix Chrome options to allow to configure them at the same time in *Chrome* sections and in *goog:chromeOptions*
+  capability
 
 v2.1.0
 ------
diff --git a/docs/browser_configuration.rst b/docs/browser_configuration.rst
index 37b0990b..7cde5d01 100644
--- a/docs/browser_configuration.rst
+++ b/docs/browser_configuration.rst
@@ -280,3 +280,28 @@ replacing ':' with '___' in the key name::
     [Capabilities]
     goog___loggingPrefs: {'performance': 'ALL', 'browser': 'ALL', 'driver': 'ALL'}
     goog___chromeOptions: {'excludeSwitches': ['enable-automation'], 'useAutomationExtension': False}
+
+In fact, all the settings in above Chrome sections can be also configured in *goog___chromeOptions*:
+
+- Chrome section::
+
+    [Capabilities]
+    goog___chromeOptions: {'binary': '/usr/local/chrome_beta/chrome'}
+
+- ChromePreferences section::
+
+    [Capabilities]
+    goog___chromeOptions: {'prefs': {'download.default_directory': 'C:\tmp'}}
+
+- ChromeArguments section::
+
+    [Capabilities]
+    goog___chromeOptions: {'args': {'user-data-dir': 'C:\Users\USERNAME\AppData\Local\Google\Chrome\User Data'}}
+
+- ChromeMobileEmulation section::
+
+    [Capabilities]
+    goog___chromeOptions: {'mobileEmulation': {'deviceName': 'Google Nexus 5'}}
+
+In case of defining the same setting both in its dedicated section and in the capabilities section, the dedicated
+section value will be used.
diff --git a/toolium/config_driver.py b/toolium/config_driver.py
index b025c97e..00f1df17 100644
--- a/toolium/config_driver.py
+++ b/toolium/config_driver.py
@@ -19,11 +19,12 @@
 import ast
 import logging
 import os
-from appium import webdriver as appiumdriver
 from configparser import NoSectionError
+
+from appium import webdriver as appiumdriver
 from selenium import webdriver
 from selenium.webdriver.common.desired_capabilities import DesiredCapabilities
-from selenium.webdriver.firefox.options import Options
+from selenium.webdriver.firefox.options import Options as FirefoxOptions
 
 from toolium.driver_wrappers_pool import DriverWrappersPool
 
@@ -88,17 +89,13 @@ def _create_remote_driver(self):
             capabilities['opera.arguments'] = '-fullscreen'
         elif driver_name == 'firefox':
             capabilities['firefox_profile'] = self._create_firefox_profile().encoded
-        elif driver_name == 'chrome':
-            chrome_capabilities = self._create_chrome_options().to_capabilities()
-            try:
-                capabilities['goog:chromeOptions'] = chrome_capabilities["goog:chromeOptions"]
-            except KeyError:
-                # Selenium 3.5.3 and older
-                capabilities['chromeOptions'] = chrome_capabilities["chromeOptions"]
 
         # Add custom driver capabilities
         self._add_capabilities_from_properties(capabilities, 'Capabilities')
 
+        if driver_name == 'chrome':
+            self._add_chrome_options_to_capabilities(capabilities)
+
         if driver_name in ('android', 'ios', 'iphone'):
             # Create remote appium driver
             self._add_capabilities_from_properties(capabilities, 'AppiumCapabilities')
@@ -199,7 +196,8 @@ def _add_capabilities_from_properties(self, capabilities, section):
         try:
             for cap, cap_value in dict(self.config.items(section)).items():
                 self.logger.debug("Added %s capability: %s = %s", cap_type[section], cap, cap_value)
-                capabilities[cap] = cap_value if cap == 'version' else self._convert_property_type(cap_value)
+                cap_value = cap_value if cap == 'version' else self._convert_property_type(cap_value)
+                self._update_dict(capabilities, {cap: cap_value}, initial_key=cap)
         except NoSectionError:
             pass
 
@@ -218,7 +216,7 @@ def _setup_firefox(self, capabilities):
         # Get Firefox binary
         firefox_binary = self.config.get_optional('Firefox', 'binary')
 
-        firefox_options = Options()
+        firefox_options = FirefoxOptions()
 
         if self.config.getboolean_optional('Driver', 'headless'):
             self.logger.debug("Running Firefox in headless mode")
@@ -314,8 +312,8 @@ def _setup_chrome(self, capabilities):
         """
         chrome_driver = self.config.get('Driver', 'chrome_driver_path')
         self.logger.debug("Chrome driver path given in properties: %s", chrome_driver)
-        return webdriver.Chrome(chrome_driver, chrome_options=self._create_chrome_options(),
-                                desired_capabilities=capabilities)
+        self._add_chrome_options_to_capabilities(capabilities)
+        return webdriver.Chrome(chrome_driver, desired_capabilities=capabilities)
 
     def _create_chrome_options(self):
         """Create and configure a chrome options object
@@ -375,6 +373,34 @@ def _add_chrome_arguments(self, options):
         except NoSectionError:
             pass
 
+    def _add_chrome_options_to_capabilities(self, capabilities):
+        """Add Chrome options to capabilities
+
+        :param capabilities: dictionary with driver capabilities
+        """
+        chrome_capabilities = self._create_chrome_options().to_capabilities()
+        options_key = None
+        if 'goog:chromeOptions' in chrome_capabilities:
+            options_key = 'goog:chromeOptions'
+        elif 'chromeOptions' in chrome_capabilities:
+            # Selenium 3.5.3 and older
+            options_key = 'chromeOptions'
+        if options_key:
+            self._update_dict(capabilities, chrome_capabilities, initial_key=options_key)
+
+    def _update_dict(self, initial, update, initial_key=None):
+        """ Update a initial dict with another dict values recursively
+
+        :param initial: initial dict to be updated
+        :param update: new dict
+        :param initial_key: update only one key in initial dicts
+        :return: merged dict
+        """
+        for key, value in update.items():
+            if initial_key is None or key == initial_key:
+                initial[key] = self._update_dict(initial.get(key, {}), value) if isinstance(value, dict) else value
+        return initial
+
     def _setup_safari(self, capabilities):
         """Setup Safari webdriver
 
diff --git a/toolium/test/test_config_driver.py b/toolium/test/test_config_driver.py
index 61bd195f..43d6d905 100644
--- a/toolium/test/test_config_driver.py
+++ b/toolium/test/test_config_driver.py
@@ -16,8 +16,9 @@
 limitations under the License.
 """
 
-import mock
 import os
+
+import mock
 import pytest
 from selenium.webdriver.common.desired_capabilities import DesiredCapabilities
 from selenium.webdriver.firefox.options import Options
@@ -80,7 +81,7 @@ def test_create_driver_remote(config, utils):
     assert driver == 'remote driver mock'
 
 
-@mock.patch('toolium.config_driver.Options')
+@mock.patch('toolium.config_driver.FirefoxOptions')
 @mock.patch('toolium.config_driver.webdriver')
 def test_create_local_driver_firefox(webdriver_mock, options, config, utils):
     config.set('Driver', 'type', 'firefox')
@@ -99,7 +100,7 @@ def test_create_local_driver_firefox(webdriver_mock, options, config, utils):
                                                    firefox_options=options(), log_path='geckodriver.log')
 
 
-@mock.patch('toolium.config_driver.Options')
+@mock.patch('toolium.config_driver.FirefoxOptions')
 @mock.patch('toolium.config_driver.webdriver')
 def test_create_local_driver_firefox_gecko(webdriver_mock, options, config, utils):
     config.set('Driver', 'type', 'firefox')
@@ -150,10 +151,52 @@ def test_create_local_driver_chrome(webdriver_mock, config, utils):
     utils.get_driver_name.return_value = 'chrome'
     config_driver = ConfigDriver(config, utils)
     config_driver._create_chrome_options = lambda: 'chrome options'
+    config_driver._add_chrome_options_to_capabilities = lambda x: None
 
     config_driver._create_local_driver()
-    webdriver_mock.Chrome.assert_called_once_with('/tmp/driver', desired_capabilities=DesiredCapabilities.CHROME,
-                                                  chrome_options='chrome options')
+    webdriver_mock.Chrome.assert_called_once_with('/tmp/driver', desired_capabilities=DesiredCapabilities.CHROME)
+
+
+@mock.patch('toolium.config_driver.webdriver')
+def test_create_local_driver_chrome_multiple_options(webdriver_mock, config, utils):
+    # From goog:chromeOptions in Capabilities section
+    options_from_capabilities = {
+        'excludeSwitches': ['enable-automation'], 'useAutomationExtension': False,
+        'prefs': {'download.default_directory': '/this_value_will_be_overwritten',
+                  'download.prompt_for_download': False}
+    }
+    # From ChromePreferences, ChromeMobileEmulation, ChromeArguments and Chrome sections
+    options_from_sections = {
+        'prefs': {'download.default_directory': '/tmp'},
+        'mobileEmulation': {'deviceName': 'Google Nexus 5'},
+        'args': ['user-data-dir=C:\\Users\\USERNAME\\AppData\\Local\\Google\\Chrome\\User Data'],
+        'binary': '/usr/local/chrome_beta/chrome'
+    }
+    # Merged chrome options
+    final_chrome_options = {
+        'excludeSwitches': ['enable-automation'], 'useAutomationExtension': False,
+        'prefs': {'download.default_directory': '/tmp', 'download.prompt_for_download': False},
+        'mobileEmulation': {'deviceName': 'Google Nexus 5'},
+        'args': ['user-data-dir=C:\\Users\\USERNAME\\AppData\\Local\\Google\\Chrome\\User Data'],
+        'binary': '/usr/local/chrome_beta/chrome'
+    }
+
+    config.set('Driver', 'type', 'chrome')
+    config.set('Driver', 'chrome_driver_path', '/tmp/driver')
+    config.add_section('Capabilities')
+    config.set('Capabilities', 'goog:chromeOptions', str(options_from_capabilities))
+    utils.get_driver_name.return_value = 'chrome'
+    config_driver = ConfigDriver(config, utils)
+
+    # Chrome options mock
+    chrome_options = mock.MagicMock()
+    chrome_options.to_capabilities.return_value = {'goog:chromeOptions': options_from_sections}
+    config_driver._create_chrome_options = mock.MagicMock(return_value=chrome_options)
+
+    config_driver._create_local_driver()
+    capabilities = DesiredCapabilities.CHROME.copy()
+    capabilities['goog:chromeOptions'] = final_chrome_options
+    webdriver_mock.Chrome.assert_called_once_with('/tmp/driver', desired_capabilities=capabilities)
 
 
 @mock.patch('toolium.config_driver.webdriver')
@@ -252,7 +295,7 @@ def test_create_local_driver_unknown_driver(config, utils):
     assert 'Unknown driver unknown' == str(excinfo.value)
 
 
-@mock.patch('toolium.config_driver.Options')
+@mock.patch('toolium.config_driver.FirefoxOptions')
 @mock.patch('toolium.config_driver.webdriver')
 def test_create_local_driver_capabilities(webdriver_mock, options, config, utils):
     config.set('Driver', 'type', 'firefox')
@@ -314,6 +357,50 @@ def test_create_remote_driver_chrome(webdriver_mock, config, utils):
                                                   desired_capabilities=capabilities)
 
 
+@mock.patch('toolium.config_driver.webdriver')
+def test_create_remote_driver_chrome_multiple_options(webdriver_mock, config, utils):
+    # From goog:chromeOptions in Capabilities section
+    options_from_capabilities = {
+        'excludeSwitches': ['enable-automation'], 'useAutomationExtension': False,
+        'prefs': {'download.default_directory': '/this_value_will_be_overwritten',
+                  'download.prompt_for_download': False}
+    }
+    # From ChromePreferences, ChromeMobileEmulation, ChromeArguments and Chrome sections
+    options_from_sections = {
+        'prefs': {'download.default_directory': '/tmp'},
+        'mobileEmulation': {'deviceName': 'Google Nexus 5'},
+        'args': ['user-data-dir=C:\\Users\\USERNAME\\AppData\\Local\\Google\\Chrome\\User Data'],
+        'binary': '/usr/local/chrome_beta/chrome'
+    }
+    # Merged chrome options
+    final_chrome_options = {
+        'excludeSwitches': ['enable-automation'], 'useAutomationExtension': False,
+        'prefs': {'download.default_directory': '/tmp', 'download.prompt_for_download': False},
+        'mobileEmulation': {'deviceName': 'Google Nexus 5'},
+        'args': ['user-data-dir=C:\\Users\\USERNAME\\AppData\\Local\\Google\\Chrome\\User Data'],
+        'binary': '/usr/local/chrome_beta/chrome'
+    }
+
+    config.set('Driver', 'type', 'chrome')
+    config.add_section('Capabilities')
+    config.set('Capabilities', 'goog:chromeOptions', str(options_from_capabilities))
+    server_url = 'http://10.20.30.40:5555'
+    utils.get_server_url.return_value = server_url
+    utils.get_driver_name.return_value = 'chrome'
+    config_driver = ConfigDriver(config, utils)
+
+    # Chrome options mock
+    chrome_options = mock.MagicMock()
+    chrome_options.to_capabilities.return_value = {'goog:chromeOptions': options_from_sections}
+    config_driver._create_chrome_options = mock.MagicMock(return_value=chrome_options)
+
+    config_driver._create_remote_driver()
+    capabilities = DesiredCapabilities.CHROME.copy()
+    capabilities['goog:chromeOptions'] = final_chrome_options
+    webdriver_mock.Remote.assert_called_once_with(command_executor='%s/wd/hub' % server_url,
+                                                  desired_capabilities=capabilities)
+
+
 @mock.patch('toolium.config_driver.webdriver')
 def test_create_remote_driver_chrome_old_selenium(webdriver_mock, config, utils):
     config.set('Driver', 'type', 'chrome')