Add methot to replace Python format for PY2 and PY3 (robotframework#1468)

aaltat · web-flow · commit 9eeada356332 · 2019-10-01T00:01:54.000+03:00
/path/{to}/file-{index}.log types of paths causes problems for Python format method. This is now fixed. Fixes robotframework#1452
diff --git 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/src/SeleniumLibrary/keywords/screenshot.py b/src/SeleniumLibrary/keywords/screenshot.py
@@ -20,6 +20,7 @@
 
 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 @@ def _get_screenshot_path(self, filename):
         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
@@ -28,6 +28,8 @@
 
 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 @@ def _get_log_path(self, log_file):
             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
@@ -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
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
@@ -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
@@ -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)
