From 4066a81413c89a6f9eb58e56bb1a47255e730a8a Mon Sep 17 00:00:00 2001 From: Mark Verbeek Date: Thu, 2 Jun 2022 16:41:44 +0200 Subject: [PATCH 1/2] refactor(runner) added bits of refactoring, and a test, removed python27 support in tox --- dirsync/run.py | 2 +- dirsync/syncer.py | 96 +++++++++++++++++----------------- tests/test_run_from_cmdline.py | 24 +++++++++ tox.ini | 2 +- 4 files changed, 75 insertions(+), 49 deletions(-) create mode 100644 tests/test_run_from_cmdline.py diff --git a/dirsync/run.py b/dirsync/run.py index 0d6a9dc..f087150 100644 --- a/dirsync/run.py +++ b/dirsync/run.py @@ -8,6 +8,7 @@ import os from .syncer import Syncer +from .options import ArgParser, USER_CFG_FILE, DEFAULT_USER_CFG def sync(sourcedir, targetdir, action, **options): @@ -22,7 +23,6 @@ def sync(sourcedir, targetdir, action, **options): def from_cmdline(): - from .options import ArgParser, USER_CFG_FILE, DEFAULT_USER_CFG # create config file if it does not exist user_cfg_file = os.path.expanduser(USER_CFG_FILE) diff --git a/dirsync/syncer.py b/dirsync/syncer.py index f96d66e..03222fd 100644 --- a/dirsync/syncer.py +++ b/dirsync/syncer.py @@ -25,9 +25,12 @@ from .options import OPTIONS from .version import __pkg_name__ +PERMISSION_777 = 1911 +PERMISSION_666 = 1638 class DCMP(object): """Dummy object for directory comparison data storage""" + def __init__(self, l, r, c): self.left_only = l self.right_only = r @@ -247,78 +250,77 @@ def _dowork(self, dir1, dir2, copyfunc=None, updatefunc=None): continue # Files & directories only in source directory - for f1 in self._dcmp.left_only: - try: - st = os.stat(os.path.join(self._dir1, f1)) - except os.error: - continue + for file, st in self.iter_files(self._dir1, self._dcmp.left_only): + if stat.S_ISREG(st.st_mode) and copyfunc: + copyfunc(file, self._dir1, self._dir2) + self._added.append(os.path.join(self._dir2, file)) - if stat.S_ISREG(st.st_mode): - if copyfunc: - copyfunc(f1, self._dir1, self._dir2) - self._added.append(os.path.join(self._dir2, f1)) elif stat.S_ISDIR(st.st_mode): - to_make = os.path.join(self._dir2, f1) + to_make = os.path.join(self._dir2, file) if not os.path.exists(to_make): os.makedirs(to_make) self._numnewdirs += 1 self._added.append(to_make) + self.update_common_files(updatefunc) + + def update_common_files(self, updatefunc): # common files/directories - for f1 in self._dcmp.common: + for file, st in self.iter_files(self._dir1, self._dcmp.common): + if stat.S_ISREG(st.st_mode) and updatefunc: + updatefunc(file, self._dir1, self._dir2) + + @staticmethod + def iter_files(base_path, files): + """Yield file and it's statistics, ignore when errors""" + for file in files: try: - st = os.stat(os.path.join(self._dir1, f1)) + yield file, os.stat(os.path.join(base_path, file)) except os.error: continue - if stat.S_ISREG(st.st_mode): - if updatefunc: - updatefunc(f1, self._dir1, self._dir2) - # nothing to do if we have a directory - - def _copy(self, filename, dir1, dir2): + def _copy(self, filename, source_dir, target_dir): """ Private function for copying a file """ - # NOTE: dir1 is source & dir2 is target if self._copyfiles: rel_path = filename.replace('\\', '/').split('/') rel_dir = '/'.join(rel_path[:-1]) filename = rel_path[-1] - dir2_root = dir2 + target_root = target_dir - dir1 = os.path.join(dir1, rel_dir) - dir2 = os.path.join(dir2, rel_dir) + source_dir = os.path.join(source_dir, rel_dir) + target_dir = os.path.join(target_dir, rel_dir) if self._verbose: self.log('Copying file %s from %s to %s' % - (filename, dir1, dir2)) + (filename, source_dir, target_dir)) try: # source to target if self._copydirection == 0 or self._copydirection == 2: - if not os.path.exists(dir2): + if not os.path.exists(target_dir): if self._forcecopy: - # 1911 = 0o777 - os.chmod(os.path.dirname(dir2_root), 1911) + os.chmod(os.path.dirname(target_root), PERMISSION_777) + try: - os.makedirs(dir2) + os.makedirs(target_dir) self._numnewdirs += 1 except OSError as e: self.log(str(e)) self._numdirsfld += 1 if self._forcecopy: - os.chmod(dir2, 1911) # 1911 = 0o777 + os.chmod(target_dir, PERMISSION_777) - sourcefile = os.path.join(dir1, filename) + sourcefile = os.path.join(source_dir, filename) try: if os.path.islink(sourcefile): os.symlink(os.readlink(sourcefile), - os.path.join(dir2, filename)) + os.path.join(target_dir, filename)) else: - shutil.copy2(sourcefile, dir2) + shutil.copy2(sourcefile, target_dir) self._numfiles += 1 except (IOError, OSError) as e: self.log(str(e)) @@ -327,28 +329,27 @@ def _copy(self, filename, dir1, dir2): if self._copydirection == 1 or self._copydirection == 2: # target to source - if not os.path.exists(dir1): + if not os.path.exists(source_dir): if self._forcecopy: - # 1911 = 0o777 - os.chmod(os.path.dirname(self.dir1_root), 1911) + os.chmod(os.path.dirname(self.dir1_root), PERMISSION_777) try: - os.makedirs(dir1) + os.makedirs(source_dir) self._numnewdirs += 1 except OSError as e: self.log(str(e)) self._numdirsfld += 1 - targetfile = os.path.abspath(os.path.join(dir1, filename)) + targetfile = os.path.abspath(os.path.join(source_dir, filename)) if self._forcecopy: - os.chmod(dir1, 1911) # 1911 = 0o777 + os.chmod(source_dir, PERMISSION_777) - sourcefile = os.path.join(dir2, filename) + sourcefile = os.path.join(target_dir, filename) try: if os.path.islink(sourcefile): os.symlink(os.readlink(sourcefile), - os.path.join(dir1, filename)) + os.path.join(source_dir, filename)) else: shutil.copy2(sourcefile, targetfile) self._numfiles += 1 @@ -371,15 +372,15 @@ def _cmptimestamps(self, filest1, filest2): else: return mtime_cmp - def _update(self, filename, dir1, dir2): + def _update(self, filename, source_dir, target_dir): """ Private function for updating a file based on last time stamp of modification or difference of content""" # NOTE: dir1 is source & dir2 is target if self._updatefiles: - file1 = os.path.join(dir1, filename) - file2 = os.path.join(dir2, filename) + file1 = os.path.join(source_dir, filename) + file2 = os.path.join(target_dir, filename) try: st1 = os.stat(file1) @@ -398,14 +399,15 @@ def _update(self, filename, dir1, dir2): # source file's modification time, or creation time. Sometimes # it so happens that a file's creation time is newer than it's # modification time! (Seen this on windows) - need_upd = (not filecmp.cmp(file1, file2, False)) if self._use_content else self._cmptimestamps(st1, st2) + need_upd = (not filecmp.cmp(file1, file2, False)) if self._use_content else self._cmptimestamps(st1, + st2) if need_upd: if self._verbose: # source to target self.log('Updating file %s' % file2) try: if self._forcecopy: - os.chmod(file2, 1638) # 1638 = 0o666 + os.chmod(file2, PERMISSION_666) try: if os.path.islink(file1): @@ -418,9 +420,9 @@ def _update(self, filename, dir1, dir2): shutil.copy2(file1, file2) self._changed.append(file2) if self._use_content: - self._numcontupdates += 1 + self._numcontupdates += 1 else: - self._numtimeupdates += 1 + self._numtimeupdates += 1 return 0 except (IOError, OSError) as e: self.log(str(e)) @@ -445,7 +447,7 @@ def _update(self, filename, dir1, dir2): self.log('Updating file %s' % file1) try: if self._forcecopy: - os.chmod(file1, 1638) # 1638 = 0o666 + os.chmod(file1, PERMISSION_666) try: if os.path.islink(file2): diff --git a/tests/test_run_from_cmdline.py b/tests/test_run_from_cmdline.py new file mode 100644 index 0000000..32baeb4 --- /dev/null +++ b/tests/test_run_from_cmdline.py @@ -0,0 +1,24 @@ +from unittest.mock import patch +from dirsync import run + + +def test_run_from_cmd_line_then_create_user_config_file_when_not_exist(): + with patch.object(run, "os") as os_mock: + os_mock.path.isfile.return_value = False + + with patch("builtins.open") as mock_file: + with patch.object(run, "sync"), patch.object(run, "ArgParser"): + run.from_cmdline() + + assert mock_file.called + + +def test_run_from_cmd_line_error_with_exit_2(): + with patch.object(run, "os") as os_mock: + os_mock.path.isfile.return_value = True + + with patch.object(run, "sync"), patch.object(run, "ArgParser") as arg_parser_mock: + arg_parser_mock.side_effect = Exception() + with patch.object(run, "sys") as sys_mock: + run.from_cmdline() + sys_mock.exit.assert_called_with(2) diff --git a/tox.ini b/tox.ini index cc35e53..28aab70 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py27, py36, py37, py38, py39 +envlist = py36, py37, py38, py39 [testenv] deps = From cc02977c3b1132bde385ea6e12482d9811c349a4 Mon Sep 17 00:00:00 2001 From: Mark Verbeek Date: Sat, 9 Jul 2022 10:53:27 +0200 Subject: [PATCH 2/2] add strict assert to test --- dirsync/options.py | 7 ++++--- tests/test_run_from_cmdline.py | 10 +++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/dirsync/options.py b/dirsync/options.py index 7f176c4..80746e7 100644 --- a/dirsync/options.py +++ b/dirsync/options.py @@ -18,7 +18,6 @@ from six import string_types - from .version import __pkg_name__ __all__ = ['USER_CFG_FILE', 'DEFAULT_USER_CFG', 'OPTIONS', 'ArgParser'] @@ -29,7 +28,6 @@ action = sync """ % __pkg_name__ - options = ( ('verbose', (('-v',), dict( action='store_true', @@ -112,7 +110,6 @@ ))), ) - OPTIONS = OrderedDict(options) @@ -189,3 +186,7 @@ def load_cfg(self, src_dir): defaults[name] = newdef self.set_defaults(**defaults) + + +class InvalidArgumentError(Exception): + """Custom exception for argument errors.""" diff --git a/tests/test_run_from_cmdline.py b/tests/test_run_from_cmdline.py index 32baeb4..764268f 100644 --- a/tests/test_run_from_cmdline.py +++ b/tests/test_run_from_cmdline.py @@ -1,16 +1,20 @@ -from unittest.mock import patch +from unittest.mock import patch, mock_open from dirsync import run +from dirsync import options def test_run_from_cmd_line_then_create_user_config_file_when_not_exist(): with patch.object(run, "os") as os_mock: os_mock.path.isfile.return_value = False - with patch("builtins.open") as mock_file: + mock_file = mock_open() + with patch("builtins.open", mock_file): with patch.object(run, "sync"), patch.object(run, "ArgParser"): run.from_cmdline() - assert mock_file.called + assert mock_file.called + handle = mock_file() + handle.write.assert_called_once_with(options.DEFAULT_USER_CFG) def test_run_from_cmd_line_error_with_exit_2():