From 7ff3cee63c53520650db14bdf6e55551b8848567 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 25 Nov 2023 00:23:40 -0500 Subject: [PATCH] Make _WinBashStatus instances carry all their info This changes _WinBashStatus from an enum to a sum type so that, rather than having a global _WinBashStatus.ERROR_WHILE_CHECKING object whose error_or_process attribute may be absent and, if present, carries an object set on the most recent call to check(), we get a _WinBashStatus.ErrorWhileChecking instance that carries an error_or_process attribute specific to the call. Ordinarily this would not make a difference, other than that the global attribute usage was confusing in the code, because typically _WinBashStatus.check() is called only once. However, when debugging, having the old attribute on the same object be clobbered is undesirable. This adds the sumtypes library as a test dependency, to allow the enum to be rewritten as a sum type without loss of clarity. This is a development-only dependency; the main dependencies are unchanged. --- test-requirements.txt | 1 + test/test_index.py | 66 ++++++++++++++++++++----------------------- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/test-requirements.txt b/test-requirements.txt index 7cfb977a1..fcdc93c1d 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -9,3 +9,4 @@ pytest-cov pytest-instafail pytest-mock pytest-sugar +sumtypes diff --git a/test/test_index.py b/test/test_index.py index e1bcd5b25..f60a86309 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -3,7 +3,6 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ -import enum from io import BytesIO import logging import os @@ -14,6 +13,7 @@ import tempfile import pytest +from sumtypes import constructor, sumtype from git import ( IndexFile, @@ -39,35 +39,34 @@ log = logging.getLogger(__name__) -@enum.unique -class _WinBashStatus(enum.Enum): +@sumtype +class _WinBashStatus: """Status of bash.exe for native Windows. Affects which commit hook tests can pass. Call :meth:`check` to check the status. """ - INAPPLICABLE = enum.auto() + Inapplicable = constructor() """This system is not native Windows: either not Windows at all, or Cygwin.""" - ABSENT = enum.auto() + Absent = constructor() """No command for ``bash.exe`` is found on the system.""" - NATIVE = enum.auto() + Native = constructor() """Running ``bash.exe`` operates outside any WSL distribution (as with Git Bash).""" - WSL = enum.auto() + Wsl = constructor() """Running ``bash.exe`` calls ``bash`` in a WSL distribution.""" - WSL_NO_DISTRO = enum.auto() + WslNoDistro = constructor() """Running ``bash.exe` tries to run bash on a WSL distribution, but none exists.""" - ERROR_WHILE_CHECKING = enum.auto() + ErrorWhileChecking = constructor("error_or_process") """Could not determine the status. This should not trigger a skip or xfail, as it typically indicates either a fixable problem on the test machine, such as an "Insufficient system resources exist to complete the requested service" error starting WSL, or a bug in this detection code. - ``ERROR_WHILE_CHECKING.error_or_process`` has details about the most recent failure. """ @classmethod @@ -93,7 +92,13 @@ def check(cls): administrators occasionally put executables there in lieu of extending ``PATH``. """ if os.name != "nt": - return cls.INAPPLICABLE + return cls.Inapplicable() + + no_distro_message = "Windows Subsystem for Linux has no installed distributions." + + def error_running_bash(error): + log.error("Error running bash.exe to check WSL status: %s", error) + return cls.ErrorWhileChecking(error) try: # Output rather than forwarding the test command's exit status so that if a @@ -103,30 +108,21 @@ def check(cls): command = ["bash.exe", "-c", script] proc = subprocess.run(command, capture_output=True, check=True, text=True) except FileNotFoundError: - return cls.ABSENT + return cls.Absent() except OSError as error: - return cls._error(error) + return error_running_bash(error) except subprocess.CalledProcessError as error: - no_distro_message = "Windows Subsystem for Linux has no installed distributions." if error.returncode == 1 and error.stdout.startswith(no_distro_message): - return cls.WSL_NO_DISTRO - return cls._error(error) + return cls.WslNoDistro() + return error_running_bash(error) status = proc.stdout.rstrip() if status == "0": - return cls.WSL + return cls.Wsl() if status == "1": - return cls.NATIVE - return cls._error(proc) - - @classmethod - def _error(cls, error_or_process): - if isinstance(error_or_process, subprocess.CompletedProcess): - log.error("Strange output checking WSL status: %s", error_or_process.stdout) - else: - log.error("Error running bash.exe to check WSL status: %s", error_or_process) - cls.ERROR_WHILE_CHECKING.error_or_process = error_or_process - return cls.ERROR_WHILE_CHECKING + return cls.Native() + log.error("Strange output checking WSL status: %s", proc.stdout) + return cls.ErrorWhileChecking(proc) _win_bash_status = _WinBashStatus.check() @@ -1001,7 +997,7 @@ class Mocked: self.assertEqual(rel, os.path.relpath(path, root)) @pytest.mark.xfail( - _win_bash_status is _WinBashStatus.WSL_NO_DISTRO, + type(_win_bash_status) is _WinBashStatus.WslNoDistro, reason="Currently uses the bash.exe for WSL even with no WSL distro installed", raises=HookExecutionError, ) @@ -1012,7 +1008,7 @@ def test_pre_commit_hook_success(self, rw_repo): index.commit("This should not fail") @pytest.mark.xfail( - _win_bash_status is _WinBashStatus.WSL_NO_DISTRO, + type(_win_bash_status) is _WinBashStatus.WslNoDistro, reason="Currently uses the bash.exe for WSL even with no WSL distro installed", raises=AssertionError, ) @@ -1023,7 +1019,7 @@ def test_pre_commit_hook_fail(self, rw_repo): try: index.commit("This should fail") except HookExecutionError as err: - if _win_bash_status is _WinBashStatus.ABSENT: + if type(_win_bash_status) is _WinBashStatus.Absent: self.assertIsInstance(err.status, OSError) self.assertEqual(err.command, [hp]) self.assertEqual(err.stdout, "") @@ -1039,12 +1035,12 @@ def test_pre_commit_hook_fail(self, rw_repo): raise AssertionError("Should have caught a HookExecutionError") @pytest.mark.xfail( - _win_bash_status in {_WinBashStatus.ABSENT, _WinBashStatus.WSL}, + type(_win_bash_status) in {_WinBashStatus.Absent, _WinBashStatus.Wsl}, reason="Specifically seems to fail on WSL bash (in spite of #1399)", raises=AssertionError, ) @pytest.mark.xfail( - _win_bash_status is _WinBashStatus.WSL_NO_DISTRO, + type(_win_bash_status) is _WinBashStatus.WslNoDistro, reason="Currently uses the bash.exe for WSL even with no WSL distro installed", raises=HookExecutionError, ) @@ -1062,7 +1058,7 @@ def test_commit_msg_hook_success(self, rw_repo): self.assertEqual(new_commit.message, "{} {}".format(commit_message, from_hook_message)) @pytest.mark.xfail( - _win_bash_status is _WinBashStatus.WSL_NO_DISTRO, + type(_win_bash_status) is _WinBashStatus.WslNoDistro, reason="Currently uses the bash.exe for WSL even with no WSL distro installed", raises=AssertionError, ) @@ -1073,7 +1069,7 @@ def test_commit_msg_hook_fail(self, rw_repo): try: index.commit("This should fail") except HookExecutionError as err: - if _win_bash_status is _WinBashStatus.ABSENT: + if type(_win_bash_status) is _WinBashStatus.Absent: self.assertIsInstance(err.status, OSError) self.assertEqual(err.command, [hp]) self.assertEqual(err.stdout, "")