From 44e2ff7f839dd51d00701be00e173fcf0deaf2e0 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Wed, 27 Mar 2019 19:35:13 +0600 Subject: [PATCH] repo: change the returning value of repo.diff repo.diff now returns list of dictionaries for each file/directory change --- dvc/command/diff.py | 67 +++++++++++++++++++++++++++++++- dvc/repo/diff.py | 95 +++++++++++++++------------------------------ tests/test_diff.py | 67 +++++++++++++++++++------------- 3 files changed, 138 insertions(+), 91 deletions(-) diff --git a/dvc/command/diff.py b/dvc/command/diff.py index c48905fe5..d93147a34 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -1,5 +1,9 @@ from __future__ import unicode_literals +import humanize +import inflect + + import dvc.logger as logger from dvc.exceptions import DvcException from dvc.command.base import CmdBase @@ -7,7 +11,68 @@ from dvc.utils.collections import compact class CmdDiff(CmdBase): - def _show(self, msg): + def _print_size(self, size): + if size < 0: + change = "decreased by {}" + elif size > 0: + change = "increased by {}" + else: + change = "not changed" + natur_size = humanize.naturalsize(abs(size)) + return change.format(natur_size) + + def _show(self, diff_dct): + engine = inflect.engine() + msg = "dvc diff from {} to {}".format( + diff_dct["a_ref"], diff_dct["b_ref"] + ) + if diff_dct.get("equal"): + logger.info(msg) + return + for dct in diff_dct["diffs"]: + msg += "\n\ndiff for '{}' \n".format(dct["target"]) + if dct.get("old_file"): + msg += "-{} with md5 {}\n".format( + dct["old_file"], dct["old_checksum"] + ) + if dct.get("new_file"): + msg += "+{} with md5 {}\n".format( + dct["new_file"], dct["new_checksum"] + ) + msg += "\n" + if dct.get("is_dir"): + msg += "{} {} not changed, ".format( + dct["ident"], engine.plural("file", dct["ident"]) + ) + msg += "{} {} modified, ".format( + dct["changes"], engine.plural("file", dct["changes"]) + ) + msg += "{} {} added, ".format( + dct["new"], engine.plural("file", dct["new"]) + ) + msg += "{} {} deleted, ".format( + dct["del"], engine.plural("file", dct["del"]) + ) + msg += "size was {}".format(self._print_size(dct["size_diff"])) + else: + if ( + dct.get("old_file") + and dct.get("new_file") + and dct["size_diff"] == 0 + ): + msg += "file size was not changed" + elif dct.get("new_file"): + msg += "added file with size {}".format( + humanize.naturalsize(dct["size_diff"]) + ) + elif dct.get("old_file"): + msg += "deleted file with size {}".format( + humanize.naturalsize(abs(dct["size_diff"])) + ) + else: + msg += "file was modified, file size {}".format( + self._print_size(dct["size_diff"]) + ) logger.info(msg) def run(self): diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index cd34a8cba..bc7228ebb 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -1,8 +1,6 @@ from __future__ import unicode_literals import os -import humanize -import inflect from dvc.scm.base import FileNotInCommitError @@ -41,8 +39,7 @@ def _get_tree_changes(self, a_entries, b_entries): else: result["new"] += 1 diff_size += os.path.getsize(self.cache.local.get(b_entries[key])) - result["diff_size"] = humanize.naturalsize(abs(diff_size)) - result["diff_sign"] = "-" if diff_size < 0 else "" + result["size_diff"] = diff_size return result @@ -82,75 +79,46 @@ def _get_diff_outs(self, diff_dct): def _diff_dir(self, target, diff_dct): - engine = inflect.engine() - msg = "" + result = {"target": target} + result["is_dir"] = True a_entries, b_entries = {}, {} if not diff_dct["new_file"]: - msg += "-{} with md5 {}\n".format( - target, diff_dct["a_output"].checksum - ) + result["old_file"] = target + result["old_checksum"] = diff_dct["a_output"].checksum a_entries = _extract_dir(self, diff_dct["a_output"]) if not diff_dct["deleted_file"]: - msg += "+{} with md5 {}\n".format( - target, diff_dct["b_output"].checksum - ) + result["new_file"] = target + result["new_checksum"] = diff_dct["b_output"].checksum b_entries = _extract_dir(self, diff_dct["b_output"]) - msg += "\n" - result = _get_tree_changes(self, a_entries, b_entries) - msg += f"{result['ident']} " + engine.plural("file", result["ident"]) - msg += " not changed, " - msg += f"{result['changes']} " + engine.plural("file", result["changes"]) - msg += " modified, " - msg += f"{result['new']} " + engine.plural("file", result["new"]) - msg += " added, " - msg += f"{result['del']} " + engine.plural("file", result["del"]) - msg += " deleted, " - if result["diff_size"] == "0 Bytes": - msg += "size wasn't changed" - else: - msg += "size was " + "increased" * (result["diff_sign"] != "-") - msg += ( - "decreased" * (result["diff_sign"] == "-") - + " by " - + result["diff_size"] - ) - return msg + result.update(_get_tree_changes(self, a_entries, b_entries)) + return result def _diff_file(self, target, diff_dct): - msg = "" + result = {"target": target} size = 0 if not diff_dct["new_file"]: - msg += "-{} with md5 {}\n".format( - target, diff_dct["a_output"].checksum - ) + result["old_file"] = target + result["old_checksum"] = diff_dct["a_output"].checksum size -= os.path.getsize( self.cache.local.get(diff_dct["a_output"].checksum) ) + print("first", size) if not diff_dct["deleted_file"]: - msg += "+{} with md5 {}\n\n".format( - target, diff_dct["b_output"].checksum - ) + result["new_file"] = target + result["new_checksum"] = diff_dct["b_output"].checksum size += os.path.getsize( self.cache.local.get(diff_dct["b_output"].checksum) ) - if not diff_dct["new_file"] and not diff_dct["deleted_file"] and size == 0: - msg += "file was not changed" - elif diff_dct["new_file"]: - msg += "added file with size {}".format(humanize.naturalsize(size)) - elif diff_dct["deleted_file"]: - msg += "deleted file with size {}".format(humanize.naturalsize(size)) - else: - msg += "file was modified, file size " + "increased" * (size >= 0) - msg += "decreased" * (size < 0) + " by " + humanize.naturalsize(size) - return msg + print("second", size) + result["size_diff"] = size + return result def _diff_royal(self, target, diff_dct): - msg = "diff for '" + target + "'\n" if diff_dct["is_dir"]: - return msg + _diff_dir(self, target, diff_dct) - return msg + _diff_file(self, target, diff_dct) + return _diff_dir(self, target, diff_dct) + return _diff_file(self, target, diff_dct) def diff(self, target, a_ref=None, b_ref=None): @@ -164,23 +132,22 @@ def diff(self, target, a_ref=None, b_ref=None): Returns: string: string of output message with diff info """ + result = {} diff_dct = self.scm.get_diff_trees(a_ref, b_ref=b_ref) - msg = "dvc diff from {} to {}\n\n".format( - diff_dct["a_ref"], diff_dct["b_ref"] - ) + result["a_ref"] = diff_dct["a_ref"] + result["b_ref"] = diff_dct["b_ref"] if diff_dct["equal"]: - return msg + result["equal"] = True + return result + result["diffs"] = [] diff_outs = _get_diff_outs(self, diff_dct) if target is None: - for path in diff_outs: - info_msg = _diff_royal(self, path, diff_outs[path]) - msg += info_msg - msg += "\n\n" - msg = msg[:-2] + result["diffs"] = [ + _diff_royal(self, path, diff_outs[path]) for path in diff_outs + ] elif target in diff_outs: - info_msg = _diff_royal(self, target, diff_outs[target]) - msg += info_msg + result["diffs"] = [_diff_royal(self, target, diff_outs[target])] else: msg = "Have not found file/directory '{}' in the commits" raise FileNotInCommitError(msg.format(target)) - return msg + return result diff --git a/tests/test_diff.py b/tests/test_diff.py index f1ce66993..434e5d94a 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -32,22 +32,30 @@ class TestDiff(TestDvc): class TestDiffRepo(TestDiff): def test(self): - new_ref = self.git.head.commit - msg = self.dvc.diff(self.new_file, a_ref=self.a_ref) - test_msg = "dvc diff from {} to {}\n\n".format(new_ref, self.a_ref) - test_msg += "diff for '{}'\n".format(self.new_file) - test_msg += "-new_file with md5 25fa8325e4e0eb8180445e42558e60bd\n" - test_msg += "deleted file with size -8 Bytes" - self.assertEqual(test_msg, msg) + result = self.dvc.diff(self.new_file, a_ref=self.a_ref) + test_dct = { + "b_ref": str(self.a_ref), + "a_ref": str(self.git.head.commit), + "diffs": [ + { + "target": self.new_file, + "old_file": self.new_file, + "old_checksum": "25fa8325e4e0eb8180445e42558e60bd", + "size_diff": -8, + } + ], + } + self.assertEqual(test_dct, result) class TestDiffCmdLine(TestDiff): def test(self): - with patch("dvc.repo.Repo", config="testing") as MockRepo: - MockRepo.return_value.diff.return_value = "testing" - MockRepo.return_value.diff.return_value = "testing" - ret = main(["diff", "-t", self.new_file, str(self.a_ref)]) - self.assertEqual(ret, 0) + with patch("dvc.cli.diff.CmdDiff._show", autospec=True): + with patch("dvc.repo.Repo", config="testing") as MockRepo: + MockRepo.return_value.diff.return_value = "testing" + MockRepo.return_value.diff.return_value = "testing" + ret = main(["diff", "-t", self.new_file, str(self.a_ref)]) + self.assertEqual(ret, 0) class TestDiffDir(TestDvc): @@ -73,20 +81,27 @@ class TestDiffDir(TestDvc): class TestDiffDirRepo(TestDiffDir): def test(self): - new_ref = self.git.head.commit - msg = self.dvc.diff(self.DATA_DIR, a_ref=self.a_ref) - test_msg = "dvc diff from ".format(self.DATA_DIR) - test_msg += "{} to {}\n\n".format(new_ref, self.a_ref) - test_msg += "diff for '{}'\n".format(self.DATA_DIR) - test_msg += "-data_dir with md5 d5782f3072167ad3a53ee80b92b30718.dir\n" - test_msg += ( - "+data_dir with md5 bff5a787d16460f32e9f2e62b183b1cc.dir\n\n" - ) - test_msg += "2 files not changed, 0 files modified, 0 files added, " - test_msg += "1 file deleted, size was decreased by 30 Bytes" - self.assertEqual( - msg, test_msg, msg="msg is %s test_msg is %s" % (msg, test_msg) - ) + result = self.dvc.diff(self.DATA_DIR, a_ref=self.a_ref) + test_dct = { + "b_ref": str(self.a_ref), + "a_ref": str(self.git.head.commit), + "diffs": [ + { + "changes": 0, + "del": 1, + "ident": 2, + "moves": 0, + "new": 0, + "target": self.DATA_DIR, + "old_file": self.DATA_DIR, + "old_checksum": "d5782f3072167ad3a53ee80b92b30718.dir", + "new_file": self.DATA_DIR, + "new_checksum": "bff5a787d16460f32e9f2e62b183b1cc.dir", + "size_diff": -30, + } + ], + } + self.assertEqual(test_dct, result) class TestDiffFileNotFound(TestDiffDir):