Refactor "finally" cleanup in tests, fix minor bug

This refactors code in the test suite that uses try-finally, to
simplify and clarify what it is doing. An exception to this being
a refactoring is that a possible bug is fixed where a throwaway
environment variable "FOO" was patched for a test and never
unpatched.

There are two patterns refactored here:

- try-(try-except)-finally to try-except-finally. When the entire
  suite of the try-block in try-finally is itself a try-except,
  that has the same effect as try-except-finally. (Python always
  attempts to run the finally suite in either case, and does so
  after any applicable except suite is run.)

- Replacing try-finally with an appropriate context manager.

(These changes do not fix, nor introduce, any flake8 errors, but
they were found by manual search for possible problems related to
recent flake8 findings but outside of what flake8 can catch. To
limit scope, this only refactors try-finally in the test suite.)

Author: EliahKagan

Diff for: test/lib/helper.py

@@ -94,17 +94,16 @@ def wrapper(self):
         os.mkdir(path)
         keep = False
         try:
-            try:
-                return func(self, path)
-            except Exception:
-                log.info(
-                    "Test %s.%s failed, output is at %r\n",
-                    type(self).__name__,
-                    func.__name__,
-                    path,
-                )
-                keep = True
-                raise
+            return func(self, path)
+        except Exception:
+            log.info(
+                "Test %s.%s failed, output is at %r\n",
+                type(self).__name__,
+                func.__name__,
+                path,
+            )
+            keep = True
+            raise
         finally:
             # Need to collect here to be sure all handles have been closed. It appears
             # a windows-only issue. In fact things should be deleted, as well as
@@ -147,12 +146,11 @@ def repo_creator(self):
             prev_cwd = os.getcwd()
             os.chdir(rw_repo.working_dir)
             try:
-                try:
-                    return func(self, rw_repo)
-                except:  # noqa E722
-                    log.info("Keeping repo after failure: %s", repo_dir)
-                    repo_dir = None
-                    raise
+                return func(self, rw_repo)
+            except:  # noqa E722
+                log.info("Keeping repo after failure: %s", repo_dir)
+                repo_dir = None
+                raise
             finally:
                 os.chdir(prev_cwd)
                 rw_repo.git.clear_cache()

Diff for: test/test_git.py

@@ -195,17 +195,12 @@ def test_version(self):
         # END verify number types
 
     def test_cmd_override(self):
-        prev_cmd = self.git.GIT_PYTHON_GIT_EXECUTABLE
-        exc = GitCommandNotFound
-        try:
-            # set it to something that doesn't exist, assure it raises
-            type(self.git).GIT_PYTHON_GIT_EXECUTABLE = osp.join(
-                "some", "path", "which", "doesn't", "exist", "gitbinary"
-            )
-            self.assertRaises(exc, self.git.version)
-        finally:
-            type(self.git).GIT_PYTHON_GIT_EXECUTABLE = prev_cmd
-        # END undo adjustment
+        with mock.patch.object(
+            type(self.git),
+            "GIT_PYTHON_GIT_EXECUTABLE",
+            osp.join("some", "path", "which", "doesn't", "exist", "gitbinary"),
+        ):
+            self.assertRaises(GitCommandNotFound, self.git.version)
 
     def test_refresh(self):
         # test a bad git path refresh

Diff for: test/test_repo.py

@@ -847,18 +847,13 @@ def test_comparison_and_hash(self):
 
     @with_rw_directory
     def test_tilde_and_env_vars_in_repo_path(self, rw_dir):
-        ph = os.environ.get("HOME")
-        try:
+        with mock.patch.dict(os.environ, {"HOME": rw_dir}):
             os.environ["HOME"] = rw_dir
             Repo.init(osp.join("~", "test.git"), bare=True)
 
+        with mock.patch.dict(os.environ, {"FOO": rw_dir}):
             os.environ["FOO"] = rw_dir
             Repo.init(osp.join("$FOO", "test.git"), bare=True)
-        finally:
-            if ph:
-                os.environ["HOME"] = ph
-                del os.environ["FOO"]
-        # end assure HOME gets reset to what it was
 
     def test_git_cmd(self):
         # test CatFileContentStream, just to be very sure we have no fencepost errors