ソースを参照

Fixed race condition in directory calls in ly_test_tools (#16420)

* Fixed race condition in directory calls in ly_test_tools

Between checking whether a directory exist and making a call to create
the directory in the ly_test_tools python script, a parallel execution
of another test could create that directory, which would cause the
directory creation to raise a python exception causing the test to fail.

Signed-off-by: lumberyard-employee-dm <[email protected]>

* Fixing mock calls of os.makedirs inside of the LyTestTools Workspace
pytest

Signed-off-by: lumberyard-employee-dm <[email protected]>

---------

Signed-off-by: lumberyard-employee-dm <[email protected]>
lumberyard-employee-dm 2 年 前
コミット
8e874f6a82

+ 1 - 1
Tools/LyTestTools/ly_test_tools/_internal/managers/artifact_manager.py

@@ -74,7 +74,7 @@ class ArtifactManager(object):
             try:
                 logger.debug(f'Attempting to create new artifact path: "{self.dest_path}"')
                 if not os.path.exists(self.dest_path):
-                    os.makedirs(self.dest_path)
+                    os.makedirs(self.dest_path, exist_ok=True)
                     logger.info(f'Created new artifact path: "{self.dest_path}"')
                     return self.dest_path
             except (IOError, OSError, WindowsError) as err:

+ 3 - 3
Tools/LyTestTools/ly_test_tools/_internal/managers/workspace.py

@@ -83,11 +83,11 @@ class AbstractWorkspaceManager:
                 ly_test_tools.environment.file_system.delete([self.tmp_path], True, True)
 
             print("Creating tmp path")
-            os.makedirs(self.tmp_path)
+            os.makedirs(self.tmp_path, exist_ok=True)
 
         if not os.path.exists(self.output_path):
             print(f"Creating logs path at {self.output_path}")
-            os.makedirs(self.output_path)
+            os.makedirs(self.output_path, exist_ok=True)
 
         print(f"Configuring Artifact Manager with path {self.output_path}")
         self.artifact_manager = artifact_manager.ArtifactManager(self.output_path)
@@ -139,7 +139,7 @@ class AbstractWorkspaceManager:
         temp_file_dir = os.path.join(tempfile.gettempdir(), "LyTestTools")
         temp_file_path = os.path.join(temp_file_dir, log_file_name)
         if not os.path.exists(temp_file_dir):
-            os.makedirs(temp_file_dir)
+            os.makedirs(temp_file_dir, exist_ok=True)
 
         if os.path.exists(temp_file_path):
             # assume temp is not being used for long-term storage, and safe to delete

+ 1 - 1
Tools/LyTestTools/ly_test_tools/environment/file_system.py

@@ -41,7 +41,7 @@ def safe_makedirs(dest_path):
     """ This allows an OSError in the case the directory cannot be created, which is logged but does not propagate."""
     try:
         logger.info(f'Creating directory "{dest_path}"')
-        os.makedirs(dest_path)
+        os.makedirs(dest_path, exist_ok=True)
 
     except OSError as e:
         if e.errno == errno.EEXIST:

+ 1 - 1
Tools/LyTestTools/ly_test_tools/mobile/android.py

@@ -136,7 +136,7 @@ def pull_files_to_pc(package_name, logs_path, device=None):
     """
     directory = os.path.join(logs_path, device)
     if not os.path.exists(directory):
-        os.makedirs(directory)
+        os.makedirs(directory, exist_ok=True)
 
     pull_cmd = ['adb']
     if device is not None:

+ 4 - 4
Tools/LyTestTools/ly_test_tools/o3de/asset_processor.py

@@ -719,7 +719,7 @@ class AssetProcessor(object):
         for copy_dir in [self._workspace.project, 'Assets/Engine', 'Registry']:
             make_dir = os.path.join(self._temp_asset_root, copy_dir)
             if not os.path.isdir(make_dir):
-                os.makedirs(make_dir)
+                os.makedirs(make_dir, exist_ok=True)
         for copyfile_name in ['Registry/AssetProcessorPlatformConfig.setreg',
                               os.path.join(self._workspace.project, "project.json"),
                               os.path.join('Assets', 'Engine', 'exclude.filetag')]:
@@ -813,7 +813,7 @@ class AssetProcessor(object):
             scan_folder = os.path.join(self._temp_asset_root if self._temp_asset_root else self._workspace.paths.engine_root(),
                                        folder_name)
             if not os.path.isdir(scan_folder):
-                os.makedirs(scan_folder)
+                os.makedirs(scan_folder, exist_ok=True)
             if folder_name not in self._override_scan_folders:
                 self._override_scan_folders.append(scan_folder)
                 logger.debug(f"Adding scan folder {scan_folder}")
@@ -865,7 +865,7 @@ class AssetProcessor(object):
         test_folder = os.path.join(test_asset_root, function_name if existing_function_name is None
                                    else existing_function_name)
         if not os.path.isdir(test_folder):
-            os.makedirs(test_folder)
+            os.makedirs(test_folder, exist_ok=True)
         if add_scan_folder:
             self.add_scan_folder(test_asset_root)
         self._function_name = function_name
@@ -962,7 +962,7 @@ class AssetProcessor(object):
         dest_root = dest_root or self._temp_asset_root
         make_dir = os.path.dirname(os.path.join(dest_root, relative_asset))
         if not os.path.isdir(make_dir):
-            os.makedirs(make_dir)
+            os.makedirs(make_dir, exist_ok=True)
         shutil.copyfile(os.path.join(source_root, relative_asset),
                         os.path.join(dest_root, relative_asset))
 

+ 1 - 1
Tools/LyTestTools/ly_test_tools/o3de/pipeline_utils.py

@@ -197,7 +197,7 @@ def create_asset_processor_backup_directories(backup_root_directory: str, test_b
     :return: None
     """
     if not os.path.exists(os.path.join(backup_root_directory, test_backup_directory)):
-        os.makedirs(os.path.join(backup_root_directory, test_backup_directory))
+        os.makedirs(os.path.join(backup_root_directory, test_backup_directory), exist_ok=True)
 
 
 def backup_asset_processor_logs(bin_directory: str, backup_directory: str) -> None:

+ 2 - 2
Tools/LyTestTools/tests/unit/test_workspace.py

@@ -86,7 +86,7 @@ class TestSetup(unittest.TestCase):
 
         self.mock_workspace.setup()
 
-        mock_makedirs.assert_called_once_with(self.mock_workspace.tmp_path)
+        mock_makedirs.assert_called_once_with(self.mock_workspace.tmp_path, exist_ok=True)
 
     @mock.patch('os.makedirs')
     @mock.patch('os.path.exists')
@@ -117,7 +117,7 @@ class TestSetup(unittest.TestCase):
         self.mock_workspace.output_path = 'mock_output_path'
 
         self.mock_workspace.setup()
-        mock_makedirs.assert_called_with(self.mock_workspace.output_path)
+        mock_makedirs.assert_called_with(self.mock_workspace.output_path, exist_ok=True)
         assert mock_makedirs.call_count == 2  # ArtifactManager.__init__() calls os.path.exists()