Marco Ricci commited on 2025-05-22 23:14:58
Zeige 7 geänderte Dateien mit 134 Einfügungen und 17 Löschungen.
Add the changes that are necessary to appease The Annoying Operating System, a.k.a. Microsoft Windows. Some of these changes are OS-specific and (mostly) sensible, such as error codes being slightly different, or some functionality not being available. Some of these changes are coincidental, such as linting or type checking errors because unavailable symbols aren't even defined for the type checker's benefit. Finally, some changes are outright stupid, such as Python's documentation on `tempfile.gettempdir` being incomplete. In the end, most changes boil down to checking for a range of error codes instead of a specific one, or adding missing fixture calls, or silencing type checker warnings that aren't encountered in practice, or ruling out bad degenerate hypothesis test cases that aren't degenerate or aren't functional at all on The Annoying OS.
... | ... |
@@ -2,7 +2,6 @@ |
2 | 2 |
# |
3 | 3 |
# SPDX-License-Identifier: Zlib |
4 | 4 |
|
5 |
-# ruff: noqa: TRY400 |
|
6 | 5 |
|
7 | 6 |
"""Helper functions for the derivepassphrase command-line. |
8 | 7 |
|
... | ... |
@@ -275,6 +274,8 @@ def get_tempdir() -> pathlib.Path: |
275 | 274 |
pathlib.PurePosixPath('/usr/tmp'), |
276 | 275 |
] |
277 | 276 |
windows_paths_to_try = [ |
277 |
+ pathlib.PureWindowsPath(r'~\AppData\Local\Temp'), |
|
278 |
+ pathlib.PureWindowsPath(os.path.expandvars(r'%SYSTEMROOT%\Temp')), |
|
278 | 279 |
pathlib.PureWindowsPath(r'C:\TEMP'), |
279 | 280 |
pathlib.PureWindowsPath(r'C:\TMP'), |
280 | 281 |
pathlib.PureWindowsPath(r'\TEMP'), |
... | ... |
@@ -284,7 +285,7 @@ def get_tempdir() -> pathlib.Path: |
284 | 285 |
windows_paths_to_try if sys.platform == 'win32' else posix_paths_to_try |
285 | 286 |
) |
286 | 287 |
for p in paths_to_try: |
287 |
- path = pathlib.Path(p) |
|
288 |
+ path = pathlib.Path(p).expanduser() |
|
288 | 289 |
try: |
289 | 290 |
points_to_dir = path.is_dir() |
290 | 291 |
except OSError: |
... | ... |
@@ -63,11 +63,21 @@ def get_vault_key() -> bytes: |
63 | 63 |
Please set `VAULT_KEY` manually to the desired value. |
64 | 64 |
|
65 | 65 |
""" |
66 |
+ if os.supports_bytes_environ: |
|
67 |
+ |
|
68 |
+ def getenv(env_var: str) -> bytes: |
|
69 |
+ return os.environb.get(env_var.encode('UTF-8'), b'') # type: ignore[attr-defined] |
|
70 |
+ |
|
71 |
+ else: |
|
72 |
+ |
|
73 |
+ def getenv(env_var: str) -> bytes: |
|
74 |
+ return os.environ.get(env_var, '').encode('UTF-8') |
|
75 |
+ |
|
66 | 76 |
username = ( |
67 |
- os.environb.get(b'VAULT_KEY') |
|
68 |
- or os.environb.get(b'LOGNAME') |
|
69 |
- or os.environb.get(b'USER') |
|
70 |
- or os.environb.get(b'USERNAME') |
|
77 |
+ getenv('VAULT_KEY') |
|
78 |
+ or getenv('LOGNAME') |
|
79 |
+ or getenv('USER') |
|
80 |
+ or getenv('USERNAME') |
|
71 | 81 |
) |
72 | 82 |
if not username: |
73 | 83 |
env_var = 'VAULT_KEY' |
... | ... |
@@ -16,6 +16,7 @@ import pathlib |
16 | 16 |
import re |
17 | 17 |
import shlex |
18 | 18 |
import stat |
19 |
+import sys |
|
19 | 20 |
import tempfile |
20 | 21 |
import types |
21 | 22 |
import zipfile |
... | ... |
@@ -1603,6 +1604,61 @@ available. Usually this means that the test targets the |
1603 | 1604 |
`derivepassphrase export vault` subcommand, whose functionality depends |
1604 | 1605 |
on cryptography support being available. |
1605 | 1606 |
""" |
1607 |
+skip_if_on_the_annoying_os = pytest.mark.skipif( |
|
1608 |
+ sys.platform == 'win32', |
|
1609 |
+ reason='The Annoying OS behaves differently.', |
|
1610 |
+) |
|
1611 |
+""" |
|
1612 |
+A cached pytest mark to skip this test if running on The Annoying |
|
1613 |
+Operating System, a.k.a. Microsoft Windows. Usually this is due to |
|
1614 |
+unnecessary and stupid differences in the OS internals, and these |
|
1615 |
+differences are deemed irreconcilable in the context of the decorated |
|
1616 |
+test, so the test is to be skipped. |
|
1617 |
+ |
|
1618 |
+See also: |
|
1619 |
+ [`xfail_on_the_annoying_os`][] |
|
1620 |
+ |
|
1621 |
+""" |
|
1622 |
+ |
|
1623 |
+ |
|
1624 |
+def xfail_on_the_annoying_os( |
|
1625 |
+ f: Callable | None = None, |
|
1626 |
+ /, |
|
1627 |
+ *, |
|
1628 |
+ reason: str = '', |
|
1629 |
+) -> pytest.MarkDecorator | Any: |
|
1630 |
+ """Annotate a test which fails on The Annoying OS. |
|
1631 |
+ |
|
1632 |
+ Annotate a test to indicate that it fails on The Annoying Operating |
|
1633 |
+ System, a.k.a. Microsoft Windows. Usually this is due to |
|
1634 |
+ differences in the design of OS internals, and usually, these |
|
1635 |
+ differences are both unnecessary and stupid. |
|
1636 |
+ |
|
1637 |
+ Args: |
|
1638 |
+ f: |
|
1639 |
+ A callable to decorate. If not given, return the pytest |
|
1640 |
+ mark directly. |
|
1641 |
+ reason: |
|
1642 |
+ An optional, more detailed reason stating why this test |
|
1643 |
+ fails on The Annoying OS. |
|
1644 |
+ |
|
1645 |
+ Returns: |
|
1646 |
+ The callable, marked as an expected failure on the Annoying OS, |
|
1647 |
+ or alternatively a suitable pytest mark if no callable was |
|
1648 |
+ passed. The reason will begin with the phrase "The Annoying OS |
|
1649 |
+ behaves differently.", and the optional detailed reason, if not |
|
1650 |
+ empty, will follow. |
|
1651 |
+ |
|
1652 |
+ """ |
|
1653 |
+ base_reason = 'The Annoying OS behaves differently.' |
|
1654 |
+ full_reason = base_reason if not reason else f'{base_reason} {reason}' |
|
1655 |
+ mark = pytest.mark.xfail( |
|
1656 |
+ sys.platform == 'win32', |
|
1657 |
+ reason=full_reason, |
|
1658 |
+ raises=(AssertionError, hypothesis.errors.FailedHealthCheck), |
|
1659 |
+ strict=True, |
|
1660 |
+ ) |
|
1661 |
+ return mark if f is None else mark(f) |
|
1606 | 1662 |
|
1607 | 1663 |
|
1608 | 1664 |
def list_keys(self: Any = None) -> list[_types.SSHKeyCommentPair]: |
... | ... |
@@ -1739,7 +1795,8 @@ def isolated_config( |
1739 | 1795 |
) |
1740 | 1796 |
cwd = str(pathlib.Path.cwd().resolve()) |
1741 | 1797 |
monkeypatch.setenv('HOME', cwd) |
1742 |
- monkeypatch.setenv('USERPROFILE', cwd) |
|
1798 |
+ monkeypatch.setenv('APPDATA', cwd) |
|
1799 |
+ monkeypatch.setenv('LOCALAPPDATA', cwd) |
|
1743 | 1800 |
monkeypatch.delenv(env_name, raising=False) |
1744 | 1801 |
config_dir = cli_helpers.config_filename(subsystem=None) |
1745 | 1802 |
config_dir.mkdir(parents=True, exist_ok=True) |
... | ... |
@@ -1934,9 +1991,12 @@ def make_file_readonly( |
1934 | 1991 |
""" |
1935 | 1992 |
fname: int | str | bytes | os.PathLike |
1936 | 1993 |
if try_race_free_implementation and {os.stat, os.chmod} <= os.supports_fd: |
1994 |
+ # The Annoying OS (v11 at least) supports fstat and fchmod, but |
|
1995 |
+ # does not support changing the file mode on file descriptors |
|
1996 |
+ # for read-only files. |
|
1937 | 1997 |
fname = os.open( |
1938 | 1998 |
pathname, |
1939 |
- os.O_RDONLY |
|
1999 |
+ os.O_RDWR |
|
1940 | 2000 |
| getattr(os, 'O_CLOEXEC', 0) |
1941 | 2001 |
| getattr(os, 'O_NOCTTY', 0), |
1942 | 2002 |
) |
... | ... |
@@ -12,6 +12,7 @@ import errno |
12 | 12 |
import io |
13 | 13 |
import json |
14 | 14 |
import logging |
15 |
+import operator |
|
15 | 16 |
import os |
16 | 17 |
import pathlib |
17 | 18 |
import re |
... | ... |
@@ -2176,9 +2177,11 @@ class TestCLI: |
2176 | 2177 |
@Parametrize.CONFIG_WITH_KEY |
2177 | 2178 |
def test_204a_key_from_config( |
2178 | 2179 |
self, |
2180 |
+ running_ssh_agent: tests.RunningSSHAgentInfo, |
|
2179 | 2181 |
config: _types.VaultConfig, |
2180 | 2182 |
) -> None: |
2181 | 2183 |
"""A stored configured SSH key will be used.""" |
2184 |
+ del running_ssh_agent |
|
2182 | 2185 |
runner = tests.CliRunner(mix_stderr=False) |
2183 | 2186 |
# TODO(the-13th-letter): Rewrite using parenthesized |
2184 | 2187 |
# with-statements. |
... | ... |
@@ -2214,8 +2217,10 @@ class TestCLI: |
2214 | 2217 |
|
2215 | 2218 |
def test_204b_key_from_command_line( |
2216 | 2219 |
self, |
2220 |
+ running_ssh_agent: tests.RunningSSHAgentInfo, |
|
2217 | 2221 |
) -> None: |
2218 | 2222 |
"""An SSH key requested on the command-line will be used.""" |
2223 |
+ del running_ssh_agent |
|
2219 | 2224 |
runner = tests.CliRunner(mix_stderr=False) |
2220 | 2225 |
# TODO(the-13th-letter): Rewrite using parenthesized |
2221 | 2226 |
# with-statements. |
... | ... |
@@ -2817,7 +2822,10 @@ class TestCLI: |
2817 | 2822 |
['--import', os.fsdecode(dname)], |
2818 | 2823 |
catch_exceptions=False, |
2819 | 2824 |
) |
2820 |
- assert result.error_exit(error=os.strerror(errno.EISDIR)), ( |
|
2825 |
+ # The Annoying OS uses EACCES, other OSes use EISDIR. |
|
2826 |
+ assert result.error_exit( |
|
2827 |
+ error=os.strerror(errno.EISDIR) |
|
2828 |
+ ) or result.error_exit(error=os.strerror(errno.EACCES)), ( |
|
2821 | 2829 |
'expected error exit and known error message' |
2822 | 2830 |
) |
2823 | 2831 |
|
... | ... |
@@ -2983,6 +2991,7 @@ class TestCLI: |
2983 | 2991 |
'expected error exit and known error message' |
2984 | 2992 |
) |
2985 | 2993 |
|
2994 |
+ @tests.skip_if_on_the_annoying_os |
|
2986 | 2995 |
@Parametrize.EXPORT_FORMAT_OPTIONS |
2987 | 2996 |
def test_214e_export_settings_settings_directory_not_a_directory( |
2988 | 2997 |
self, |
... | ... |
@@ -3681,8 +3690,10 @@ class TestCLI: |
3681 | 3690 |
|
3682 | 3691 |
def test_225c_store_config_fail_manual_bad_ssh_agent_connection( |
3683 | 3692 |
self, |
3693 |
+ running_ssh_agent: tests.RunningSSHAgentInfo, |
|
3684 | 3694 |
) -> None: |
3685 | 3695 |
"""Not running a reachable SSH agent during `--config --key` fails.""" |
3696 |
+ del running_ssh_agent |
|
3686 | 3697 |
runner = tests.CliRunner(mix_stderr=False) |
3687 | 3698 |
# TODO(the-13th-letter): Rewrite using parenthesized |
3688 | 3699 |
# with-statements. |
... | ... |
@@ -4851,12 +4862,22 @@ Boo. |
4851 | 4862 |
assert _types.is_vault_config(config) |
4852 | 4863 |
return self.export_as_sh_helper(config) |
4853 | 4864 |
|
4865 |
+ # The Annoying OS appears to silently truncate spaces at the end of |
|
4866 |
+ # filenames. |
|
4854 | 4867 |
@hypothesis.given( |
4855 | 4868 |
env_var=strategies.sampled_from(['TMPDIR', 'TEMP', 'TMP']), |
4856 |
- suffix=strategies.text( |
|
4869 |
+ suffix=strategies.builds( |
|
4870 |
+ operator.add, |
|
4871 |
+ strategies.text( |
|
4872 |
+ tuple(' 0123456789abcdefghijklmnopqrstuvwxyz'), |
|
4873 |
+ min_size=11, |
|
4874 |
+ max_size=11, |
|
4875 |
+ ), |
|
4876 |
+ strategies.text( |
|
4857 | 4877 |
tuple('0123456789abcdefghijklmnopqrstuvwxyz'), |
4858 |
- min_size=12, |
|
4859 |
- max_size=12, |
|
4878 |
+ min_size=1, |
|
4879 |
+ max_size=1, |
|
4880 |
+ ), |
|
4860 | 4881 |
), |
4861 | 4882 |
) |
4862 | 4883 |
@hypothesis.example(env_var='', suffix='.') |
... | ... |
@@ -4873,6 +4894,17 @@ Boo. |
4873 | 4894 |
`cli_helpers.get_tempdir` returned the configuration directory. |
4874 | 4895 |
|
4875 | 4896 |
""" |
4897 |
+ |
|
4898 |
+ @contextlib.contextmanager |
|
4899 |
+ def make_temporary_directory( |
|
4900 |
+ path: pathlib.Path, |
|
4901 |
+ ) -> Iterator[pathlib.Path]: |
|
4902 |
+ try: |
|
4903 |
+ path.mkdir() |
|
4904 |
+ yield path |
|
4905 |
+ finally: |
|
4906 |
+ shutil.rmtree(path) |
|
4907 |
+ |
|
4876 | 4908 |
runner = tests.CliRunner(mix_stderr=False) |
4877 | 4909 |
# TODO(the-13th-letter): Rewrite using parenthesized |
4878 | 4910 |
# with-statements. |
... | ... |
@@ -4886,11 +4918,20 @@ Boo. |
4886 | 4918 |
vault_config={'services': {}}, |
4887 | 4919 |
) |
4888 | 4920 |
) |
4921 |
+ old_tempdir = os.fsdecode(tempfile.gettempdir()) |
|
4889 | 4922 |
monkeypatch.delenv('TMPDIR', raising=False) |
4890 | 4923 |
monkeypatch.delenv('TEMP', raising=False) |
4891 | 4924 |
monkeypatch.delenv('TMP', raising=False) |
4925 |
+ monkeypatch.setattr(tempfile, 'tempdir', None) |
|
4926 |
+ temp_path = pathlib.Path.cwd() / suffix |
|
4892 | 4927 |
if env_var: |
4893 |
- monkeypatch.setenv(env_var, str(pathlib.Path.cwd() / suffix)) |
|
4928 |
+ monkeypatch.setenv(env_var, os.fsdecode(temp_path)) |
|
4929 |
+ stack.enter_context(make_temporary_directory(temp_path)) |
|
4930 |
+ new_tempdir = os.fsdecode(tempfile.gettempdir()) |
|
4931 |
+ hypothesis.assume( |
|
4932 |
+ temp_path.resolve() == pathlib.Path.cwd().resolve() |
|
4933 |
+ or old_tempdir != new_tempdir |
|
4934 |
+ ) |
|
4894 | 4935 |
system_tempdir = os.fsdecode(tempfile.gettempdir()) |
4895 | 4936 |
our_tempdir = cli_helpers.get_tempdir() |
4896 | 4937 |
assert system_tempdir == os.fsdecode(our_tempdir) or ( |
... | ... |
@@ -4901,6 +4942,7 @@ Boo. |
4901 | 4942 |
system_tempdir == os.getcwd() # noqa: PTH109 |
4902 | 4943 |
and our_tempdir == cli_helpers.config_filename(subsystem=None) |
4903 | 4944 |
) |
4945 |
+ assert not temp_path.exists(), f'temp path {temp_path} not cleaned up!' |
|
4904 | 4946 |
|
4905 | 4947 |
def test_140b_get_tempdir_force_default(self) -> None: |
4906 | 4948 |
"""[`cli_helpers.get_tempdir`][] returns a temporary directory. |
... | ... |
@@ -5031,7 +5073,8 @@ Boo. |
5031 | 5073 |
if conn_hint == 'client': |
5032 | 5074 |
hint = ssh_agent.SSHAgentClient() |
5033 | 5075 |
elif conn_hint == 'socket': |
5034 |
- hint = socket.socket(family=socket.AF_UNIX) |
|
5076 |
+ # socket.AF_UNIX is not defined everywhere. |
|
5077 |
+ hint = socket.socket(family=socket.AF_UNIX) # type: ignore[attr-defined] |
|
5035 | 5078 |
hint.connect(running_ssh_agent.socket) |
5036 | 5079 |
else: |
5037 | 5080 |
assert conn_hint == 'none' |
... | ... |
@@ -5216,7 +5259,8 @@ class TestCLITransition: |
5216 | 5259 |
config2, err = cli_helpers.migrate_and_load_old_config() |
5217 | 5260 |
assert config2 == config |
5218 | 5261 |
assert isinstance(err, OSError) |
5219 |
- assert err.errno == errno.EISDIR |
|
5262 |
+ # The Annoying OS uses EEXIST, other OSes use EISDIR. |
|
5263 |
+ assert err.errno in {errno.EISDIR, errno.EEXIST} |
|
5220 | 5264 |
|
5221 | 5265 |
@Parametrize.BAD_CONFIGS |
5222 | 5266 |
def test_113_migrate_config_error_bad_config_value( |
... | ... |
@@ -711,7 +711,8 @@ class TestAgentInteraction: |
711 | 711 |
"""Fail if the agent address is invalid.""" |
712 | 712 |
with pytest.MonkeyPatch.context() as monkeypatch: |
713 | 713 |
monkeypatch.setenv('SSH_AUTH_SOCK', running_ssh_agent.socket + '~') |
714 |
- sock = socket.socket(family=socket.AF_UNIX) |
|
714 |
+ # socket.AF_UNIX is not defined everywhere. |
|
715 |
+ sock = socket.socket(family=socket.AF_UNIX) # type: ignore[attr-defined] |
|
715 | 716 |
with pytest.raises(OSError): # noqa: PT011 |
716 | 717 |
ssh_agent.SSHAgentClient(socket=sock) |
717 | 718 |
|
... | ... |
@@ -175,6 +175,8 @@ class TestL10nMachineryWithDebugTranslations: |
175 | 175 |
"""TranslatableStrings are hashable even with interpolations.""" |
176 | 176 |
assert len(errnos) == 2 |
177 | 177 |
error1, error2 = [os.strerror(c) for c in errnos] |
178 |
+ # The Annoying OS has error codes with identical strerror values. |
|
179 |
+ hypothesis.assume(error1 != error2) |
|
178 | 180 |
ts1 = msg.TranslatedString( |
179 | 181 |
value, error=error1, filename=None |
180 | 182 |
).maybe_without_filename() |
181 | 183 |