Skip to content

Commit

Permalink
Make _WinBashStatus instances carry all their info
Browse files Browse the repository at this point in the history
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 converted to a sum type without loss of clarity. This is
a development-only dependency; the main dependencies are unchanged.
  • Loading branch information
EliahKagan committed Nov 25, 2023
1 parent 8621e89 commit 0c3a616
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 35 deletions.
1 change: 1 addition & 0 deletions test-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ pytest-cov
pytest-instafail
pytest-mock
pytest-sugar
sumtypes
66 changes: 31 additions & 35 deletions test/test_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -14,6 +13,7 @@
import tempfile

import pytest
from sumtypes import constructor, sumtype

from git import (
IndexFile,
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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,
)
Expand All @@ -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,
)
Expand All @@ -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, "")
Expand All @@ -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,
)
Expand All @@ -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,
)
Expand All @@ -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, "")
Expand Down

0 comments on commit 0c3a616

Please sign in to comment.