remove target file first in move_file, add test for move_file

easybuild/tools/filetools.py

@@ -1220,8 +1220,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 +1587,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,50 @@ 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 suite():
     """ returns all the testcases in this module """