Skip to content

Commit

Permalink
Add methot to replace Python format for PY2 and PY3 (#1468)
Browse files Browse the repository at this point in the history
/path/{to}/file-{index}.log types of paths causes problems for Python format method. This is now fixed. 

Fixes  #1452
  • Loading branch information
aaltat authored Sep 30, 2019
1 parent 69e94b3 commit 9eeada3
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 5 deletions.
6 changes: 3 additions & 3 deletions atest/acceptance/keywords/screenshots.robot
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion src/SeleniumLibrary/keywords/screenshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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):
Expand Down
30 changes: 30 additions & 0 deletions src/SeleniumLibrary/utils/path_formatter.py
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
43 changes: 43 additions & 0 deletions utest/test/api/test_filepath_unusual_characters.py
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit 9eeada3

Please sign in to comment.