From 874013e617cbf553a258fdf6f044f76bbe160aa4 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Fri, 19 Apr 2019 01:51:11 -0500 Subject: [PATCH 1/7] Document dvc.scm.git::Git::_get_diff_trees and shorten commit hashes in dvc.scm.git::Git::get_diff_trees --- dvc/scm/git/__init__.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index c6db51da5c..6dc8837612 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -247,7 +247,16 @@ def get_tree(self, rev): return GitTree(self.git, rev) def _get_diff_trees(self, a_ref, b_ref): + """Private method for getting the trees and commit hashes of 2 git + references + Args: + a_ref(str) - git reference + b_ref(str) - second git reference. If None, uses HEAD + + Returns: + tuple - tuple with elements: (trees, commits) + """ from gitdb.exc import BadObject, BadName trees = {DIFF_A_TREE: None, DIFF_B_TREE: None} @@ -282,8 +291,8 @@ def get_diff_trees(self, a_ref, b_ref=None): """ diff_dct = {DIFF_EQUAL: False} trees, commit_refs = self._get_diff_trees(a_ref, b_ref) - diff_dct[DIFF_A_REF] = commit_refs[0] - diff_dct[DIFF_B_REF] = commit_refs[1] + diff_dct[DIFF_A_REF] = commit_refs[0][:7] + diff_dct[DIFF_B_REF] = commit_refs[1][:7] if commit_refs[0] == commit_refs[1]: diff_dct[DIFF_EQUAL] = True return diff_dct From 3d2dc83eb12ff93150d24b49d2828cc19d9f76f4 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Thu, 25 Apr 2019 00:05:40 -0400 Subject: [PATCH 2/7] diff: Changes to dvc.scm.git so `dvc diff` outputs short SHAs - Docstring for Git class constructor - Renames self.git for self.repo throughout module (involves several test changes) - Better doc and logical refactor in Git._get_diff_trees - `dvc diff` now outputs short SHA strings (Git._get_diff_trees) - hard coded to 7 length str (involves some test changes in tests/func/test_diff.py) For #1902 --- dvc/scm/git/__init__.py | 74 +++++++++++++++++++------------------- tests/func/test_diff.py | 24 ++++++------- tests/func/test_metrics.py | 2 +- tests/func/test_repo.py | 2 +- tests/func/test_scm.py | 2 +- 5 files changed, 53 insertions(+), 51 deletions(-) diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 6dc8837612..774ae3828f 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -33,13 +33,16 @@ class Git(Base): GIT_DIR = ".git" def __init__(self, root_dir=os.curdir, repo=None): + """Git class constructor. + Requires `Repo` class from `git` module (from gitpython package). + """ super(Git, self).__init__(root_dir, repo=repo) - import git + from git import Repo from git.exc import InvalidGitRepositoryError try: - self.git = git.Repo(root_dir) + self.repo = Repo(root_dir) except InvalidGitRepositoryError: msg = "{} is not a git repository" raise SCMError(msg.format(root_dir)) @@ -48,7 +51,7 @@ def __init__(self, root_dir=os.curdir, repo=None): # http://pyinstaller.readthedocs.io/en/stable/runtime-information.html env = fix_env(None) libpath = env.get("LD_LIBRARY_PATH", None) - self.git.git.update_environment(LD_LIBRARY_PATH=libpath) + self.repo.git.update_environment(LD_LIBRARY_PATH=libpath) self.ignored_paths = [] self.files_to_track = [] @@ -67,7 +70,7 @@ def _get_git_dir(root_dir): @property def dir(self): - return self.git.git_dir + return self.repo.git_dir @property def ignore_file(self): @@ -155,7 +158,7 @@ def add(self, paths): # NOTE: GitPython is not currently able to handle index version >= 3. # See https://github.com/iterative/dvc/issues/610 for more details. try: - self.git.index.add(paths) + self.repo.index.add(paths) except AssertionError: msg = ( "failed to add '{}' to git. You can add those files" @@ -167,41 +170,41 @@ def add(self, paths): logger.exception(msg) def commit(self, msg): - self.git.index.commit(msg) + self.repo.index.commit(msg) def checkout(self, branch, create_new=False): if create_new: - self.git.git.checkout("HEAD", b=branch) + self.repo.git.checkout("HEAD", b=branch) else: - self.git.git.checkout(branch) + self.repo.git.checkout(branch) def branch(self, branch): - self.git.git.branch(branch) + self.repo.git.branch(branch) def tag(self, tag): - self.git.git.tag(tag) + self.repo.git.tag(tag) def untracked_files(self): - files = self.git.untracked_files - return [os.path.join(self.git.working_dir, fname) for fname in files] + files = self.repo.untracked_files + return [os.path.join(self.repo.working_dir, fname) for fname in files] def is_tracked(self, path): - # it is equivalent to `bool(self.git.git.ls_files(path))` by + # it is equivalent to `bool(self.repo.git.ls_files(path))` by # functionality, but ls_files fails on unicode filenames path = os.path.relpath(path, self.root_dir) - return path in [i[0] for i in self.git.index.entries] + return path in [i[0] for i in self.repo.index.entries] def is_dirty(self): - return self.git.is_dirty() + return self.repo.is_dirty() def active_branch(self): - return self.git.active_branch.name + return self.repo.active_branch.name def list_branches(self): - return [h.name for h in self.git.heads] + return [h.name for h in self.repo.heads] def list_tags(self): - return [t.name for t in self.git.tags] + return [t.name for t in self.repo.tags] def _install_hook(self, name, cmd): hook = os.path.join(self.root_dir, self.GIT_DIR, "hooks", name) @@ -244,11 +247,12 @@ def belongs_to_scm(self, path): return basename == self.ignore_file or Git.GIT_DIR in path_parts def get_tree(self, rev): - return GitTree(self.git, rev) + return GitTree(self.repo, rev) def _get_diff_trees(self, a_ref, b_ref): """Private method for getting the trees and commit hashes of 2 git - references + references. + Requires `gitdb` module (from gitpython package). Args: a_ref(str) - git reference @@ -261,17 +265,15 @@ def _get_diff_trees(self, a_ref, b_ref): trees = {DIFF_A_TREE: None, DIFF_B_TREE: None} commits = [] + if b_ref is None: + b_ref = self.repo.head.commit try: - if b_ref is not None: - a_commit = self.git.commit(a_ref) - b_commit = self.git.commit(b_ref) - commits.append(str(a_commit)) - commits.append(str(b_commit)) - else: - a_commit = self.git.commit(a_ref) - b_commit = self.git.head.commit - commits.append(str(a_commit)) - commits.append(str(b_commit)) + a_commit = self.repo.commit(a_ref) + b_commit = self.repo.commit(b_ref) + # See https://gitpython.readthedocs.io + # /en/2.1.11/reference.html#git.objects.base.Object.__str__ + commits.append(str(a_commit)[:7]) + commits.append(str(b_commit)[:7]) trees[DIFF_A_TREE] = self.get_tree(commits[0]) trees[DIFF_B_TREE] = self.get_tree(commits[1]) except (BadName, BadObject) as e: @@ -279,8 +281,8 @@ def _get_diff_trees(self, a_ref, b_ref): return trees, commits def get_diff_trees(self, a_ref, b_ref=None): - """Method for getting two repo trees between two git tag commits - returns the dvc hash names of changed file/directory + """Method for getting two repo trees between two git tag commits. + Returns the dvc hash names of changed file/directory Args: a_ref(str) - git reference @@ -290,10 +292,10 @@ def get_diff_trees(self, a_ref, b_ref=None): dict - dictionary with keys: (a_tree, b_tree, a_ref, b_ref, equal) """ diff_dct = {DIFF_EQUAL: False} - trees, commit_refs = self._get_diff_trees(a_ref, b_ref) - diff_dct[DIFF_A_REF] = commit_refs[0][:7] - diff_dct[DIFF_B_REF] = commit_refs[1][:7] - if commit_refs[0] == commit_refs[1]: + trees, commits = self._get_diff_trees(a_ref, b_ref) + diff_dct[DIFF_A_REF] = commits[0] + diff_dct[DIFF_B_REF] = commits[1] + if commits[0] == commits[1]: diff_dct[DIFF_EQUAL] = True return diff_dct diff_dct[DIFF_A_TREE] = trees[DIFF_A_TREE] diff --git a/tests/func/test_diff.py b/tests/func/test_diff.py index 8407fc42ef..4ce68056ee 100644 --- a/tests/func/test_diff.py +++ b/tests/func/test_diff.py @@ -33,8 +33,8 @@ def setUp(self): self.git.index.add([self.new_file + ".dvc"]) self.git.index.commit("adds new_file") self.test_dct = { - diff.DIFF_A_REF: str(self.a_ref), - diff.DIFF_B_REF: str(self.git.head.commit), + diff.DIFF_A_REF: str(self.a_ref)[:7], + diff.DIFF_B_REF: str(self.git.head.commit)[:7], diff.DIFF_LIST: [ { diff.DIFF_TARGET: self.new_file, @@ -48,8 +48,8 @@ def setUp(self): def test(self): out = self.dvc.scm.get_diff_trees(self.a_ref) self.assertFalse(out[diff.DIFF_EQUAL]) - self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF]) - self.assertEqual(str(self.git.head.commit), out[diff.DIFF_B_REF]) + self.assertEqual(str(self.a_ref)[:7], out[diff.DIFF_A_REF]) + self.assertEqual(str(self.git.head.commit)[:7], out[diff.DIFF_B_REF]) class TestDiffRepo(TestDiff): @@ -97,7 +97,7 @@ def setUp(self): self.dvc.add(self.DATA_DIR) self.git.index.add([self.DATA_DIR + ".dvc"]) self.git.index.commit("adds data_dir") - self.a_ref = str(self.dvc.scm.git.head.commit) + self.a_ref = str(self.dvc.scm.repo.head.commit)[:7] self.old_checksum = _get_checksum(self.dvc, self.DATA_DIR) self.new_file = os.path.join(self.DATA_SUB_DIR, diff.DIFF_NEW_FILE) self.create(self.new_file, self.new_file) @@ -109,8 +109,8 @@ def setUp(self): def test(self): out = self.dvc.scm.get_diff_trees(self.a_ref) self.assertFalse(out[diff.DIFF_EQUAL]) - self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF]) - self.assertEqual(str(self.git.head.commit), out[diff.DIFF_B_REF]) + self.assertEqual(str(self.a_ref)[:7], out[diff.DIFF_A_REF]) + self.assertEqual(str(self.git.head.commit)[:7], out[diff.DIFF_B_REF]) class TestDiffDirRepo(TestDiffDir): @@ -119,8 +119,8 @@ class TestDiffDirRepo(TestDiffDir): def test(self): result = self.dvc.diff(self.a_ref, target=self.DATA_DIR) test_dct = { - diff.DIFF_A_REF: str(self.a_ref), - diff.DIFF_B_REF: str(self.git.head.commit), + diff.DIFF_A_REF: str(self.a_ref)[:7], + diff.DIFF_B_REF: str(self.git.head.commit)[:7], diff.DIFF_LIST: [ { diff.DIFF_CHANGE: 0, @@ -149,7 +149,7 @@ def setUp(self): self.b_ref = self.a_ref self.new_checksum = self.old_checksum - self.a_ref = str(self.dvc.scm.git.head.commit) + self.a_ref = str(self.dvc.scm.repo.head.commit) self.old_checksum = _get_checksum(self.dvc, self.DATA_DIR) def test(self): @@ -157,8 +157,8 @@ def test(self): self.a_ref, b_ref=self.b_ref, target=self.DATA_DIR ) test_dct = { - diff.DIFF_A_REF: str(self.a_ref), - diff.DIFF_B_REF: str(self.b_ref), + diff.DIFF_A_REF: str(self.a_ref)[:7], + diff.DIFF_B_REF: str(self.b_ref)[:7], diff.DIFF_LIST: [ { diff.DIFF_CHANGE: 0, diff --git a/tests/func/test_metrics.py b/tests/func/test_metrics.py index 90370264a9..ef968c8505 100644 --- a/tests/func/test_metrics.py +++ b/tests/func/test_metrics.py @@ -605,7 +605,7 @@ def _test_metrics(self, func): # TestDvc currently is based on TestGit, so it is safe to use # scm.git for now - self.dvc.scm.git.git.clean("-fd") + self.dvc.scm.repo.git.clean("-fd") self.dvc = DvcRepo(".") diff --git a/tests/func/test_repo.py b/tests/func/test_repo.py index 43aced7c4d..964a8a1e60 100644 --- a/tests/func/test_repo.py +++ b/tests/func/test_repo.py @@ -29,7 +29,7 @@ def setUp(self): def _check(self, branch, target, with_deps, expected): if branch: - self.dvc.tree = GitTree(self.dvc.scm.git, branch) + self.dvc.tree = GitTree(self.dvc.scm.repo, branch) else: self.dvc.tree = WorkingTree() result = self.dvc.collect(target + ".dvc", with_deps=with_deps) diff --git a/tests/func/test_scm.py b/tests/func/test_scm.py index 885b9c613e..9c7839cadc 100644 --- a/tests/func/test_scm.py +++ b/tests/func/test_scm.py @@ -41,7 +41,7 @@ def test_is_tracked(self): G.commit("add") self.assertTrue(G.is_tracked(foo)) self.assertTrue(G.is_tracked(self.FOO)) - G.git.index.remove([self.FOO], working_tree=True) + G.repo.index.remove([self.FOO], working_tree=True) self.assertFalse(G.is_tracked(foo)) self.assertFalse(G.is_tracked(self.FOO)) self.assertFalse(G.is_tracked("not-existing-file")) From 98f461d3cfa87160ce22af361bab0fd957c094d8 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Wed, 8 May 2019 17:03:15 -0400 Subject: [PATCH 3/7] Undoes "Renames self.git for self.repo throughout module" introduced in 3d2dc83. --- dvc/scm/git/__init__.py | 44 +++++++++++++++++++------------------- tests/func/test_diff.py | 4 ++-- tests/func/test_metrics.py | 2 +- tests/func/test_repo.py | 2 +- tests/func/test_scm.py | 2 +- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 85bd7debf4..53e8e71368 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -38,11 +38,11 @@ def __init__(self, root_dir=os.curdir, repo=None): """ super(Git, self).__init__(root_dir, repo=repo) - from git import Repo + import git from git.exc import InvalidGitRepositoryError try: - self.repo = Repo(root_dir) + self.git = git.Repo(root_dir) except InvalidGitRepositoryError: msg = "{} is not a git repository" raise SCMError(msg.format(root_dir)) @@ -51,7 +51,7 @@ def __init__(self, root_dir=os.curdir, repo=None): # http://pyinstaller.readthedocs.io/en/stable/runtime-information.html env = fix_env(None) libpath = env.get("LD_LIBRARY_PATH", None) - self.repo.git.update_environment(LD_LIBRARY_PATH=libpath) + self.git.git.update_environment(LD_LIBRARY_PATH=libpath) self.ignored_paths = [] self.files_to_track = [] @@ -70,7 +70,7 @@ def _get_git_dir(root_dir): @property def dir(self): - return self.repo.git_dir + return self.git.git_dir @property def ignore_file(self): @@ -158,7 +158,7 @@ def add(self, paths): # NOTE: GitPython is not currently able to handle index version >= 3. # See https://github.com/iterative/dvc/issues/610 for more details. try: - self.repo.index.add(paths) + self.git.index.add(paths) except AssertionError: msg = ( "failed to add '{}' to git. You can add those files" @@ -170,41 +170,41 @@ def add(self, paths): logger.exception(msg) def commit(self, msg): - self.repo.index.commit(msg) + self.git.index.commit(msg) def checkout(self, branch, create_new=False): if create_new: - self.repo.git.checkout("HEAD", b=branch) + self.git.git.checkout("HEAD", b=branch) else: - self.repo.git.checkout(branch) + self.git.git.checkout(branch) def branch(self, branch): - self.repo.git.branch(branch) + self.git.git.branch(branch) def tag(self, tag): - self.repo.git.tag(tag) + self.git.git.tag(tag) def untracked_files(self): - files = self.repo.untracked_files - return [os.path.join(self.repo.working_dir, fname) for fname in files] + files = self.git.untracked_files + return [os.path.join(self.git.working_dir, fname) for fname in files] def is_tracked(self, path): - # it is equivalent to `bool(self.repo.git.ls_files(path))` by + # it is equivalent to `bool(self.git.git.ls_files(path))` by # functionality, but ls_files fails on unicode filenames path = os.path.relpath(path, self.root_dir) - return path in [i[0] for i in self.repo.index.entries] + return path in [i[0] for i in self.git.index.entries] def is_dirty(self): - return self.repo.is_dirty() + return self.git.is_dirty() def active_branch(self): - return self.repo.active_branch.name + return self.git.active_branch.name def list_branches(self): - return [h.name for h in self.repo.heads] + return [h.name for h in self.git.heads] def list_tags(self): - return [t.name for t in self.repo.tags] + return [t.name for t in self.git.tags] def _install_hook(self, name, cmd): hook = os.path.join(self.root_dir, self.GIT_DIR, "hooks", name) @@ -248,7 +248,7 @@ def belongs_to_scm(self, path): return basename == self.ignore_file or Git.GIT_DIR in path_parts def get_tree(self, rev): - return GitTree(self.repo, rev) + return GitTree(self.git, rev) def _get_diff_trees(self, a_ref, b_ref): """Private method for getting the trees and commit hashes of 2 git @@ -267,10 +267,10 @@ def _get_diff_trees(self, a_ref, b_ref): trees = {DIFF_A_TREE: None, DIFF_B_TREE: None} commits = [] if b_ref is None: - b_ref = self.repo.head.commit + b_ref = self.git.head.commit try: - a_commit = self.repo.commit(a_ref) - b_commit = self.repo.commit(b_ref) + a_commit = self.git.commit(a_ref) + b_commit = self.git.commit(b_ref) # See https://gitpython.readthedocs.io # /en/2.1.11/reference.html#git.objects.base.Object.__str__ commits.append(str(a_commit)[:7]) diff --git a/tests/func/test_diff.py b/tests/func/test_diff.py index 4ce68056ee..302bd03104 100644 --- a/tests/func/test_diff.py +++ b/tests/func/test_diff.py @@ -97,7 +97,7 @@ def setUp(self): self.dvc.add(self.DATA_DIR) self.git.index.add([self.DATA_DIR + ".dvc"]) self.git.index.commit("adds data_dir") - self.a_ref = str(self.dvc.scm.repo.head.commit)[:7] + self.a_ref = str(self.dvc.scm.git.head.commit)[:7] self.old_checksum = _get_checksum(self.dvc, self.DATA_DIR) self.new_file = os.path.join(self.DATA_SUB_DIR, diff.DIFF_NEW_FILE) self.create(self.new_file, self.new_file) @@ -149,7 +149,7 @@ def setUp(self): self.b_ref = self.a_ref self.new_checksum = self.old_checksum - self.a_ref = str(self.dvc.scm.repo.head.commit) + self.a_ref = str(self.dvc.scm.git.head.commit) self.old_checksum = _get_checksum(self.dvc, self.DATA_DIR) def test(self): diff --git a/tests/func/test_metrics.py b/tests/func/test_metrics.py index 31f171ddd7..9c1c55a061 100644 --- a/tests/func/test_metrics.py +++ b/tests/func/test_metrics.py @@ -691,7 +691,7 @@ def _test_metrics(self, func): # TestDvc currently is based on TestGit, so it is safe to use # scm.git for now - self.dvc.scm.repo.git.clean("-fd") + self.dvc.scm.git.git.clean("-fd") self.dvc = DvcRepo(".") diff --git a/tests/func/test_repo.py b/tests/func/test_repo.py index 964a8a1e60..43aced7c4d 100644 --- a/tests/func/test_repo.py +++ b/tests/func/test_repo.py @@ -29,7 +29,7 @@ def setUp(self): def _check(self, branch, target, with_deps, expected): if branch: - self.dvc.tree = GitTree(self.dvc.scm.repo, branch) + self.dvc.tree = GitTree(self.dvc.scm.git, branch) else: self.dvc.tree = WorkingTree() result = self.dvc.collect(target + ".dvc", with_deps=with_deps) diff --git a/tests/func/test_scm.py b/tests/func/test_scm.py index 9c7839cadc..885b9c613e 100644 --- a/tests/func/test_scm.py +++ b/tests/func/test_scm.py @@ -41,7 +41,7 @@ def test_is_tracked(self): G.commit("add") self.assertTrue(G.is_tracked(foo)) self.assertTrue(G.is_tracked(self.FOO)) - G.repo.index.remove([self.FOO], working_tree=True) + G.git.index.remove([self.FOO], working_tree=True) self.assertFalse(G.is_tracked(foo)) self.assertFalse(G.is_tracked(self.FOO)) self.assertFalse(G.is_tracked("not-existing-file")) From 9516e70a17697b05d414f12ea2e8970e0c9c4b13 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Thu, 9 May 2019 12:24:03 -0400 Subject: [PATCH 4/7] diff: `dvc diff` uses code for `git rev_parse --short` instead of hard `[:7]` (for short commit hashes). - Tests updated too. Per https://github.com/iterative/dvc/pull/1906#discussion_r278510340 --- dvc/scm/git/__init__.py | 8 ++++---- tests/func/test_diff.py | 34 ++++++++++++++++++++++------------ 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 53e8e71368..85b66db81b 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -269,12 +269,12 @@ def _get_diff_trees(self, a_ref, b_ref): if b_ref is None: b_ref = self.git.head.commit try: - a_commit = self.git.commit(a_ref) - b_commit = self.git.commit(b_ref) + a_commit = self.git.git.rev_parse(a_ref, short=True) + b_commit = self.git.git.rev_parse(b_ref, short=True) # See https://gitpython.readthedocs.io # /en/2.1.11/reference.html#git.objects.base.Object.__str__ - commits.append(str(a_commit)[:7]) - commits.append(str(b_commit)[:7]) + commits.append(a_commit) + commits.append(b_commit) trees[DIFF_A_TREE] = self.get_tree(commits[0]) trees[DIFF_B_TREE] = self.get_tree(commits[1]) except (BadName, BadObject) as e: diff --git a/tests/func/test_diff.py b/tests/func/test_diff.py index 302bd03104..809165ed68 100644 --- a/tests/func/test_diff.py +++ b/tests/func/test_diff.py @@ -28,13 +28,15 @@ def setUp(self): self.new_file = "new_test_file" self.create(self.new_file, self.new_file) self.dvc.add(self.new_file) - self.a_ref = self.git.head.commit + self.a_ref = self.git.git.rev_parse(self.git.head.commit, short=True) self.new_checksum = _get_checksum(self.dvc, self.new_file) self.git.index.add([self.new_file + ".dvc"]) self.git.index.commit("adds new_file") self.test_dct = { - diff.DIFF_A_REF: str(self.a_ref)[:7], - diff.DIFF_B_REF: str(self.git.head.commit)[:7], + diff.DIFF_A_REF: str(self.a_ref), + diff.DIFF_B_REF: str(self.git.git.rev_parse( + self.git.head.commit, + short=True)), diff.DIFF_LIST: [ { diff.DIFF_TARGET: self.new_file, @@ -48,8 +50,10 @@ def setUp(self): def test(self): out = self.dvc.scm.get_diff_trees(self.a_ref) self.assertFalse(out[diff.DIFF_EQUAL]) - self.assertEqual(str(self.a_ref)[:7], out[diff.DIFF_A_REF]) - self.assertEqual(str(self.git.head.commit)[:7], out[diff.DIFF_B_REF]) + self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF]) + self.assertEqual( + self.git.git.rev_parse(self.git.head.commit, short=True), + out[diff.DIFF_B_REF]) class TestDiffRepo(TestDiff): @@ -97,7 +101,9 @@ def setUp(self): self.dvc.add(self.DATA_DIR) self.git.index.add([self.DATA_DIR + ".dvc"]) self.git.index.commit("adds data_dir") - self.a_ref = str(self.dvc.scm.git.head.commit)[:7] + self.a_ref = self.git.git.rev_parse( + self.dvc.scm.git.head.commit, + short=True) self.old_checksum = _get_checksum(self.dvc, self.DATA_DIR) self.new_file = os.path.join(self.DATA_SUB_DIR, diff.DIFF_NEW_FILE) self.create(self.new_file, self.new_file) @@ -109,8 +115,10 @@ def setUp(self): def test(self): out = self.dvc.scm.get_diff_trees(self.a_ref) self.assertFalse(out[diff.DIFF_EQUAL]) - self.assertEqual(str(self.a_ref)[:7], out[diff.DIFF_A_REF]) - self.assertEqual(str(self.git.head.commit)[:7], out[diff.DIFF_B_REF]) + self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF]) + self.assertEqual( + str(self.git.git.rev_parse(self.git.head.commit, short=True)), + out[diff.DIFF_B_REF]) class TestDiffDirRepo(TestDiffDir): @@ -119,8 +127,10 @@ class TestDiffDirRepo(TestDiffDir): def test(self): result = self.dvc.diff(self.a_ref, target=self.DATA_DIR) test_dct = { - diff.DIFF_A_REF: str(self.a_ref)[:7], - diff.DIFF_B_REF: str(self.git.head.commit)[:7], + diff.DIFF_A_REF: self.git.git.rev_parse(self.a_ref, short=True), + diff.DIFF_B_REF: self.git.git.rev_parse( + self.git.head.commit, + short=True), diff.DIFF_LIST: [ { diff.DIFF_CHANGE: 0, @@ -157,8 +167,8 @@ def test(self): self.a_ref, b_ref=self.b_ref, target=self.DATA_DIR ) test_dct = { - diff.DIFF_A_REF: str(self.a_ref)[:7], - diff.DIFF_B_REF: str(self.b_ref)[:7], + diff.DIFF_A_REF: self.git.git.rev_parse(self.a_ref, short=True), + diff.DIFF_B_REF: self.git.git.rev_parse(self.b_ref, short=True), diff.DIFF_LIST: [ { diff.DIFF_CHANGE: 0, From d5daa97d258d68eec2ea9f28dcf264cdba2d9513 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Thu, 9 May 2019 12:34:03 -0400 Subject: [PATCH 5/7] tests: Reformatted with `black tests/func/test_diff.py` --- tests/func/test_diff.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/func/test_diff.py b/tests/func/test_diff.py index 809165ed68..e69184ee88 100644 --- a/tests/func/test_diff.py +++ b/tests/func/test_diff.py @@ -34,9 +34,9 @@ def setUp(self): self.git.index.commit("adds new_file") self.test_dct = { diff.DIFF_A_REF: str(self.a_ref), - diff.DIFF_B_REF: str(self.git.git.rev_parse( - self.git.head.commit, - short=True)), + diff.DIFF_B_REF: str( + self.git.git.rev_parse(self.git.head.commit, short=True) + ), diff.DIFF_LIST: [ { diff.DIFF_TARGET: self.new_file, @@ -53,7 +53,8 @@ def test(self): self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF]) self.assertEqual( self.git.git.rev_parse(self.git.head.commit, short=True), - out[diff.DIFF_B_REF]) + out[diff.DIFF_B_REF], + ) class TestDiffRepo(TestDiff): @@ -102,8 +103,8 @@ def setUp(self): self.git.index.add([self.DATA_DIR + ".dvc"]) self.git.index.commit("adds data_dir") self.a_ref = self.git.git.rev_parse( - self.dvc.scm.git.head.commit, - short=True) + self.dvc.scm.git.head.commit, short=True + ) self.old_checksum = _get_checksum(self.dvc, self.DATA_DIR) self.new_file = os.path.join(self.DATA_SUB_DIR, diff.DIFF_NEW_FILE) self.create(self.new_file, self.new_file) @@ -118,7 +119,8 @@ def test(self): self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF]) self.assertEqual( str(self.git.git.rev_parse(self.git.head.commit, short=True)), - out[diff.DIFF_B_REF]) + out[diff.DIFF_B_REF], + ) class TestDiffDirRepo(TestDiffDir): @@ -129,8 +131,8 @@ def test(self): test_dct = { diff.DIFF_A_REF: self.git.git.rev_parse(self.a_ref, short=True), diff.DIFF_B_REF: self.git.git.rev_parse( - self.git.head.commit, - short=True), + self.git.head.commit, short=True + ), diff.DIFF_LIST: [ { diff.DIFF_CHANGE: 0, From 6e04c3c4bff090ed70a309875eb1c9c2fac68ff4 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Fri, 10 May 2019 03:24:58 -0500 Subject: [PATCH 6/7] tests: Remove uneccessary `str()` casting form `dvc diff` tests (and reformat) --- tests/func/test_diff.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/func/test_diff.py b/tests/func/test_diff.py index e69184ee88..ee2fd5f5e7 100644 --- a/tests/func/test_diff.py +++ b/tests/func/test_diff.py @@ -33,9 +33,9 @@ def setUp(self): self.git.index.add([self.new_file + ".dvc"]) self.git.index.commit("adds new_file") self.test_dct = { - diff.DIFF_A_REF: str(self.a_ref), - diff.DIFF_B_REF: str( - self.git.git.rev_parse(self.git.head.commit, short=True) + diff.DIFF_A_REF: self.a_ref, + diff.DIFF_B_REF: self.git.git.rev_parse( + self.git.head.commit, short=True ), diff.DIFF_LIST: [ { @@ -50,7 +50,7 @@ def setUp(self): def test(self): out = self.dvc.scm.get_diff_trees(self.a_ref) self.assertFalse(out[diff.DIFF_EQUAL]) - self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF]) + self.assertEqual(self.a_ref, out[diff.DIFF_A_REF]) self.assertEqual( self.git.git.rev_parse(self.git.head.commit, short=True), out[diff.DIFF_B_REF], @@ -68,7 +68,7 @@ def test(self): 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" - ret = main(["diff", "-t", self.new_file, str(self.a_ref)]) + ret = main(["diff", "-t", self.new_file, self.a_ref]) self.assertEqual(ret, 0) @@ -116,9 +116,9 @@ def setUp(self): def test(self): out = self.dvc.scm.get_diff_trees(self.a_ref) self.assertFalse(out[diff.DIFF_EQUAL]) - self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF]) + self.assertEqual(self.a_ref, out[diff.DIFF_A_REF]) self.assertEqual( - str(self.git.git.rev_parse(self.git.head.commit, short=True)), + self.git.git.rev_parse(self.git.head.commit, short=True), out[diff.DIFF_B_REF], ) From 004185e6042ee295b522eff27203a09bacd847a1 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Mon, 13 May 2019 18:39:43 -0500 Subject: [PATCH 7/7] config: Updates usage output with better terminology To match this change: https://github.com/iterative/dvc.org/pull/331/files#diff-efa51f4bb4922f00bd8afd1ad364ca9dR10 --- dvc/command/config.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dvc/command/config.py b/dvc/command/config.py index f62ac5d7dc..003bc8798b 100644 --- a/dvc/command/config.py +++ b/dvc/command/config.py @@ -98,7 +98,7 @@ def run(self): def add_parser(subparsers, parent_parser): - CONFIG_HELP = "Get or set config options." + CONFIG_HELP = "Get or set config settings." config_parser = subparsers.add_parser( "config", @@ -114,8 +114,8 @@ def add_parser(subparsers, parent_parser): action="store_true", help="Unset option.", ) - config_parser.add_argument("name", help="Option name.") + config_parser.add_argument("name", help="Setting name.") config_parser.add_argument( - "value", nargs="?", default=None, help="Option value." + "value", nargs="?", default=None, help="Setting value." ) config_parser.set_defaults(func=CmdConfig)