Merge pull request #4 from boegel/backup_modules

cleanup & tests for find_backup_name_candidate & move_file

easybuild/tools/filetools.py

@@ -1163,16 +1163,15 @@ def rmtree2(path, n=3):
     else:
         _log.info("Path %s successfully removed." % path)
 
+
 def find_backup_name_candidate(src_file):
-    """Returns a non-existing file to be used as destination for rotating backups"""
+    """Returns a non-existing file to be used as destination for backup files"""
     dst_file = src_file
 
-    if os.path.exists(src_file):
-        i = 0
-        dst_file = "%s_%d" % (src_file, i)
-        while os.path.exists(dst_file):
-            i += 1
-            dst_file = "%s_%d" % (src_file, i)
+    i = 0
+    while os.path.exists(dst_file):
+        dst_file = '%s_%d' % (src_file, i)
+        i += 1
 
     return dst_file
 
@@ -1220,8 +1219,6 @@ def move_logs(src_logfile, target_logfile):
             if os.path.exists(new_log_path):
                 back_up_file(new_log_path)
 
-            # remove first to ensure portability (shutil.move might fail when overwriting files in some systems)
-            remove_file(new_log_path)
             # move log to target path
             move_file(app_log, new_log_path)
             _log.info("Moved log file %s to %s" % (src_logfile, new_log_path))
@@ -1589,12 +1586,14 @@ def move_file(path, target_path, force_in_dry_run=False):
     Move a file from path to target_path
 
     :param path: the original filepath
-    :param target_path: path to copy the file to
+    :param target_path: path to move the file to
     :param force_in_dry_run: force running the command during dry run
     """
     if not force_in_dry_run and build_option('extended_dry_run'):
         dry_run_msg("moved file %s to %s" % (path, target_path))
     else:
+        # remove first to ensure portability (shutil.move might fail when overwriting files in some systems)
+        remove_file(target_path)
         try:
             mkdir(os.path.dirname(target_path), parents=True)
             shutil.move(path, target_path)

test/framework/filetools.py

@@ -1311,6 +1311,72 @@ def test_find_eb_script(self):
 
         self.assertTrue(os.path.samefile(ft.find_eb_script('justatest.sh'), justatest))
 
+    def test_move_file(self):
+        """Test move_file function"""
+        test_file = os.path.join(self.test_prefix, 'test.txt')
+        ft.write_file(test_file, 'test123')
+
+        new_test_file = os.path.join(self.test_prefix, 'subdir', 'new_test.txt')
+        ft.move_file(test_file, new_test_file)
+
+        self.assertFalse(os.path.exists(test_file))
+        self.assertTrue(os.path.exists(new_test_file))
+        self.assertEqual(ft.read_file(new_test_file), 'test123')
+
+        # test moving to an existing file
+        ft.write_file(test_file, 'gibberish')
+        ft.move_file(new_test_file, test_file)
+
+        self.assertTrue(os.path.exists(test_file))
+        self.assertEqual(ft.read_file(test_file), 'test123')
+        self.assertFalse(os.path.exists(new_test_file))
+
+        # also test behaviour of move_file under --dry-run
+        build_options = {
+            'extended_dry_run': True,
+            'silent': False,
+        }
+        init_config(build_options=build_options)
+
+        self.mock_stdout(True)
+        self.mock_stderr(True)
+        ft.move_file(test_file, new_test_file)
+        stdout = self.get_stdout()
+        stderr = self.get_stderr()
+        self.mock_stdout(False)
+        self.mock_stderr(False)
+
+        # informative message printed, but file was not actually moved
+        regex = re.compile("^moved file .*/test\.txt to .*/new_test\.txt$")
+        self.assertTrue(regex.search(stdout), "Pattern '%s' found in: %s" % (regex.pattern, stdout))
+        self.assertEqual(stderr, '')
+
+        self.assertTrue(os.path.exists(test_file))
+        self.assertEqual(ft.read_file(test_file), 'test123')
+        self.assertFalse(os.path.exists(new_test_file))
+
+    def test_find_backup_name_candidate(self):
+        """Test find_backup_name_candidate"""
+        test_file = os.path.join(self.test_prefix, 'test.txt')
+        ft.write_file(test_file, 'foo')
+
+        res = ft.find_backup_name_candidate(test_file)
+        self.assertTrue(os.path.samefile(os.path.dirname(res), self.test_prefix))
+        self.assertEqual(os.path.basename(res), 'test.txt_0')
+
+        ft.write_file(os.path.join(self.test_prefix, 'test.txt_0'), '')
+        res = ft.find_backup_name_candidate(test_file)
+        self.assertTrue(os.path.samefile(os.path.dirname(res), self.test_prefix))
+        self.assertEqual(os.path.basename(res), 'test.txt_1')
+
+        # generate backup files until test.txt_122
+        for idx in range(1, 123):
+            ft.write_file(os.path.join(self.test_prefix, 'test.txt_%d' % idx), '')
+
+        res = ft.find_backup_name_candidate(test_file)
+        self.assertTrue(os.path.samefile(os.path.dirname(res), self.test_prefix))
+        self.assertEqual(os.path.basename(res), 'test.txt_123')
+
 
 def suite():
     """ returns all the testcases in this module """