From 8cb035a584650f9b7c51b3205bd0c289bb7f7003 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Gonz=C3=A1lez=20Alonso?= Date: Mon, 20 Jan 2020 18:30:26 +0100 Subject: [PATCH] Fix concurrent folder creation Add *makedirs_safe* method to create a new folder. --- CHANGELOG.rst | 2 + toolium/driver_wrapper.py | 2 +- toolium/driver_wrappers_pool.py | 5 +- toolium/{format_utils.py => path_utils.py} | 13 +++++ toolium/selenoid.py | 5 +- ...est_format_utils.py => test_path_utils.py} | 52 +++++++++++++++++-- toolium/utils.py | 9 ++-- toolium/visual_test.py | 8 ++- 8 files changed, 77 insertions(+), 19 deletions(-) rename toolium/{format_utils.py => path_utils.py} (83%) rename toolium/test/{test_format_utils.py => test_path_utils.py} (51%) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 58a97844..bea0922d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,6 +6,8 @@ v1.6.1.dev0 *In development* +- Fix concurrent folder creation. Add *makedirs_safe* method to create a new folder. + v1.6.0 ------ diff --git a/toolium/driver_wrapper.py b/toolium/driver_wrapper.py index cb9441f9..cf666f57 100644 --- a/toolium/driver_wrapper.py +++ b/toolium/driver_wrapper.py @@ -25,7 +25,7 @@ from toolium.config_parser import ExtendedConfigParser from toolium.driver_wrappers_pool import DriverWrappersPool from toolium.utils import Utils -from toolium.format_utils import get_valid_filename +from toolium.path_utils import get_valid_filename class DriverWrapper(object): diff --git a/toolium/driver_wrappers_pool.py b/toolium/driver_wrappers_pool.py index ebeab4ff..a96a964d 100644 --- a/toolium/driver_wrappers_pool.py +++ b/toolium/driver_wrappers_pool.py @@ -22,7 +22,7 @@ import datetime from toolium.config_files import ConfigFiles -from toolium.format_utils import get_valid_filename +from toolium.path_utils import get_valid_filename, makedirs_safe from toolium.selenoid import Selenoid @@ -285,8 +285,7 @@ def configure_common_directories(cls, tc_config_files): if not os.path.isabs(cls.output_directory): # If output directory is relative, we use the same path as config directory cls.output_directory = os.path.join(os.path.dirname(cls.config_directory), cls.output_directory) - if not os.path.exists(cls.output_directory): - os.makedirs(cls.output_directory) + makedirs_safe(cls.output_directory) # Get visual baseline directory from properties default_baseline = os.path.join(cls.output_directory, 'visualtests', 'baseline') diff --git a/toolium/format_utils.py b/toolium/path_utils.py similarity index 83% rename from toolium/format_utils.py rename to toolium/path_utils.py index af222922..2cc0128d 100644 --- a/toolium/format_utils.py +++ b/toolium/path_utils.py @@ -16,6 +16,7 @@ limitations under the License. """ +import os import re FILENAME_MAX_LENGTH = 100 @@ -33,3 +34,15 @@ def get_valid_filename(s, max_length=FILENAME_MAX_LENGTH): s = str(s).strip().replace(' -- @', '_') s = re.sub(r'(?u)[^-\w]', '_', s).strip('_') return s[:max_length] + + +def makedirs_safe(folder): + """ + Create a new folder if it does not exist + :param folder: folder path + """ + try: + os.makedirs(folder) + except OSError as e: + if e.errno != os.errno.EEXIST: + raise diff --git a/toolium/selenoid.py b/toolium/selenoid.py index 8cd7fd5f..4f2cff3d 100644 --- a/toolium/selenoid.py +++ b/toolium/selenoid.py @@ -21,6 +21,8 @@ import requests +from toolium.path_utils import makedirs_safe + # constants STATUS_OK = 200 STATUS_PORT = "8888" @@ -93,8 +95,7 @@ def __download_file(self, url, path_file, timeout): # create the folders and store the file downloaded if status_code == STATUS_OK: path, name = os.path.split(path_file) - if not os.path.exists(path): - os.makedirs(path) + makedirs_safe(path) try: fp = open(path_file, 'wb') fp.write(body.content) diff --git a/toolium/test/test_format_utils.py b/toolium/test/test_path_utils.py similarity index 51% rename from toolium/test/test_format_utils.py rename to toolium/test/test_path_utils.py index 532bcd69..a00c76a8 100644 --- a/toolium/test/test_format_utils.py +++ b/toolium/test/test_path_utils.py @@ -16,8 +16,14 @@ limitations under the License. """ +import os +import Queue +import threading +import uuid + import pytest -import toolium.format_utils as format_utils + +from toolium.path_utils import get_valid_filename, makedirs_safe filename_tests = ( ('hola_pepito', 'hola_pepito'), @@ -30,7 +36,7 @@ @pytest.mark.parametrize('input_filename, expected_filename', filename_tests) def test_get_valid_filename(input_filename, expected_filename): - valid_filename = format_utils.get_valid_filename(input_filename) + valid_filename = get_valid_filename(input_filename) assert expected_filename == valid_filename @@ -38,6 +44,46 @@ def test_get_valid_filename(input_filename, expected_filename): def test_get_valid_filename_length(): input_filename = ' hola:pep /ito* ' expected_filename = 'hola_pep__it' - valid_filename = format_utils.get_valid_filename(input_filename, 12) + valid_filename = get_valid_filename(input_filename, 12) assert expected_filename == valid_filename + + +def test_create_new_folder(): + folder = os.path.join('output', str(uuid.uuid4())) + makedirs_safe(folder) + + assert os.path.isdir(folder) + os.rmdir(folder) + + +def test_create_existing_folder(): + folder = os.path.join('output', str(uuid.uuid4())) + os.makedirs(folder) + makedirs_safe(folder) + + assert os.path.isdir(folder) + os.rmdir(folder) + + +def test_create_new_folder_parallel(): + folder = os.path.join('output', str(uuid.uuid4())) + + def run_makedirs(folder, exceptions): + try: + makedirs_safe(folder) + except Exception as exc: + exceptions.put(exc) + + for _ in range(5): + exceptions = Queue.Queue() + thread1 = threading.Thread(target=run_makedirs, args=(folder, exceptions)) + thread2 = threading.Thread(target=run_makedirs, args=(folder, exceptions)) + thread1.start() + thread2.start() + thread1.join() + thread2.join() + + assert exceptions.qsize() == 0 + assert os.path.isdir(folder) + os.rmdir(folder) diff --git a/toolium/utils.py b/toolium/utils.py index 54774697..2c5d0967 100644 --- a/toolium/utils.py +++ b/toolium/utils.py @@ -32,7 +32,7 @@ from six.moves.urllib.parse import urlparse # Python 2 and 3 compatibility from toolium.driver_wrappers_pool import DriverWrappersPool -from toolium.format_utils import get_valid_filename +from toolium.path_utils import get_valid_filename, makedirs_safe class Utils(object): @@ -69,8 +69,7 @@ def capture_screenshot(self, name): filename = '{0:0=2d}_{1}'.format(DriverWrappersPool.screenshots_number, name) filename = '{}.png'.format(get_valid_filename(filename)) filepath = os.path.join(DriverWrappersPool.screenshots_directory, filename) - if not os.path.exists(DriverWrappersPool.screenshots_directory): - os.makedirs(DriverWrappersPool.screenshots_directory) + makedirs_safe(DriverWrappersPool.screenshots_directory) if self.driver_wrapper.driver.get_screenshot_as_file(filepath): self.logger.info('Screenshot saved in %s', filepath) DriverWrappersPool.screenshots_number += 1 @@ -530,8 +529,7 @@ def _download_video(self, video_url, video_name): filename = '{0:0=2d}_{1}'.format(DriverWrappersPool.videos_number, video_name) filename = '{}.mp4'.format(get_valid_filename(filename)) filepath = os.path.join(DriverWrappersPool.videos_directory, filename) - if not os.path.exists(DriverWrappersPool.videos_directory): - os.makedirs(DriverWrappersPool.videos_directory) + makedirs_safe(DriverWrappersPool.videos_directory) response = requests.get(video_url) open(filepath, 'wb').write(response.content) self.logger.info("Video saved in '%s'", filepath) @@ -665,3 +663,4 @@ def get_first_webview_context(self): def switch_to_first_webview_context(self): """Switch to the first WEBVIEW context""" self.driver_wrapper.driver.switch_to.context(self.get_first_webview_context()) + diff --git a/toolium/visual_test.py b/toolium/visual_test.py index adb2b820..76772c70 100644 --- a/toolium/visual_test.py +++ b/toolium/visual_test.py @@ -31,7 +31,7 @@ from six.moves import xrange # Python 2 and 3 compatibility from toolium.driver_wrappers_pool import DriverWrappersPool -from toolium.format_utils import get_valid_filename +from toolium.path_utils import get_valid_filename, makedirs_safe try: from needle.engines.perceptualdiff_engine import Engine as PerceptualEngine @@ -80,10 +80,8 @@ def __init__(self, driver_wrapper=None, force=False): self.save_baseline = self.driver_wrapper.config.getboolean_optional('VisualTests', 'save') # Create folders - if not os.path.exists(self.baseline_directory): - os.makedirs(self.baseline_directory) - if not os.path.exists(self.output_directory): - os.makedirs(self.output_directory) + makedirs_safe(self.baseline_directory) + makedirs_safe(self.output_directory) # Copy js, css and html template to output directory dst_template_path = os.path.join(self.output_directory, self.report_name)