Merge pull request #702 from yarikoptic/bf-happy-travis

BF (codename "happy travis"): trying to address lints etc to make Travis green again

6 files changed, 75 insertions, 67 deletions

diff --git a/git/test/lib/helper.py b/git/test/lib/helper.py
index 729c76a4..ff337f29 100644
--- a/git/test/lib/helper.py
+++ b/git/test/lib/helper.py
@@ -139,7 +139,7 @@ def with_rw_repo(working_tree_ref, bare=False):
             try:
                 try:
                     return func(self, rw_repo)
-                except:
+                except:  # noqa E722
                     log.info("Keeping repo after failure: %s", repo_dir)
                     repo_dir = None
                     raise
@@ -227,7 +227,7 @@ def with_rw_and_rw_remote_repo(working_tree_ref):
     Same as with_rw_repo, but also provides a writable remote repository from which the
     rw_repo has been forked as well as a handle for a git-daemon that may be started to
     run the remote_repo.
-    The remote repository was cloned as bare repository from the rorepo, whereas
+    The remote repository was cloned as bare repository from the ro repo, whereas
     the rw repo has a working tree and was cloned from the remote repository.
 
     remote_repo has two remotes: origin and daemon_origin. One uses a local url,
@@ -296,7 +296,7 @@ def with_rw_and_rw_remote_repo(working_tree_ref):
                     with cwd(rw_repo.working_dir):
                         try:
                             return func(self, rw_repo, rw_daemon_repo)
-                        except:
+                        except:  # noqa E722
                             log.info("Keeping repos after failure: \n  rw_repo_dir: %s \n  rw_daemon_repo_dir: %s",
                                      rw_repo_dir, rw_daemon_repo_dir)
                             rw_repo_dir = rw_daemon_repo_dir = None
diff --git a/git/test/test_commit.py b/git/test/test_commit.py
index fbb1c244..cd6c5d5f 100644
--- a/git/test/test_commit.py
+++ b/git/test/test_commit.py
@@ -169,8 +169,7 @@ class TestCommit(TestBase):
         # at some point, both iterations should stop
         self.assertEqual(list(bfirst)[-1], first)
         stoptraverse = self.rorepo.commit("254d04aa3180eb8b8daf7b7ff25f010cd69b4e7d").traverse(as_edge=True)
-        l = list(stoptraverse)
-        self.assertEqual(len(l[0]), 2)
+        self.assertEqual(len(next(stoptraverse)), 2)
 
         # ignore self
         self.assertEqual(next(start.traverse(ignore_self=False)), start)
diff --git a/git/test/test_git.py b/git/test/test_git.py
index 0a0d7ef7..059f90c0 100644
--- a/git/test/test_git.py
+++ b/git/test/test_git.py
@@ -37,6 +37,8 @@ try:
 except ImportError:
     import mock
 
+from git.compat import is_win
+
 
 class TestGit(TestBase):
 
@@ -177,7 +179,8 @@ class TestGit(TestBase):
         self.assertRaises(GitCommandNotFound, refresh, "yada")
 
         # test a good path refresh
-        path = os.popen("which git").read().strip()
+        which_cmd = "where" if is_win else "which"
+        path = os.popen("{0} git".format(which_cmd)).read().strip().split('\n')[0]
         refresh(path)
 
     def test_options_are_passed_to_git(self):
diff --git a/git/test/test_index.py b/git/test/test_index.py
index cf746140..757bec9f 100644
--- a/git/test/test_index.py
+++ b/git/test/test_index.py
@@ -54,6 +54,22 @@ from gitdb.base import IStream
 import os.path as osp
 from git.cmd import Git
 
+HOOKS_SHEBANG = "#!/usr/bin/env sh\n"
+
+
+@skipIf(HIDE_WINDOWS_KNOWN_ERRORS, "TODO: fix hooks execution on Windows: #703")
+def _make_hook(git_dir, name, content, make_exec=True):
+    """A helper to create a hook"""
+    hp = hook_path(name, git_dir)
+    hpd = osp.dirname(hp)
+    if not osp.isdir(hpd):
+        os.mkdir(hpd)
+    with open(hp, "wt") as fp:
+        fp.write(HOOKS_SHEBANG + content)
+    if make_exec:
+        os.chmod(hp, 0o744)
+    return hp
+
 
 class TestIndex(TestBase):
 
@@ -834,25 +850,21 @@ class TestIndex(TestBase):
     @with_rw_repo('HEAD', bare=True)
     def test_pre_commit_hook_success(self, rw_repo):
         index = rw_repo.index
-        hp = hook_path('pre-commit', index.repo.git_dir)
-        hpd = osp.dirname(hp)
-        if not osp.isdir(hpd):
-            os.mkdir(hpd)
-        with open(hp, "wt") as fp:
-            fp.write("#!/usr/bin/env sh\nexit 0")
-        os.chmod(hp, 0o744)
+        _make_hook(
+            index.repo.git_dir,
+            'pre-commit',
+            "exit 0"
+        )
         index.commit("This should not fail")
 
     @with_rw_repo('HEAD', bare=True)
     def test_pre_commit_hook_fail(self, rw_repo):
         index = rw_repo.index
-        hp = hook_path('pre-commit', index.repo.git_dir)
-        hpd = osp.dirname(hp)
-        if not osp.isdir(hpd):
-            os.mkdir(hpd)
-        with open(hp, "wt") as fp:
-            fp.write("#!/usr/bin/env sh\necho stdout; echo stderr 1>&2; exit 1")
-        os.chmod(hp, 0o744)
+        hp = _make_hook(
+            index.repo.git_dir,
+            'pre-commit',
+            "echo stdout; echo stderr 1>&2; exit 1"
+        )
         try:
             index.commit("This should fail")
         except HookExecutionError as err:
@@ -869,35 +881,29 @@ class TestIndex(TestBase):
                 self.assertEqual(err.stderr, "\n  stderr: 'stderr\n'")
                 assert str(err)
         else:
-            raise AssertionError("Should have cought a HookExecutionError")
+            raise AssertionError("Should have caught a HookExecutionError")
 
     @with_rw_repo('HEAD', bare=True)
     def test_commit_msg_hook_success(self, rw_repo):
-        index = rw_repo.index
         commit_message = u"commit default head by Frèderic Çaufl€"
         from_hook_message = u"from commit-msg"
-
-        hp = hook_path('commit-msg', index.repo.git_dir)
-        hpd = osp.dirname(hp)
-        if not osp.isdir(hpd):
-            os.mkdir(hpd)
-        with open(hp, "wt") as fp:
-            fp.write('#!/usr/bin/env sh\necho -n " {}" >> "$1"'.format(from_hook_message))
-        os.chmod(hp, 0o744)
-
+        index = rw_repo.index
+        _make_hook(
+            index.repo.git_dir,
+            'commit-msg',
+            'echo -n " {0}" >> "$1"'.format(from_hook_message)
+        )
         new_commit = index.commit(commit_message)
-        self.assertEqual(new_commit.message, u"{} {}".format(commit_message, from_hook_message))
+        self.assertEqual(new_commit.message, u"{0} {1}".format(commit_message, from_hook_message))
 
     @with_rw_repo('HEAD', bare=True)
     def test_commit_msg_hook_fail(self, rw_repo):
         index = rw_repo.index
-        hp = hook_path('commit-msg', index.repo.git_dir)
-        hpd = osp.dirname(hp)
-        if not osp.isdir(hpd):
-            os.mkdir(hpd)
-        with open(hp, "wt") as fp:
-            fp.write("#!/usr/bin/env sh\necho stdout; echo stderr 1>&2; exit 1")
-        os.chmod(hp, 0o744)
+        hp = _make_hook(
+            index.repo.git_dir,
+            'commit-msg',
+            "echo stdout; echo stderr 1>&2; exit 1"
+        )
         try:
             index.commit("This should fail")
         except HookExecutionError as err:
diff --git a/git/test/test_submodule.py b/git/test/test_submodule.py
index f970dd2c..5c8a2798 100644
--- a/git/test/test_submodule.py
+++ b/git/test/test_submodule.py
@@ -920,4 +920,4 @@ class TestSubmodule(TestBase):
         submodule_path = 'D:\\submodule_path'
         relative_path = Submodule._to_relative_path(super_repo, submodule_path)
         msg = '_to_relative_path should be "submodule_path" but was "%s"' % relative_path
-        assert relative_path == 'submodule_path', msg
\ No newline at end of file
+        assert relative_path == 'submodule_path', msg
diff --git a/git/test/test_util.py b/git/test/test_util.py
index 525c8609..d30c8376 100644
--- a/git/test/test_util.py
+++ b/git/test/test_util.py
@@ -217,49 +217,49 @@ class TestUtils(TestBase):
     @ddt.data(('name', ''), ('name', 'prefix_'))
     def test_iterable_list(self, case):
         name, prefix = case
-        l = IterableList(name, prefix)
+        ilist = IterableList(name, prefix)
 
         name1 = "one"
         name2 = "two"
         m1 = TestIterableMember(prefix + name1)
         m2 = TestIterableMember(prefix + name2)
 
-        l.extend((m1, m2))
+        ilist.extend((m1, m2))
 
-        self.assertEqual(len(l), 2)
+        self.assertEqual(len(ilist), 2)
 
         # contains works with name and identity
-        self.assertIn(name1, l)
-        self.assertIn(name2, l)
-        self.assertIn(m2, l)
-        self.assertIn(m2, l)
-        self.assertNotIn('invalid', l)
+        self.assertIn(name1, ilist)
+        self.assertIn(name2, ilist)
+        self.assertIn(m2, ilist)
+        self.assertIn(m2, ilist)
+        self.assertNotIn('invalid', ilist)
 
         # with string index
-        self.assertIs(l[name1], m1)
-        self.assertIs(l[name2], m2)
+        self.assertIs(ilist[name1], m1)
+        self.assertIs(ilist[name2], m2)
 
         # with int index
-        self.assertIs(l[0], m1)
-        self.assertIs(l[1], m2)
+        self.assertIs(ilist[0], m1)
+        self.assertIs(ilist[1], m2)
 
         # with getattr
-        self.assertIs(l.one, m1)
-        self.assertIs(l.two, m2)
+        self.assertIs(ilist.one, m1)
+        self.assertIs(ilist.two, m2)
 
         # test exceptions
-        self.failUnlessRaises(AttributeError, getattr, l, 'something')
-        self.failUnlessRaises(IndexError, l.__getitem__, 'something')
+        self.failUnlessRaises(AttributeError, getattr, ilist, 'something')
+        self.failUnlessRaises(IndexError, ilist.__getitem__, 'something')
 
         # delete by name and index
-        self.failUnlessRaises(IndexError, l.__delitem__, 'something')
-        del(l[name2])
-        self.assertEqual(len(l), 1)
-        self.assertNotIn(name2, l)
-        self.assertIn(name1, l)
-        del(l[0])
-        self.assertNotIn(name1, l)
-        self.assertEqual(len(l), 0)
-
-        self.failUnlessRaises(IndexError, l.__delitem__, 0)
-        self.failUnlessRaises(IndexError, l.__delitem__, 'something')
+        self.failUnlessRaises(IndexError, ilist.__delitem__, 'something')
+        del(ilist[name2])
+        self.assertEqual(len(ilist), 1)
+        self.assertNotIn(name2, ilist)
+        self.assertIn(name1, ilist)
+        del(ilist[0])
+        self.assertNotIn(name1, ilist)
+        self.assertEqual(len(ilist), 0)
+
+        self.failUnlessRaises(IndexError, ilist.__delitem__, 0)
+        self.failUnlessRaises(IndexError, ilist.__delitem__, 'something')