diff --git a/AutomatedTesting/Gem/PythonTests/Physics/utils/FileManagement.py b/AutomatedTesting/Gem/PythonTests/Physics/utils/FileManagement.py index 017bb672a4..03287d0542 100644 --- a/AutomatedTesting/Gem/PythonTests/Physics/utils/FileManagement.py +++ b/AutomatedTesting/Gem/PythonTests/Physics/utils/FileManagement.py @@ -98,38 +98,32 @@ class FileManagement: """ file_map = FileManagement._load_file_map() backup_path = FileManagement.backup_folder_path - backup_file_name = "{}.bak".format(file_name) - backup_file = os.path.join(backup_path, backup_file_name) # If backup directory DNE, make one if not os.path.exists(backup_path): os.mkdir(backup_path) - # If "traditional" backup file exists, delete it (myFile.txt.bak) - if os.path.exists(backup_file): - fs.delete([backup_file], True, False) - # Find my next storage name (myFile_1.txt.bak) - backup_storage_file_name = FileManagement._next_available_name(backup_file_name, file_map) - if backup_storage_file_name is None: + + # Find my next storage name (myFile_1.txt) + backup_file_name = FileManagement._next_available_name(file_name, file_map) + if backup_file_name is None: # If _next_available_name returns None, we have backed up MAX_BACKUPS of files name [file_name] raise Exception( "FileManagement class ran out of backups per name. Max: {}".format(FileManagement.MAX_BACKUPS) ) - backup_storage_file = os.path.join(backup_path, backup_storage_file_name) + + # If this backup file already exists, delete it. + backup_storage_file = "{}.bak".format(os.path.normpath(os.path.join(backup_path, backup_file_name))) if os.path.exists(backup_storage_file): - # This file should not exists, but if it does it's about to get clobbered! - fs.unlock_file(backup_storage_file) - # Create "traditional" backup file (myFile.txt.bak) - fs.create_backup(os.path.join(file_path, file_name), backup_path) - # Copy "traditional" backup file into storage backup (myFile_1.txt.bak) - FileManagement._copy_file(backup_file_name, backup_path, backup_storage_file_name, backup_path) - fs.lock_file(backup_storage_file) - # Delete "traditional" back up file - fs.unlock_file(backup_file) - fs.delete([backup_file], True, False) + fs.delete([backup_storage_file], True, False) + + # Create backup file (myFile_1.txt.bak) + original_file = os.path.normpath(os.path.join(file_path, file_name)) + fs.create_backup(original_file, backup_path, backup_file_name) + # Update file map with new file - file_map[os.path.join(file_path, file_name)] = backup_storage_file_name + file_map[original_file] = backup_file_name FileManagement._save_file_map(file_map) # Unlock original file to get it ready to be edited by the test - fs.unlock_file(os.path.join(file_path, file_name)) + fs.unlock_file(original_file) @staticmethod def _restore_file(file_name, file_path): @@ -143,20 +137,15 @@ class FileManagement: """ file_map = FileManagement._load_file_map() backup_path = FileManagement.backup_folder_path - src_file = os.path.join(file_path, file_name) + src_file = os.path.normpath(os.path.join(file_path, file_name)) if src_file in file_map: - backup_file = os.path.join(backup_path, file_map[src_file]) - if os.path.exists(backup_file): - fs.unlock_file(backup_file) - fs.unlock_file(src_file) - # Make temporary copy of backed up file to restore from - temp_file = "{}.bak".format(file_name) - FileManagement._copy_file(file_map[src_file], backup_path, temp_file, backup_path) - fs.restore_backup(src_file, backup_path) - fs.lock_file(src_file) - # Delete backup file - fs.delete([os.path.join(backup_path, temp_file)], True, False) + backup_file_name = file_map[src_file] + backup_file = "{}.bak".format(os.path.join(backup_path, backup_file_name)) + + fs.unlock_file(src_file) + if fs.restore_backup(src_file, backup_path, backup_file_name): fs.delete([backup_file], True, False) + # Remove from file map del file_map[src_file] FileManagement._save_file_map(file_map) diff --git a/Tools/LyTestTools/ly_test_tools/environment/file_system.py b/Tools/LyTestTools/ly_test_tools/environment/file_system.py index c67ecc5b23..e8fb1d13cf 100755 --- a/Tools/LyTestTools/ly_test_tools/environment/file_system.py +++ b/Tools/LyTestTools/ly_test_tools/environment/file_system.py @@ -293,61 +293,75 @@ def delete(file_list, del_files, del_dirs): return True -def create_backup(source, backup_dir): +def create_backup(source, backup_dir, backup_name=None): """ Creates a backup of a single source file by creating a copy of it with the same name + '.bak' in backup_dir e.g.: foo.txt is stored as backup_dir/foo.txt.bak + If backup_name is provided, it will create a copy of the source file named "backup_name + .bak" instead. :param source: Full path to file to backup :param backup_dir: Path to the directory to store backup. + :param backup_name: [Optional] Name of the backed up file to use instead or the source name. """ if not backup_dir or not os.path.isdir(backup_dir): logger.error(f'Cannot create backup due to invalid backup directory {backup_dir}') - return + return False if not os.path.exists(source): logger.warning(f'Source file {source} does not exist, aborting backup creation.') - return + return False - source_filename = os.path.basename(source) - dest = os.path.join(backup_dir, f'{source_filename}.bak') + dest = None + if backup_name is None: + source_filename = os.path.basename(source) + dest = os.path.join(backup_dir, f'{source_filename}.bak') + else: + dest = os.path.join(backup_dir, f'{backup_name}.bak') logger.info(f'Saving backup of {source} in {dest}') if os.path.exists(dest): logger.warning(f'Backup file already exists at {dest}, it will be overwritten.') try: - shutil.copy(source, dest) + shutil.copy2(source, dest) except Exception: # intentionally broad logger.warning('Could not create backup, exception occurred while copying.', exc_info=True) + return False + return True -def restore_backup(original_file, backup_dir): +def restore_backup(original_file, backup_dir, backup_name=None): """ Restores a backup file to its original location. Works with a single file only. :param original_file: Full path to file to overwrite. :param backup_dir: Path to the directory storing the backup. + :param backup_name: [Optional] Provide if the backup file name is different from source. eg backup file = myFile_1.txt.bak original file = myfile.txt """ if not backup_dir or not os.path.isdir(backup_dir): logger.error(f'Cannot restore backup due to invalid or nonexistent directory {backup_dir}.') - return + return False - source_filename = os.path.basename(original_file) - backup = os.path.join(backup_dir, f'{source_filename}.bak') + backup = None + if backup_name is None: + source_filename = os.path.basename(original_file) + backup = os.path.join(backup_dir, f'{source_filename}.bak') + else: + backup = os.path.join(backup_dir, f'{backup_name}.bak') if not os.path.exists(backup): logger.warning(f'Backup file {backup} does not exist, aborting backup restoration.') - return + return False logger.info(f'Restoring backup of {original_file} from {backup}') try: - shutil.copy(backup, original_file) + shutil.copy2(backup, original_file) except Exception: # intentionally broad logger.warning('Could not restore backup, exception occurred while copying.', exc_info=True) - + return False + return True def delete_oldest(path_glob, keep_num, del_files=True, del_dirs=False): """ Delete oldest builds, keeping a specific number """ diff --git a/Tools/LyTestTools/tests/unit/test_file_system.py b/Tools/LyTestTools/tests/unit/test_file_system.py index 2ddd3f8934..2fdcf8131e 100755 --- a/Tools/LyTestTools/tests/unit/test_file_system.py +++ b/Tools/LyTestTools/tests/unit/test_file_system.py @@ -751,7 +751,7 @@ class TestFileBackup(unittest.TestCase): self._dummy_file = 'dummy.txt' self._dummy_backup_file = os.path.join(self._dummy_dir, '{}.bak'.format(self._dummy_file)) - @mock.patch('shutil.copy') + @mock.patch('shutil.copy2') @mock.patch('os.path.exists') @mock.patch('os.path.isdir') def test_BackupSettings_SourceExists_BackupCreated(self, mock_path_isdir, mock_backup_exists, mock_copy): @@ -763,7 +763,7 @@ class TestFileBackup(unittest.TestCase): mock_copy.assert_called_with(self._dummy_file, self._dummy_backup_file) @mock.patch('ly_test_tools.environment.file_system.logger.warning') - @mock.patch('shutil.copy') + @mock.patch('shutil.copy2') @mock.patch('os.path.exists') @mock.patch('os.path.isdir') def test_BackupSettings_BackupExists_WarningLogged(self, mock_path_isdir, mock_backup_exists, mock_copy, mock_logger_warning): @@ -776,7 +776,7 @@ class TestFileBackup(unittest.TestCase): mock_logger_warning.assert_called_once() @mock.patch('ly_test_tools.environment.file_system.logger.warning') - @mock.patch('shutil.copy') + @mock.patch('shutil.copy2') @mock.patch('os.path.exists') @mock.patch('os.path.isdir') def test_BackupSettings_SourceNotExists_WarningLogged(self, mock_path_isdir, mock_backup_exists, mock_copy, mock_logger_warning): @@ -789,7 +789,7 @@ class TestFileBackup(unittest.TestCase): mock_logger_warning.assert_called_once() @mock.patch('ly_test_tools.environment.file_system.logger.warning') - @mock.patch('shutil.copy') + @mock.patch('shutil.copy2') @mock.patch('os.path.exists') @mock.patch('os.path.isdir') def test_BackupSettings_CannotCopy_WarningLogged(self, mock_path_isdir, mock_backup_exists, mock_copy, mock_logger_warning): @@ -821,7 +821,7 @@ class TestFileBackupRestore(unittest.TestCase): self._dummy_file = 'dummy.txt' self._dummy_backup_file = os.path.join(self._dummy_dir, '{}.bak'.format(self._dummy_file)) - @mock.patch('shutil.copy') + @mock.patch('shutil.copy2') @mock.patch('os.path.exists') @mock.patch('os.path.isdir') def test_RestoreSettings_BackupRestore_Success(self, mock_path_isdir, mock_exists, mock_copy): @@ -832,7 +832,7 @@ class TestFileBackupRestore(unittest.TestCase): mock_copy.assert_called_with(self._dummy_backup_file, self._dummy_file) @mock.patch('ly_test_tools.environment.file_system.logger.warning') - @mock.patch('shutil.copy') + @mock.patch('shutil.copy2') @mock.patch('os.path.exists') @mock.patch('os.path.isdir') def test_RestoreSettings_CannotCopy_WarningLogged(self, mock_path_isdir, mock_exists, mock_copy, mock_logger_warning): @@ -846,7 +846,7 @@ class TestFileBackupRestore(unittest.TestCase): mock_logger_warning.assert_called_once() @mock.patch('ly_test_tools.environment.file_system.logger.warning') - @mock.patch('shutil.copy') + @mock.patch('shutil.copy2') @mock.patch('os.path.exists') @mock.patch('os.path.isdir') def test_RestoreSettings_BackupNotExists_WarningLogged(self, mock_path_isdir, mock_exists, mock_copy, mock_logger_warning):