Repository: robotframework/SeleniumLibrary
Pull request #1470 (2019-09-30 16:32:45)

Optional URL in Open Browser
Makes the URL in the `Open Browser` keyword optional. This means that you can just open a browser without immediately navigating to a URL. In response to issue #1464 

JonKoser: Added a unit test where `Open Browser` is called without any arguments. No .robot acceptance tests added since I noticed that there were no other default value `Open Browser` tests in that suite.

Patch:
diff --git a/src/SeleniumLibrary/keywords/browsermanagement.py b/src/SeleniumLibrary/keywords/browsermanagement.py
index 68cb7f29..f8418af2 100644
--- a/src/SeleniumLibrary/keywords/browsermanagement.py
+++ b/src/SeleniumLibrary/keywords/browsermanagement.py
@@ -56,10 +56,10 @@ class BrowserManagementKeywords(LibraryComponent):
             self.drivers.close()
 
     @keyword
-    def open_browser(self, url, browser='firefox', alias=None,
+    def open_browser(self, url=None, browser='firefox', alias=None,
                      remote_url=False, desired_capabilities=None,
                      ff_profile_dir=None, options=None, service_log_path=None):
-        """Opens a new browser instance to the given ``url``.
+        """Opens a new browser instance to the optional ``url``.
 
         The ``browser`` argument specifies which browser to use. The
         supported browsers are listed in the table below. The browser names
@@ -87,6 +87,9 @@ class BrowserManagementKeywords(LibraryComponent):
         Headless Chrome are new additions in SeleniumLibrary 3.1.0
         and require Selenium 3.8.0 or newer.
 
+        After opening the browser, it is possible to use optional 
+        ``url`` to navigate the browser to the desired address.
+
         Optional ``alias`` is an alias given for this browser instance and
         it can be used for switching between browsers. When same ``alias``
         is given with two `Open Browser` keywords, the first keyword will
@@ -189,6 +192,15 @@ class BrowserManagementKeywords(LibraryComponent):
         Then the `${options}` variable can be used as an argument to
         ``options``.
 
+        Example the ``options`` argument can be used to launch Chomium-based
+        applications which utilize the 
+        [https://bitbucket.org/chromiumembedded/cef/wiki/UsingChromeDriver|Chromium Embedded Framework]
+        . To lauch Chomium-based application, use ``options`` to define
+        `binary_location` attribute and use `add_argument` method to define
+        `remote-debugging-port` port for the application. Once the browser
+        is opened, the test can interact with the embedded web-content of
+        the system under test.
+
         Optional ``service_log_path`` argument defines the name of the
         file where to write the browser driver logs. If the
         ``service_log_path``  argument contain a  marker ``{index}``, it
@@ -203,6 +215,7 @@ class BrowserManagementKeywords(LibraryComponent):
         | `Open Browser` | http://example.com | Firefox | alias=Firefox                           |
         | `Open Browser` | http://example.com | Edge    | remote_url=http://127.0.0.1:4444/wd/hub |
         | `Open Browser` | about:blank        |         |                                         |
+        | `Open Browser` | browser=Chrome     |         |                                         |
 
         Alias examples:
         | ${1_index} =    | `Open Browser` | http://example.com | Chrome  | alias=Chrome     | # Opens new browser because alias is new.         |
@@ -216,9 +229,10 @@ class BrowserManagementKeywords(LibraryComponent):
         Example when using
         [https://seleniumhq.github.io/selenium/docs/api/py/webdriver_chrome/selenium.webdriver.chrome.options.html#selenium.webdriver.chrome.options.Options|Chrome options]
         method:
-        | `Open Browser` | http://example.com | Chrome | options=add_argument("--disable-popup-blocking"); add_argument("--ignore-certificate-errors") | # Sting format              |
-        |  ${options} =  |     Get Options    |        |                                                                                               | # Selenium options instance |
-        | `Open Browser` | http://example.com | Chrome | options=${options}                                                                            |                             |
+        | `Open Browser` | http://example.com | Chrome | options=add_argument("--disable-popup-blocking"); add_argument("--ignore-certificate-errors") | # Sting format                    |
+        |  ${options} =  |     Get Options    |        |                                                                                               | # Selenium options instance       |
+        | `Open Browser` | http://example.com | Chrome | options=${options}                                                                            |                                   |
+        | `Open Browser` | None               | Chrome | options=binary_location="/path/to/binary";add_argument("remote-debugging-port=port")          | # Start Chomium-based application |
 
         Example for FirefoxProfile
         | `Open Browser` | http://example.com | Firefox | ff_profile_dir=/path/to/profile                                                  | # Using profile from disk                       |
@@ -238,18 +252,21 @@ class BrowserManagementKeywords(LibraryComponent):
         accepting an instance of the `selenium.webdriver.FirefoxProfile`
         and support defining FirefoxProfile with methods and
         attributes are new in SeleniumLibrary 4.0.
+        
+        Making ``url`` optional is new in SeleniumLibrary 4.1.
         """
         index = self.drivers.get_index(alias)
         if index:
             self.info('Using existing browser from index %s.' % index)
             self.switch_browser(alias)
-            self.go_to(url)
+            if is_truthy(url):
+                self.go_to(url)
             return index
         return self._make_new_browser(url, browser, alias, remote_url,
                                       desired_capabilities, ff_profile_dir,
                                       options, service_log_path)
 
-    def _make_new_browser(self, url, browser='firefox', alias=None,
+    def _make_new_browser(self, url=None, browser='firefox', alias=None,
                           remote_url=False, desired_capabilities=None,
                           ff_profile_dir=None, options=None, service_log_path=None):
         if is_truthy(remote_url):
@@ -262,12 +279,13 @@ class BrowserManagementKeywords(LibraryComponent):
                                    options, service_log_path)
         driver = self._wrap_event_firing_webdriver(driver)
         index = self.ctx.register_driver(driver, alias)
-        try:
-            driver.get(url)
-        except Exception:
-            self.debug("Opened browser with session id %s but failed "
-                       "to open url '%s'." % (driver.session_id, url))
-            raise
+        if is_truthy(url):
+            try:
+                driver.get(url)
+            except Exception:
+                self.debug("Opened browser with session id %s but failed "
+                        "to open url '%s'." % (driver.session_id, url))
+                raise
         self.debug('Opened browser with session id %s.' % driver.session_id)
         return index
 
diff --git a/src/SeleniumLibrary/keywords/screenshot.py b/src/SeleniumLibrary/keywords/screenshot.py
index e5fb6bff..daec0e2e 100644
--- a/src/SeleniumLibrary/keywords/screenshot.py
+++ b/src/SeleniumLibrary/keywords/screenshot.py
@@ -20,6 +20,7 @@ from robot.utils import get_link_path
 
 from SeleniumLibrary.base import LibraryComponent, keyword
 from SeleniumLibrary.utils import is_noney
+from SeleniumLibrary.utils.path_formatter import _format_path
 
 
 class ScreenshotKeywords(LibraryComponent):
@@ -130,7 +131,7 @@ class ScreenshotKeywords(LibraryComponent):
         index = 0
         while True:
             index += 1
-            formatted = filename.format(index=index)
+            formatted = _format_path(filename, index)
             path = os.path.join(directory, formatted)
             # filename didn't contain {index} or unique path was found
             if formatted == filename or not os.path.exists(path):
diff --git a/src/SeleniumLibrary/keywords/webdrivertools/webdrivertools.py b/src/SeleniumLibrary/keywords/webdrivertools/webdrivertools.py
index 20ac4f46..3482cdde 100644
--- a/src/SeleniumLibrary/keywords/webdrivertools/webdrivertools.py
+++ b/src/SeleniumLibrary/keywords/webdrivertools/webdrivertools.py
@@ -28,6 +28,8 @@ from selenium.webdriver import FirefoxProfile
 
 from SeleniumLibrary.utils import is_falsy, is_truthy, is_noney, is_string, PY3
 from SeleniumLibrary.keywords.webdrivertools.sl_file_detector import SelLibLocalFileDetector
+from SeleniumLibrary.utils.path_formatter import _format_path
+
 if not PY3:
     FileNotFoundError = object
 
@@ -285,7 +287,7 @@ class WebDriverCreator(object):
             return None
         index = 1
         while True:
-            formatted = log_file.format(index=index)
+            formatted = _format_path(log_file, index)
             path = os.path.join(self.log_dir, formatted)
             # filename didn't contain {index} or unique path was found
             if formatted == log_file or not os.path.exists(path):
diff --git a/src/SeleniumLibrary/utils/path_formatter.py b/src/SeleniumLibrary/utils/path_formatter.py
new file mode 100644
index 00000000..f478cdad
--- /dev/null
+++ b/src/SeleniumLibrary/utils/path_formatter.py
@@ -0,0 +1,30 @@
+# Copyright 2008-2011 Nokia Networks
+# Copyright 2011-2016 Ryan Tomac, Ed Manlove and contributors
+# Copyright 2016-     Robot Framework Foundation
+#
+# 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.
+from SeleniumLibrary.base.robotlibcore import PY2
+
+
+# Cab be deleted when Python 2 is not anymore supported.
+def _format_path(file_path, index):
+    if PY2:
+        import string
+        return string.Formatter().vformat(file_path, (), _SafeFormatter(index=index))
+    return file_path.format_map(_SafeFormatter(index=index))
+
+
+class _SafeFormatter(dict):
+
+    def __missing__(self, key):
+        return '{%s}' % key


Test patch:
diff --git a/atest/acceptance/keywords/screenshots.robot b/atest/acceptance/keywords/screenshots.robot
index 6f5a0fcd..f8d5e32f 100644
--- a/atest/acceptance/keywords/screenshots.robot
+++ b/atest/acceptance/keywords/screenshots.robot
@@ -53,12 +53,12 @@ Set Screenshot Directory set back to previous value
 Capture page screenshot with unique index
     [Setup]    Remove Directory    ${OUTPUTDIR}/screenshot-and-index    recursive
     ${file1} =    Capture Page Screenshot    ${OUTPUTDIR}/screenshot-and-index/other-{index}-name.png
-    ${file2} =    Capture Page Screenshot    ${OUTPUTDIR}/screenshot-and-index/some-other-name-{index}.png
+    ${file2} =    Capture Page Screenshot    ${OUTPUTDIR}/screenshot-and-index/some-{other}-name-{index}.png
     ${file3} =    Capture Page Screenshot    ${OUTPUTDIR}/screenshot-and-index/other-{index}-name.png
     File Should Exist    ${OUTPUTDIR}/screenshot-and-index/other-1-name.png
     Should Be Equal    ${file1}    ${OUTPUTDIR}${/}screenshot-and-index${/}other-1-name.png
-    File Should Exist    ${OUTPUTDIR}/screenshot-and-index/some-other-name-1.png
-    Should Be Equal    ${file2}    ${OUTPUTDIR}${/}screenshot-and-index${/}some-other-name-1.png
+    File Should Exist    ${OUTPUTDIR}/screenshot-and-index/some-{other}-name-1.png
+    Should Be Equal    ${file2}    ${OUTPUTDIR}${/}screenshot-and-index${/}some-{other}-name-1.png
     File Should Exist    ${OUTPUTDIR}/screenshot-and-index/other-2-name.png
     Should Be Equal    ${file3}    ${OUTPUTDIR}${/}screenshot-and-index${/}other-2-name.png
 
diff --git a/utest/test/api/approved_files/test_filepath_unusual_characters.test_normal_file_path.approved.txt b/utest/test/api/approved_files/test_filepath_unusual_characters.test_normal_file_path.approved.txt
new file mode 100644
index 00000000..604154bd
--- /dev/null
+++ b/utest/test/api/approved_files/test_filepath_unusual_characters.test_normal_file_path.approved.txt
@@ -0,0 +1,11 @@
+Different file paths.
+
+0) /foo/file.log
+1) /foo/file-1.log
+2) /foo/file-0001.log
+3) /foo/file-{foo}.log
+4) /{foo}/file-1.log
+5) /foo/file-001.log
+6) /foo/1234-file-1234.log
+7) /foo/file-{in dex}.log
+8) /foo/file-{in@dex}.log
diff --git a/utest/test/api/test_filepath_unusual_characters.py b/utest/test/api/test_filepath_unusual_characters.py
new file mode 100644
index 00000000..67d6d9eb
--- /dev/null
+++ b/utest/test/api/test_filepath_unusual_characters.py
@@ -0,0 +1,43 @@
+import os
+
+import pytest
+from robot.utils import JYTHON
+
+try:
+    from approvaltests.approvals import verify_all
+    from approvaltests.reporters.generic_diff_reporter_factory import GenericDiffReporterFactory
+except ImportError:
+    if JYTHON:
+        verify = None
+        GenericDiffReporterFactory = None
+    else:
+        raise
+
+from SeleniumLibrary.utils.path_formatter import _format_path
+
+
+@pytest.fixture(scope='module')
+def reporter():
+    if JYTHON:
+        return None
+    else:
+        path = os.path.dirname(__file__)
+        reporter_json = os.path.abspath(os.path.join(path, '..', 'approvals_reporters.json'))
+        factory = GenericDiffReporterFactory()
+        factory.load(reporter_json)
+        return factory.get_first_working()
+
+
+@pytest.mark.skipif(JYTHON, reason='ApprovalTest does not work with Jython')
+def test_normal_file_path(reporter):
+    results = []
+    results.append(_format_path('/foo/file.log', 1))
+    results.append(_format_path('/foo/file-{index}.log', 1))
+    results.append(_format_path('/foo/file-{index}.log', '0001'))
+    results.append(_format_path('/foo/file-{foo}.log', 1))
+    results.append(_format_path('/{foo}/file-{index}.log', 1))
+    results.append(_format_path('/foo/file-{index:03}.log', 1))
+    results.append(_format_path('/foo/{index}-file-{index}.log', '1234'))
+    results.append(_format_path('/foo/file-{in dex}.log', '1234'))
+    results.append(_format_path('/foo/file-{in@dex}.log', '1234'))
+    verify_all('Different file paths.', results, reporter=reporter)
diff --git a/utest/test/keywords/test_keyword_arguments_browsermanagement.py b/utest/test/keywords/test_keyword_arguments_browsermanagement.py
index 5cda8a55..643fc534 100644
--- a/utest/test/keywords/test_keyword_arguments_browsermanagement.py
+++ b/utest/test/keywords/test_keyword_arguments_browsermanagement.py
@@ -1,6 +1,6 @@
 import unittest
 
-from mockito import mock, unstub, when
+from mockito import mock, unstub, when, verify, ANY
 
 from SeleniumLibrary.keywords import BrowserManagementKeywords
 
@@ -12,6 +12,7 @@ class KeywordArgumentsElementTest(unittest.TestCase):
         ctx.event_firing_webdriver = None
         ctx._browser = mock()
         ctx._drivers = mock()
+        self.ctx = ctx
         self.brorser = BrowserManagementKeywords(ctx)
 
     def tearDown(self):
@@ -31,3 +32,31 @@ class KeywordArgumentsElementTest(unittest.TestCase):
         alias = self.brorser.open_browser(url, alias='None',
                                           remote_url=remote_url)
         self.assertEqual(alias, None)
+
+    def test_same_alias(self):
+        url = 'https://github.com/robotframework'
+        alias = 'tidii'
+        driver = mock()
+        driver.session_id = 'foobar'
+        self.ctx.driver = driver
+        when(self.ctx._drivers).get_index(alias).thenReturn(1)
+        when(self.ctx._drivers).switch(1).thenReturn(driver)
+        self.brorser.open_browser(url=url, alias=alias)
+        verify(driver, times=1).get(url)
+
+    def test_open_browser_no_get(self):
+        browser = mock()
+        when(self.brorser)._make_driver('firefox', None,
+                                        None, False, None, None).thenReturn(browser)
+        self.brorser.open_browser()
+        verify(browser, times=0).get(ANY)
+
+    def test_same_alias_and_not_get(self):
+        alias = 'tidii'
+        driver = mock()
+        driver.session_id = 'foobar'
+        self.ctx.driver = driver
+        when(self.ctx._drivers).get_index(alias).thenReturn(1)
+        when(self.ctx._drivers).switch(1).thenReturn(driver)
+        self.brorser.open_browser(alias=alias)
+        verify(driver, times=0).get(ANY)