Skip to content

Commit 9e43921

Browse files
Korijnclaude
andcommitted
Run the WSL tests wherever a distribution answers
Calling the windows install of keycmd from a shell inside WSL is the half of the WSL setup that needs both sides of the boundary to be real, and it was opt in through KEYCMD_TEST_WSL, on the grounds that installing WSL takes a CI job of its own. That reasoning holds for CI and nowhere else: a developer machine with WSL2 on it can run those tests today, and the only thing standing in the way was a variable nobody remembers to set. The suite works it out for itself now. find_wsl in tests/conftest.py asks whether this is a windows machine, whether a distribution answers -- which is a different question from whether one is registered, since wsl.exe is on PATH on any windows install and docker's distributions run no shell -- and whether there is a keycmd on PATH for it to reach. The `wsl` fixture hands the tests the distribution or skips them with the reason there is none, the way `os_keyring` does for the keyring, and KEYCMD_REQUIRE_WSL turns that skip into a failure for the one CI job that provisions a distro. The answer goes in the pytest header next to the shells, since a run that silently skipped these looks exactly like a run on a machine without WSL. What the tests needed of wsl.exe moves to the fixture with it, so a test reads wsl.sh(script) and wsl.path(p) rather than reaching for helpers of its own, and decode now sniffs the utf-16 that wsl.exe writes its own errors in. A machine with no distribution registered says so in utf-16, and that sentence is the skip reason, which is not the place to hand someone W\x00i\x00n\x00d\x00o\x00w\x00s. Which turned up a bug that would have made most of this moot. Subclassing a keyring backend registers it, for the session and with no way to take it back out, so FakeChainer in test_backend.py was a candidate in every search that ran after that module was imported -- and a chainer of two backends outranks the credential manager, so the keyring probe settled on a chainer of null and fail and reported that this machine has no keyring. Every credential test then skipped itself, which is what a machine without a keyring is supposed to look like. It passed in CI only because -v prints the report header, which initializes keyring before collection; -q does not, so a local run covered twenty tests fewer without saying anything. The double sets viable = False and stays out of the search. Locally that is 196 passed where it was 175 passed and 21 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent e93383e commit 9e43921

6 files changed

Lines changed: 178 additions & 92 deletions

File tree

.github/workflows/ci.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,10 @@ jobs:
132132
timeout-minutes: 20
133133
env:
134134
KEYCMD_REQUIRE_OS_KEYRING: "1"
135-
KEYCMD_TEST_WSL: "1"
135+
# the WSL tests run wherever a distribution answers and skip where
136+
# none does; this is the one job that provisions one, so here a skip
137+
# is a broken setup rather than a machine without WSL
138+
KEYCMD_REQUIRE_WSL: "1"
136139
steps:
137140
- uses: actions/checkout@v7
138141
- name: Install uv and Python 3.14

CLAUDE.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,14 @@ uv run mkdocs build --strict # what CI builds, warnings
2626

2727
The tests that read and write credentials need an OS keyring that unlocks without user interaction. They skip themselves with a message when there is none, so the rest of the suite still runs; `KEYCMD_REQUIRE_OS_KEYRING=1` turns those skips into failures, and CI sets it. Windows needs no setup, macOS needs an unlocked keychain, and Linux needs the tests to run inside a d-bus session with `gnome-keyring` unlocked (see `docs/development/testing.md` for the exact commands). `PYTHON_KEYRING_BACKEND=keyrings.alt.file.PlaintextKeyring` with `uv run --with keyrings.alt` avoids the OS keyring entirely.
2828

29-
`tests/test_wsl.py` covers calling the Windows install of keycmd from a shell inside WSL, and is opt in through `KEYCMD_TEST_WSL=1` on a Windows machine with WSL installed. `tests/test_wsl_interop.py` covers the same boundary as far as it can be reached without one, by faking the Windows process table, and runs everywhere. The rest of the suite stays off that code path entirely: the autouse `outside_wsl` fixture in `tests/conftest.py` clears the flags `wsl.py` detects with, so a run on Windows looks like a run anywhere else.
29+
`tests/test_wsl.py` covers calling the Windows install of keycmd from a shell inside WSL. The `wsl` fixture in `tests/conftest.py` decides whether that is possible here — a Windows machine, a distribution that answers, and a keycmd on the `PATH` that it can reach — and hands the tests the distribution or skips them with the reason there is none; `KEYCMD_REQUIRE_WSL=1` turns that skip into a failure, and the CI job that installs WSL sets it. The answer is reported in the pytest header, since a silent skip looks the same as a machine without WSL. `tests/test_wsl_interop.py` covers the same boundary as far as it can be reached without one, by faking the Windows process table, and runs everywhere. The rest of the suite stays off that code path entirely: the autouse `outside_wsl` fixture in `tests/conftest.py` clears the flags `wsl.py` detects with, so a run on Windows looks like a run anywhere else.
3030

3131
Things that bite in this suite:
3232

3333
- **Never assume a shell.** The `shell` fixture in `tests/conftest.py` parametrizes over every shell of the platform that is installed, so a test using it runs three times. Ask the `Shell` object for the dialect (`env_var`, `unset_env_var`, `command_not_found_statuses`) instead of branching on the platform. Shells that are not installed locally are covered by asserting on the command line keycmd builds for them.
34-
- **`wsl.exe` mangles its command line**: backslashes disappear and quotes are stripped before the distribution sees them. Pass paths translated to `/mnt/...` by `wsl_path`, unquoted and free of spaces, and keep remote scripts on one line.
34+
- **`wsl.exe` mangles its command line**: backslashes disappear and quotes are stripped before the distribution sees them. Pass paths translated to `/mnt/...` by `Wsl.path`, unquoted and free of spaces, and keep remote scripts on one line. It also reports its own errors in utf-16 while the distribution writes utf-8, which is what `decode` sniffs for.
3535
- **The remembered backend is redirected, always.** The autouse `cache_home` fixture in `tests/conftest.py` points `backend.CACHE_HOME` at a folder under `tmp_path`, so that a test run neither reads nor writes the note the machine it runs on is using, and every test starts with nothing remembered.
36+
- **A backend subclassed in a test joins keyring's registry**, for the whole session and with no way to take it back out, so the search any later test runs can settle on it and hand the suite a backend that holds no credentials — which reaches the tests as an OS keyring that skipped itself. `FakeChainer` in `tests/test_backend.py` sets `viable = False` to stay out of the running.
3637
- **Do not assume the suite runs unpinned.** `PYTHON_KEYRING_BACKEND` is how the README suggests running the suite without an OS keyring, and it outranks everything `backend.py` does, so a test about remembering has to `delenv` it first or it will be testing the path that deliberately remembers nothing.
3738
- Warnings are errors (`filterwarnings` in `pyproject.toml`), so a deprecation in a new Python release fails the suite rather than scrolling past.
3839

docs/development/testing.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,20 @@ The other half — calling the Windows install of keycmd from a WSL shell to rea
5959

6060
Everything about that boundary that can be decided without a Windows machine is in `tests/test_wsl_interop.py` instead, and runs everywhere: which process tree and working directory mean keycmd was called from a distribution, the command lines it builds for `wsl.exe`, and the `WSLENV` that carries the credentials across.
6161

62-
The end to end tests are opt in, because installing WSL takes a CI job of its own. On a Windows machine that has WSL installed:
62+
The end to end tests run by themselves on a Windows machine whose WSL install answers, and skip themselves with the reason it did not anywhere else — no distribution registered, no keycmd on the `PATH` for one to call. The report header of every run says which it was:
6363

64-
```powershell
65-
$env:KEYCMD_TEST_WSL = 1
66-
uv run pytest tests/test_wsl.py
6764
```
65+
shells exercised: cmd, powershell, pwsh
66+
WSL distribution: Ubuntu
67+
```
68+
69+
Set `KEYCMD_REQUIRE_WSL=1` to turn those skips into failures, the same way `KEYCMD_REQUIRE_OS_KEYRING` does for the keyring. CI sets it on the one job that installs WSL — the other jobs have none, and skip — so that a distribution that fails to provision fails the build instead of quietly reducing it.
6870

6971
## Things that bite in this suite
7072

7173
* **Never assume a shell.** The `shell` fixture parametrizes over every shell of the platform that is installed, so a test using it runs several times. Ask the `Shell` object for the dialect (`env_var`, `unset_env_var`, `command_not_found_statuses`) instead of branching on the platform.
7274
* **`wsl.exe` mangles its command line**: backslashes disappear and quotes are stripped before the distribution sees them. Pass paths translated to `/mnt/...` by `wsl_path`, unquoted and free of spaces, and keep remote scripts on one line.
7375
* **The remembered backend is redirected, always.** An autouse fixture points `backend.CACHE_HOME` at a folder under `tmp_path`, so a test run neither reads nor writes the note the machine it runs on is using.
76+
* **A backend subclassed in a test joins keyring's registry** for the rest of the session, so a later search can settle on it and hand the suite a backend that holds no credentials, which arrives as an OS keyring that skipped itself. A test double takes itself out of the running with `viable = False`.
7477
* **Do not assume the suite runs unpinned.** `PYTHON_KEYRING_BACKEND` outranks everything `backend.py` does, so a test about remembering has to `delenv` it first, or it will be testing the path that deliberately remembers nothing.
7578
* **Warnings are errors**, so a deprecation in a new Python release fails the suite rather than scrolling past.

tests/conftest.py

Lines changed: 134 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66

77
import os
88
from dataclasses import dataclass
9-
from pathlib import Path
9+
from functools import cache
10+
from pathlib import Path, PureWindowsPath
1011
from shutil import which
12+
from subprocess import run
1113
from typing import NamedTuple
1214

1315
import keyring
@@ -24,6 +26,11 @@
2426
# instead of silently skipping every test that touches the keyring
2527
REQUIRE_OS_KEYRING = os.environ.get("KEYCMD_REQUIRE_OS_KEYRING", "") not in {"", "0"}
2628

29+
# and this on the job that installs WSL, for the same reason: the tests
30+
# that cross the interop boundary run wherever a distribution answers, so
31+
# only the run that provisioned one can tell a skip from a broken setup
32+
REQUIRE_WSL = os.environ.get("KEYCMD_REQUIRE_WSL", "") not in {"", "0"}
33+
2734
# shells to exercise, if installed
2835
POSIX_SHELLS = ("sh", "bash", "zsh")
2936
WINDOWS_SHELLS = ("cmd", "powershell")
@@ -46,16 +53,20 @@
4653

4754

4855
def pytest_report_header(config):
49-
"""Report what this run picked up, both of which vary per machine
56+
"""Report what this run picked up, all of which varies per machine
5057
51-
Which shells a run covers is otherwise only visible in the ids of the
52-
tests that failed, so a run where they all pass does not say whether a
53-
shell was exercised or simply absent.
58+
Which shells a run covers, and whether it reached a WSL distribution,
59+
is otherwise only visible in the ids of the tests that failed, so a
60+
run where they all pass does not say whether a shell was exercised or
61+
simply absent.
5462
"""
5563
shells = ", ".join(shell.name for shell in installed_shells())
64+
found = find_wsl()
65+
wsl = found.distro if isinstance(found, Wsl) else f"none, {found}"
5666
return [
5767
f"keyring backend: {keyring.get_keyring()}",
5868
f"shells exercised: {shells or 'none'}",
69+
f"WSL distribution: {wsl}",
5970
]
6071

6172

@@ -141,6 +152,124 @@ def detect_shell(pid):
141152
return fake_shell
142153

143154

155+
def decode(raw):
156+
"""Text out of a distribution, whichever side of the boundary wrote it
157+
158+
The linux side writes utf-8, while wsl.exe reports its own errors in
159+
utf-16; a NUL byte gives those away, since utf-8 output has none.
160+
Undecodable bytes are replaced rather than raised on, so that output
161+
that went wrong still reaches the assertion it explains.
162+
"""
163+
if b"\x00" in raw:
164+
return raw.decode("utf-16", errors="replace")
165+
return raw.decode("utf-8", errors="replace")
166+
167+
168+
class Output(NamedTuple):
169+
"""What a command run inside a distribution had to say"""
170+
171+
status: int
172+
stdout: str
173+
stderr: str
174+
175+
@property
176+
def output(self):
177+
"""Both streams, for the message of the assertion that failed"""
178+
return f"{self.stdout}\n{self.stderr}"
179+
180+
181+
def wsl_run(*args):
182+
"""Run a command in the default WSL distribution"""
183+
p = run(["wsl.exe", "--", *args], capture_output=True)
184+
return Output(p.returncode, decode(p.stdout), decode(p.stderr))
185+
186+
187+
def wsl_sh(script):
188+
"""Run a shell script in the default WSL distribution
189+
190+
Plain sh, since not every distribution ships bash.
191+
"""
192+
return wsl_run("sh", "-eu", "-c", script)
193+
194+
195+
def wsl_path(path):
196+
"""Translate a Windows path into the path WSL knows it by
197+
198+
Done here rather than with wslpath, which is not part of every
199+
distribution's root file system, and whose backslashes would not
200+
survive the trip through wsl.exe's command line anyway.
201+
"""
202+
path = PureWindowsPath(path)
203+
drive = path.drive
204+
assert drive.endswith(":"), f"not an absolute windows path: {path}"
205+
rest = path.as_posix()[len(drive) :].lstrip("/")
206+
translated = f"/mnt/{drive[0].lower()}/{rest}"
207+
# quotes are stripped from wsl.exe's command line before the distribution
208+
# ever sees them, so a path with spaces cannot be passed through it
209+
assert " " not in translated, f"path with spaces: {translated}"
210+
return translated
211+
212+
213+
@dataclass(frozen=True)
214+
class Wsl:
215+
"""A distribution to run commands in, and the keycmd it can reach"""
216+
217+
distro: str
218+
# the windows console script, as WSL users reach it through the PATH
219+
keycmd: str
220+
221+
def sh(self, script):
222+
"""Run a shell script inside the distribution"""
223+
return wsl_sh(script)
224+
225+
def path(self, path):
226+
"""The path this distribution knows a windows path by"""
227+
return wsl_path(path)
228+
229+
230+
@cache
231+
def find_wsl():
232+
"""A WSL boundary this run can test across, or the reason there is none
233+
234+
Windows keycmd called from a distribution is the half of the WSL setup
235+
that needs both sides of the boundary to be real, so it is tested
236+
wherever both are there and skipped, with the reason, where they are
237+
not. The answer is worked out once and reported in the header, since a
238+
run that skipped these silently looks exactly like a run without WSL.
239+
"""
240+
if not IS_WINDOWS:
241+
return "keycmd only crosses the WSL boundary as a windows process"
242+
if which("wsl.exe") is None:
243+
return "wsl.exe is not installed"
244+
path = which("keycmd")
245+
if path is None:
246+
return "the keycmd console script is not on PATH"
247+
if " " in path:
248+
# wsl.exe strips the quotes that would hold it together, so a path
249+
# with spaces cannot be handed to the distribution at all
250+
return f"the keycmd console script is under a path with spaces: {path}"
251+
# a distribution that answers, rather than one that is merely
252+
# registered: wsl.exe is on PATH on windows whether or not there is
253+
# anything behind it, and docker's distributions run no shell
254+
found = wsl_sh("echo ${WSL_DISTRO_NAME:-default}")
255+
if found.status != 0 or not found.stdout.strip():
256+
said = " ".join(found.output.split())
257+
return f"no WSL distribution answered: {said or f'exit status {found.status}'}"
258+
return Wsl(distro=found.stdout.strip(), keycmd=wsl_path(path))
259+
260+
261+
@pytest.fixture(scope="session")
262+
def wsl():
263+
"""A WSL distribution with the windows keycmd reachable from it"""
264+
found = find_wsl()
265+
if isinstance(found, str):
266+
msg = f"no WSL to test against: {found}"
267+
if REQUIRE_WSL:
268+
pytest.fail(msg)
269+
pytest.skip(msg)
270+
return found
271+
272+
144273
@pytest.fixture(autouse=True)
145274
def outside_wsl(monkeypatch):
146275
"""Keep the suite off the WSL code path unless a test asks for it

tests/test_backend.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,16 @@
3434

3535

3636
class FakeChainer(ChainerBackend):
37-
"""A chainer with a fixed membership, instead of the one on this machine"""
37+
"""A chainer with a fixed membership, instead of the one on this machine
3838
39+
Subclassing a backend registers it with keyring, and a chainer of more
40+
than one backend outranks everything else, so the search any later
41+
test runs would settle on this one and find no credentials in it.
42+
Keyring skips a backend that says it is not viable, which is the way
43+
out of a registry there is no taking a class back out of.
44+
"""
45+
46+
viable: ClassVar = False
3947
backends: ClassVar = [NullKeyring(), NoKeyring()]
4048

4149

tests/test_wsl.py

Lines changed: 21 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -1,93 +1,36 @@
11
"""Reaching the Windows credential manager from a WSL shell
22
3-
The README tells WSL users to install keycmd on Windows and call it from
3+
The docs tell WSL users to install keycmd on Windows and call it from
44
their WSL shell, so that keyring talks to the Windows credential manager
55
instead of a keyring daemon inside the distro. These tests walk that path
66
end to end: a credential in the credential manager, a shell inside WSL,
77
and the Windows install of keycmd in between.
88
9-
Installing WSL is a CI job of its own, so they are opt in.
9+
They run on any Windows machine with a distribution that answers, and
10+
skip themselves with the reason anywhere else; the `wsl` fixture in
11+
conftest.py works out which of the two it is, and KEYCMD_REQUIRE_WSL=1
12+
turns that skip into a failure, which is what the CI job that installs
13+
WSL sets.
1014
"""
1115

12-
import os
13-
from pathlib import PureWindowsPath
14-
from shutil import which
15-
from subprocess import run
1616

17-
import pytest
18-
19-
RUN_WSL_TESTS = os.environ.get("KEYCMD_TEST_WSL", "") not in {"", "0"}
20-
21-
pytestmark = pytest.mark.skipif(
22-
not RUN_WSL_TESTS,
23-
reason="set KEYCMD_TEST_WSL=1 on a Windows machine with WSL installed",
24-
)
25-
26-
27-
def decode(raw):
28-
# the linux side writes utf-8, while wsl.exe reports its own errors in
29-
# utf-16, so keep going on undecodable bytes rather than swallow output
30-
return raw.decode("utf-8", errors="replace")
31-
32-
33-
def wsl(*args):
34-
"""Run a command in the default WSL distribution"""
35-
return run(["wsl.exe", "--", *args], capture_output=True)
36-
37-
38-
def wsl_sh(script):
39-
"""Run a shell script in the default WSL distribution
40-
41-
Plain sh, since not every distribution ships bash.
42-
"""
43-
return wsl("sh", "-eu", "-c", script)
44-
45-
46-
def wsl_path(path):
47-
"""Translate a Windows path into the path WSL knows it by
48-
49-
Done here rather than with wslpath, which is not part of every
50-
distribution's root file system, and whose backslashes would not
51-
survive the trip through wsl.exe's command line anyway.
52-
"""
53-
path = PureWindowsPath(path)
54-
drive = path.drive
55-
assert drive.endswith(":"), f"not an absolute windows path: {path}"
56-
rest = path.as_posix()[len(drive) :].lstrip("/")
57-
translated = f"/mnt/{drive[0].lower()}/{rest}"
58-
# quotes are stripped from wsl.exe's command line before the distribution
59-
# ever sees them, so a path with spaces cannot be passed through it
60-
assert " " not in translated, f"path with spaces: {translated}"
61-
return translated
62-
63-
64-
@pytest.fixture(scope="session")
65-
def keycmd_exe():
66-
"""The Windows console script, as WSL users reach it through the PATH"""
67-
path = which("keycmd")
68-
assert path is not None, "the keycmd console script is not on PATH"
69-
return wsl_path(path)
70-
71-
72-
def test_wsl_reads_windows_paths(tmp_path):
17+
def test_wsl_reads_windows_paths(wsl, tmp_path):
7318
"""WSL is reachable, and it sees the Windows file system where expected"""
7419
marker = tmp_path / "marker"
7520
marker.write_text("hello from windows", encoding="utf-8")
76-
p = wsl_sh(f"cat {wsl_path(marker)}")
77-
assert p.returncode == 0, decode(p.stderr)
78-
assert decode(p.stdout).strip() == "hello from windows"
21+
p = wsl.sh(f"cat {wsl.path(marker)}")
22+
assert p.status == 0, p.output
23+
assert p.stdout.strip() == "hello from windows"
7924

8025

81-
def test_version_from_wsl(keycmd_exe):
26+
def test_version_from_wsl(wsl):
8227
"""The Windows install runs when it is invoked from a WSL shell"""
83-
p = wsl_sh(f"{keycmd_exe} --version")
84-
assert p.returncode == 0, decode(p.stderr)
85-
assert decode(p.stdout).strip().startswith("keycmd: v")
28+
p = wsl.sh(f"{wsl.keycmd} --version")
29+
assert p.status == 0, p.output
30+
assert p.stdout.strip().startswith("keycmd: v")
8631

8732

88-
def test_credential_manager_from_wsl(
89-
keycmd_exe, ch_tmpdir, local_conf, shell_credentials
90-
):
33+
def test_credential_manager_from_wsl(wsl, ch_tmpdir, local_conf, shell_credentials):
9134
"""A credential stored on Windows reaches a command run from WSL
9235
9336
The command runs back inside the distribution the user typed it in,
@@ -98,10 +41,9 @@ def test_credential_manager_from_wsl(
9841
# one line and free of quotes, so that the script survives the trip
9942
# through wsl.exe intact; the config is picked up from the working
10043
# directory, which crosses the boundary as a windows path
101-
script = f"cd {wsl_path(ch_tmpdir)}; {keycmd_exe} --verbose printenv {var}"
102-
p = wsl_sh(script)
103-
output = f"{decode(p.stdout)}\n{decode(p.stderr)}"
104-
assert p.returncode == 0, output
105-
assert f"as environment variable {var}" in output
106-
assert "called from WSL" in output
107-
assert shell_credentials.password in decode(p.stdout)
44+
script = f"cd {wsl.path(ch_tmpdir)}; {wsl.keycmd} --verbose printenv {var}"
45+
p = wsl.sh(script)
46+
assert p.status == 0, p.output
47+
assert f"as environment variable {var}" in p.output
48+
assert "called from WSL" in p.output
49+
assert shell_credentials.password in p.stdout

0 commit comments

Comments
 (0)