Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitb215357

Browse files
committed
Simplify/clarify bash.exe check for hook tests; do it only once
- Make the design of the enum simpler.- Move some informaton to the check method's docstring.- Call the method once instead of in each decoration.Doing the check only once makes things a little faster, but themore important reason is that the checks can become stale veryquickly in some situations. This is unlikely to be an issue on CI,but locally WSL may fail (or not fail) to start up when bash.exeis used in a check, then not fail (or fail) when actaully neededin the test, if the reason it fails is due to resource usage, suchas most RAM being used on the system.Although this seems like a reason to do the check multiple times,doing it multiple times in the decorations still does it ahead ofwhen any of the tests is actually run. In contrast, with the changehere, we may get outdated results by the time the tests run, butthe xfail effects of the check are always consistent with eachother and are no longer written in a way that suggests they aremore current than they really are.
1 parent5d11394 commitb215357

File tree

1 file changed

+38
-35
lines changed

1 file changed

+38
-35
lines changed

‎test/test_index.py

Lines changed: 38 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -39,32 +39,11 @@
3939
log=logging.getLogger(__name__)
4040

4141

42-
class_WinBashMeta(enum.EnumMeta):
43-
"""Metaclass allowing :class:`_WinBash` custom behavior when called."""
44-
45-
def__call__(cls):
46-
returncls._check()
47-
48-
4942
@enum.unique
50-
class_WinBash(enum.Enum,metaclass=_WinBashMeta):
43+
class_WinBashStatus(enum.Enum):
5144
"""Status of bash.exe for native Windows. Affects which commit hook tests can pass.
5245
53-
Call ``_WinBash()`` to check the status.
54-
55-
This can't be reliably discovered using :func:`shutil.which`, as that approximates
56-
how a shell is expected to search for an executable. On Windows, there are major
57-
differences between how executables are found by a shell and otherwise. (This is the
58-
cmd.exe Windows shell and should not be confused with bash.exe.) Our run_commit_hook
59-
function in GitPython uses subprocess.Popen, including to run bash.exe on Windows.
60-
It does not pass shell=True (and should not). Popen calls CreateProcessW, which
61-
searches several locations prior to using the PATH environment variable. It is
62-
expected to search the System32 directory, even if another directory containing the
63-
executable precedes it in PATH. (There are other differences, less relevant here.)
64-
When WSL is installed, even if no WSL *systems* are installed, bash.exe exists in
65-
System32, and Popen finds it even if another bash.exe precedes it in PATH, as
66-
happens on CI. If WSL is absent, System32 may still have bash.exe, as Windows users
67-
and administrators occasionally copy executables there in lieu of extending PATH.
46+
Call :meth:`check` to check the status.
6847
"""
6948

7049
INAPPLICABLE=enum.auto()
@@ -74,13 +53,13 @@ class _WinBash(enum.Enum, metaclass=_WinBashMeta):
7453
"""No command for ``bash.exe`` is found on the system."""
7554

7655
NATIVE=enum.auto()
77-
"""Running ``bash.exe`` operates outside any WSLenvironment (as with Git Bash)."""
56+
"""Running ``bash.exe`` operates outside any WSLdistribution (as with Git Bash)."""
7857

7958
WSL=enum.auto()
80-
"""Running ``bash.exe``runsbash on a WSLsystem."""
59+
"""Running ``bash.exe``calls ``bash`` in a WSLdistribution."""
8160

8261
WSL_NO_DISTRO=enum.auto()
83-
"""Running ``bash.exe` tries to run bash on a WSLsystem, but none exists."""
62+
"""Running ``bash.exe` tries to run bash on a WSLdistribution, but none exists."""
8463

8564
ERROR_WHILE_CHECKING=enum.auto()
8665
"""Could not determine the status.
@@ -92,13 +71,34 @@ class _WinBash(enum.Enum, metaclass=_WinBashMeta):
9271
"""
9372

9473
@classmethod
95-
def_check(cls):
74+
defcheck(cls):
75+
"""Check the status of the ``bash.exe`` :func:`index.fun.run_commit_hook` uses.
76+
77+
This uses EAFP, attempting to run a command via ``bash.exe``. Which ``bash.exe``
78+
is used can't be reliably discovered by :func:`shutil.which`, which approximates
79+
how a shell is expected to search for an executable. On Windows, there are major
80+
differences between how executables are found by a shell and otherwise. (This is
81+
the cmd.exe Windows shell, and shouldn't be confused with bash.exe itself. That
82+
the command being looked up also happens to be an interpreter is not relevant.)
83+
84+
:func:`index.fun.run_commit_hook` uses :class:`subprocess.Popen`, including when
85+
it runs ``bash.exe`` on Windows. It doesn't pass ``shell=True`` (and shouldn't).
86+
On Windows, `Popen` calls ``CreateProcessW``, which searches several locations
87+
prior to using the ``PATH`` environment variable. It is expected to search the
88+
``System32`` directory, even if another directory containing the executable
89+
precedes it in ``PATH``. (Other differences are less relevant here.) When WSL is
90+
installed, even with no distributions, ``bash.exe`` exists in ``System32``, and
91+
`Popen` finds it even if another ``bash.exe`` precedes it in ``PATH``, as on CI.
92+
If WSL is absent, ``System32`` may still have ``bash.exe``, as Windows users and
93+
administrators occasionally put executables there in lieu of extending ``PATH``.
94+
"""
9695
ifos.name!="nt":
9796
returncls.INAPPLICABLE
9897

9998
try:
10099
# Print rather than returning the test command's exit status so that if a
101-
# failure occurs before we even get to this point, we will detect it.
100+
# failure occurs before we even get to this point, we will detect it. See
101+
# https://superuser.com/a/1749811 for information on ways to check for WSL.
102102
script='test -e /proc/sys/fs/binfmt_misc/WSLInterop; echo "$?"'
103103
command= ["bash.exe","-c",script]
104104
proc=subprocess.run(command,capture_output=True,check=True,text=True)
@@ -129,6 +129,9 @@ def _error(cls, error_or_process):
129129
returncls.ERROR_WHILE_CHECKING
130130

131131

132+
_win_bash_status=_WinBashStatus.check()
133+
134+
132135
def_make_hook(git_dir,name,content,make_exec=True):
133136
"""A helper to create a hook"""
134137
hp=hook_path(name,git_dir)
@@ -998,7 +1001,7 @@ class Mocked:
9981001
self.assertEqual(rel,os.path.relpath(path,root))
9991002

10001003
@pytest.mark.xfail(
1001-
_WinBash()is_WinBash.WSL_NO_DISTRO,
1004+
_win_bash_statusis_WinBashStatus.WSL_NO_DISTRO,
10021005
reason="Currently uses the bash.exe for WSL even with no WSL distro installed",
10031006
raises=HookExecutionError,
10041007
)
@@ -1009,7 +1012,7 @@ def test_pre_commit_hook_success(self, rw_repo):
10091012
index.commit("This should not fail")
10101013

10111014
@pytest.mark.xfail(
1012-
_WinBash()is_WinBash.WSL_NO_DISTRO,
1015+
_win_bash_statusis_WinBashStatus.WSL_NO_DISTRO,
10131016
reason="Currently uses the bash.exe for WSL even with no WSL distro installed",
10141017
raises=AssertionError,
10151018
)
@@ -1020,7 +1023,7 @@ def test_pre_commit_hook_fail(self, rw_repo):
10201023
try:
10211024
index.commit("This should fail")
10221025
exceptHookExecutionErroraserr:
1023-
if_WinBash()is_WinBash.ABSENT:
1026+
if_win_bash_statusis_WinBashStatus.ABSENT:
10241027
self.assertIsInstance(err.status,OSError)
10251028
self.assertEqual(err.command, [hp])
10261029
self.assertEqual(err.stdout,"")
@@ -1036,12 +1039,12 @@ def test_pre_commit_hook_fail(self, rw_repo):
10361039
raiseAssertionError("Should have caught a HookExecutionError")
10371040

10381041
@pytest.mark.xfail(
1039-
_WinBash()in {_WinBash.ABSENT,_WinBash.WSL},
1042+
_win_bash_statusin {_WinBashStatus.ABSENT,_WinBashStatus.WSL},
10401043
reason="Specifically seems to fail on WSL bash (in spite of #1399)",
10411044
raises=AssertionError,
10421045
)
10431046
@pytest.mark.xfail(
1044-
_WinBash()is_WinBash.WSL_NO_DISTRO,
1047+
_win_bash_statusis_WinBashStatus.WSL_NO_DISTRO,
10451048
reason="Currently uses the bash.exe for WSL even with no WSL distro installed",
10461049
raises=HookExecutionError,
10471050
)
@@ -1059,7 +1062,7 @@ def test_commit_msg_hook_success(self, rw_repo):
10591062
self.assertEqual(new_commit.message,"{} {}".format(commit_message,from_hook_message))
10601063

10611064
@pytest.mark.xfail(
1062-
_WinBash()is_WinBash.WSL_NO_DISTRO,
1065+
_win_bash_statusis_WinBashStatus.WSL_NO_DISTRO,
10631066
reason="Currently uses the bash.exe for WSL even with no WSL distro installed",
10641067
raises=AssertionError,
10651068
)
@@ -1070,7 +1073,7 @@ def test_commit_msg_hook_fail(self, rw_repo):
10701073
try:
10711074
index.commit("This should fail")
10721075
exceptHookExecutionErroraserr:
1073-
if_WinBash()is_WinBash.ABSENT:
1076+
if_win_bash_statusis_WinBashStatus.ABSENT:
10741077
self.assertIsInstance(err.status,OSError)
10751078
self.assertEqual(err.command, [hp])
10761079
self.assertEqual(err.stdout,"")

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp