From 8dc8eb9e76a3162636856fa31b71ba10b33705eb Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 2 Feb 2024 01:06:26 -0500 Subject: [PATCH] Test established zero-argument refresh() behavior This adds tests like the existing ones but for when git.refresh is called with no arguments and the path is provided as the value of the GIT_PYTHON_GIT_EXECUTABLE environment variable. The preexisting tests, which this retains unchanged except with slightly more specific names to avoid confusion with the new tests, are of git.refresh(path). One benefit of these tests is to make a subtle but important aspect of the established behavior clear: relative paths are immediately resolved when passed as a path argument, but they are kept relative when given as the value of the environment variable. --- test/test_git.py | 69 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 64 insertions(+), 5 deletions(-) diff --git a/test/test_git.py b/test/test_git.py index a1c6211db..1b630fee0 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -46,9 +46,24 @@ def _patch_out_env(name): @contextlib.contextmanager def _rollback_refresh(): + old_git_executable = Git.GIT_PYTHON_GIT_EXECUTABLE + + if old_git_executable is None: + raise RuntimeError("no executable string (need initial refresh before test)") + try: - yield Git.GIT_PYTHON_GIT_EXECUTABLE # Provide the old value for convenience. + yield old_git_executable # Provide the old value for convenience. finally: + # The cleanup refresh should always raise an exception if it fails, since if it + # fails then previously discovered test results could be misleading and, more + # importantly, subsequent tests may be unable to run or give misleading results. + # So pre-set a non-None value, so that the cleanup will be a "second" refresh. + # This covers cases where a test has set it to None to test a "first" refresh. + Git.GIT_PYTHON_GIT_EXECUTABLE = Git.git_exec_name + + # Do the cleanup refresh. This sets Git.GIT_PYTHON_GIT_EXECUTABLE to old_value + # in most cases. The reason to call it is to achieve other associated state + # changes as well, which include updating attributes of the FetchInfo class. refresh() @@ -314,7 +329,51 @@ def test_cmd_override(self): ): self.assertRaises(GitCommandNotFound, self.git.version) - def test_refresh_bad_absolute_git_path(self): + def test_refresh_from_bad_absolute_git_path_env(self): + """Bad absolute path from environment is reported and not set.""" + absolute_path = str(Path("yada").absolute()) + expected_pattern = rf"\n[ \t]*cmdline: {re.escape(absolute_path)}\Z" + + with _rollback_refresh() as old_git_executable: + with mock.patch.dict(os.environ, {"GIT_PYTHON_GIT_EXECUTABLE": absolute_path}): + with self.assertRaisesRegex(GitCommandNotFound, expected_pattern): + refresh() + self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, old_git_executable) + + def test_refresh_from_bad_relative_git_path_env(self): + """Bad relative path from environment is kept relative and reported, not set.""" + # Relative paths are not resolved when refresh() is called with no arguments, so + # use a string that's very unlikely to be a command name found in a path lookup. + relative_path = "yada-e47e70c6-acbf-40f8-ad65-13af93c2195b" + expected_pattern = rf"\n[ \t]*cmdline: {re.escape(relative_path)}\Z" + + with _rollback_refresh() as old_git_executable: + with mock.patch.dict(os.environ, {"GIT_PYTHON_GIT_EXECUTABLE": relative_path}): + with self.assertRaisesRegex(GitCommandNotFound, expected_pattern): + refresh() + self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, old_git_executable) + + def test_refresh_from_good_absolute_git_path_env(self): + """Good absolute path from environment is set.""" + absolute_path = shutil.which("git") + + with _rollback_refresh(): + with mock.patch.dict(os.environ, {"GIT_PYTHON_GIT_EXECUTABLE": absolute_path}): + refresh() + self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, absolute_path) + + def test_refresh_from_good_relative_git_path_env(self): + """Good relative path from environment is kept relative and set.""" + with _rollback_refresh(): + # Set a string that wouldn't work and isn't "git". + type(self.git).GIT_PYTHON_GIT_EXECUTABLE = "" + + # Now observe if setting the environment variable to "git" takes effect. + with mock.patch.dict(os.environ, {"GIT_PYTHON_GIT_EXECUTABLE": "git"}): + refresh() + self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, "git") + + def test_refresh_with_bad_absolute_git_path_arg(self): """Bad absolute path arg is reported and not set.""" absolute_path = str(Path("yada").absolute()) expected_pattern = rf"\n[ \t]*cmdline: {re.escape(absolute_path)}\Z" @@ -324,7 +383,7 @@ def test_refresh_bad_absolute_git_path(self): refresh(absolute_path) self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, old_git_executable) - def test_refresh_bad_relative_git_path(self): + def test_refresh_with_bad_relative_git_path_arg(self): """Bad relative path arg is resolved to absolute path and reported, not set.""" absolute_path = str(Path("yada").absolute()) expected_pattern = rf"\n[ \t]*cmdline: {re.escape(absolute_path)}\Z" @@ -334,7 +393,7 @@ def test_refresh_bad_relative_git_path(self): refresh("yada") self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, old_git_executable) - def test_refresh_good_absolute_git_path(self): + def test_refresh_with_good_absolute_git_path_arg(self): """Good absolute path arg is set.""" absolute_path = shutil.which("git") @@ -342,7 +401,7 @@ def test_refresh_good_absolute_git_path(self): refresh(absolute_path) self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, absolute_path) - def test_refresh_good_relative_git_path(self): + def test_refresh_with_good_relative_git_path_arg(self): """Good relative path arg is resolved to absolute path and set.""" absolute_path = shutil.which("git") dirname, basename = osp.split(absolute_path)