From ab9addb848926373d65fdbd149ba7435a5dedf63 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Thu, 17 Aug 2017 11:08:32 +0200 Subject: [PATCH 1/2] remove target file first in move_file, add test for move_file --- easybuild/tools/filetools.py | 6 ++--- test/framework/filetools.py | 44 ++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index e612729c69..31e20c5c5a 100644 --- a/easybuild/tools/filetools.py +++ b/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) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index ed5adee2ee..4660c0baf2 100644 --- a/test/framework/filetools.py +++ b/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 """ From cd4c395c66b06043a9429b030a415050b276fd32 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Thu, 17 Aug 2017 11:21:02 +0200 Subject: [PATCH 2/2] simplify find_backup_name_candidate & add test --- easybuild/tools/filetools.py | 13 ++++++------- test/framework/filetools.py | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 31e20c5c5a..2f31f08a75 100644 --- a/easybuild/tools/filetools.py +++ b/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 diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 4660c0baf2..e9e5323f18 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -1355,6 +1355,28 @@ def test_move_file(self): 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 """