diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1db01e3..cfed1f4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -132,7 +132,10 @@ jobs: timeout-minutes: 20 env: KEYCMD_REQUIRE_OS_KEYRING: "1" - KEYCMD_TEST_WSL: "1" + # the WSL tests run wherever a distribution answers and skip where + # none does; this is the one job that provisions one, so here a skip + # is a broken setup rather than a machine without WSL + KEYCMD_REQUIRE_WSL: "1" steps: - uses: actions/checkout@v7 - name: Install uv and Python 3.14 diff --git a/CLAUDE.md b/CLAUDE.md index 4f4298e..7b3acf1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,13 +26,14 @@ uv run mkdocs build --strict # what CI builds, warnings 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. -`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. +`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. Things that bite in this suite: - **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. -- **`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. +- **`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. - **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. +- **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. - **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. - Warnings are errors (`filterwarnings` in `pyproject.toml`), so a deprecation in a new Python release fails the suite rather than scrolling past. diff --git a/docs/development/testing.md b/docs/development/testing.md index e1d97fe..6a8f830 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -59,17 +59,20 @@ The other half — calling the Windows install of keycmd from a WSL shell to rea 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. -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: +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: -```powershell -$env:KEYCMD_TEST_WSL = 1 -uv run pytest tests/test_wsl.py ``` +shells exercised: cmd, powershell, pwsh +WSL distribution: Ubuntu +``` + +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. ## Things that bite in this suite * **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. * **`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. * **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. +* **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`. * **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. * **Warnings are errors**, so a deprecation in a new Python release fails the suite rather than scrolling past. diff --git a/tests/conftest.py b/tests/conftest.py index fc90c65..b29b0f6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,8 +6,10 @@ import os from dataclasses import dataclass -from pathlib import Path +from functools import cache +from pathlib import Path, PureWindowsPath from shutil import which +from subprocess import run from typing import NamedTuple import keyring @@ -24,6 +26,11 @@ # instead of silently skipping every test that touches the keyring REQUIRE_OS_KEYRING = os.environ.get("KEYCMD_REQUIRE_OS_KEYRING", "") not in {"", "0"} +# and this on the job that installs WSL, for the same reason: the tests +# that cross the interop boundary run wherever a distribution answers, so +# only the run that provisioned one can tell a skip from a broken setup +REQUIRE_WSL = os.environ.get("KEYCMD_REQUIRE_WSL", "") not in {"", "0"} + # shells to exercise, if installed POSIX_SHELLS = ("sh", "bash", "zsh") WINDOWS_SHELLS = ("cmd", "powershell") @@ -46,16 +53,20 @@ def pytest_report_header(config): - """Report what this run picked up, both of which vary per machine + """Report what this run picked up, all of which varies per machine - Which shells a run covers is otherwise only visible in the ids of the - tests that failed, so a run where they all pass does not say whether a - shell was exercised or simply absent. + Which shells a run covers, and whether it reached a WSL distribution, + is otherwise only visible in the ids of the tests that failed, so a + run where they all pass does not say whether a shell was exercised or + simply absent. """ shells = ", ".join(shell.name for shell in installed_shells()) + found = find_wsl() + wsl = found.distro if isinstance(found, Wsl) else f"none, {found}" return [ f"keyring backend: {keyring.get_keyring()}", f"shells exercised: {shells or 'none'}", + f"WSL distribution: {wsl}", ] @@ -141,6 +152,124 @@ def detect_shell(pid): return fake_shell +def decode(raw): + """Text out of a distribution, whichever side of the boundary wrote it + + The linux side writes utf-8, while wsl.exe reports its own errors in + utf-16; a NUL byte gives those away, since utf-8 output has none. + Undecodable bytes are replaced rather than raised on, so that output + that went wrong still reaches the assertion it explains. + """ + if b"\x00" in raw: + return raw.decode("utf-16", errors="replace") + return raw.decode("utf-8", errors="replace") + + +class Output(NamedTuple): + """What a command run inside a distribution had to say""" + + status: int + stdout: str + stderr: str + + @property + def output(self): + """Both streams, for the message of the assertion that failed""" + return f"{self.stdout}\n{self.stderr}" + + +def wsl_run(*args): + """Run a command in the default WSL distribution""" + p = run(["wsl.exe", "--", *args], capture_output=True) + return Output(p.returncode, decode(p.stdout), decode(p.stderr)) + + +def wsl_sh(script): + """Run a shell script in the default WSL distribution + + Plain sh, since not every distribution ships bash. + """ + return wsl_run("sh", "-eu", "-c", script) + + +def wsl_path(path): + """Translate a Windows path into the path WSL knows it by + + Done here rather than with wslpath, which is not part of every + distribution's root file system, and whose backslashes would not + survive the trip through wsl.exe's command line anyway. + """ + path = PureWindowsPath(path) + drive = path.drive + assert drive.endswith(":"), f"not an absolute windows path: {path}" + rest = path.as_posix()[len(drive) :].lstrip("/") + translated = f"/mnt/{drive[0].lower()}/{rest}" + # quotes are stripped from wsl.exe's command line before the distribution + # ever sees them, so a path with spaces cannot be passed through it + assert " " not in translated, f"path with spaces: {translated}" + return translated + + +@dataclass(frozen=True) +class Wsl: + """A distribution to run commands in, and the keycmd it can reach""" + + distro: str + # the windows console script, as WSL users reach it through the PATH + keycmd: str + + def sh(self, script): + """Run a shell script inside the distribution""" + return wsl_sh(script) + + def path(self, path): + """The path this distribution knows a windows path by""" + return wsl_path(path) + + +@cache +def find_wsl(): + """A WSL boundary this run can test across, or the reason there is none + + Windows keycmd called from a distribution is the half of the WSL setup + that needs both sides of the boundary to be real, so it is tested + wherever both are there and skipped, with the reason, where they are + not. The answer is worked out once and reported in the header, since a + run that skipped these silently looks exactly like a run without WSL. + """ + if not IS_WINDOWS: + return "keycmd only crosses the WSL boundary as a windows process" + if which("wsl.exe") is None: + return "wsl.exe is not installed" + path = which("keycmd") + if path is None: + return "the keycmd console script is not on PATH" + if " " in path: + # wsl.exe strips the quotes that would hold it together, so a path + # with spaces cannot be handed to the distribution at all + return f"the keycmd console script is under a path with spaces: {path}" + # a distribution that answers, rather than one that is merely + # registered: wsl.exe is on PATH on windows whether or not there is + # anything behind it, and docker's distributions run no shell + found = wsl_sh("echo ${WSL_DISTRO_NAME:-default}") + if found.status != 0 or not found.stdout.strip(): + said = " ".join(found.output.split()) + return f"no WSL distribution answered: {said or f'exit status {found.status}'}" + return Wsl(distro=found.stdout.strip(), keycmd=wsl_path(path)) + + +@pytest.fixture(scope="session") +def wsl(): + """A WSL distribution with the windows keycmd reachable from it""" + found = find_wsl() + if isinstance(found, str): + msg = f"no WSL to test against: {found}" + if REQUIRE_WSL: + pytest.fail(msg) + pytest.skip(msg) + return found + + @pytest.fixture(autouse=True) def outside_wsl(monkeypatch): """Keep the suite off the WSL code path unless a test asks for it diff --git a/tests/test_backend.py b/tests/test_backend.py index 73fdd2f..edde9d8 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -34,8 +34,16 @@ class FakeChainer(ChainerBackend): - """A chainer with a fixed membership, instead of the one on this machine""" + """A chainer with a fixed membership, instead of the one on this machine + Subclassing a backend registers it with keyring, and a chainer of more + than one backend outranks everything else, so the search any later + test runs would settle on this one and find no credentials in it. + Keyring skips a backend that says it is not viable, which is the way + out of a registry there is no taking a class back out of. + """ + + viable: ClassVar = False backends: ClassVar = [NullKeyring(), NoKeyring()] diff --git a/tests/test_wsl.py b/tests/test_wsl.py index cc4b0fd..2d7d099 100644 --- a/tests/test_wsl.py +++ b/tests/test_wsl.py @@ -1,93 +1,36 @@ """Reaching the Windows credential manager from a WSL shell -The README tells WSL users to install keycmd on Windows and call it from +The docs tell WSL users to install keycmd on Windows and call it from their WSL shell, so that keyring talks to the Windows credential manager instead of a keyring daemon inside the distro. These tests walk that path end to end: a credential in the credential manager, a shell inside WSL, and the Windows install of keycmd in between. -Installing WSL is a CI job of its own, so they are opt in. +They run on any Windows machine with a distribution that answers, and +skip themselves with the reason anywhere else; the `wsl` fixture in +conftest.py works out which of the two it is, and KEYCMD_REQUIRE_WSL=1 +turns that skip into a failure, which is what the CI job that installs +WSL sets. """ -import os -from pathlib import PureWindowsPath -from shutil import which -from subprocess import run -import pytest - -RUN_WSL_TESTS = os.environ.get("KEYCMD_TEST_WSL", "") not in {"", "0"} - -pytestmark = pytest.mark.skipif( - not RUN_WSL_TESTS, - reason="set KEYCMD_TEST_WSL=1 on a Windows machine with WSL installed", -) - - -def decode(raw): - # the linux side writes utf-8, while wsl.exe reports its own errors in - # utf-16, so keep going on undecodable bytes rather than swallow output - return raw.decode("utf-8", errors="replace") - - -def wsl(*args): - """Run a command in the default WSL distribution""" - return run(["wsl.exe", "--", *args], capture_output=True) - - -def wsl_sh(script): - """Run a shell script in the default WSL distribution - - Plain sh, since not every distribution ships bash. - """ - return wsl("sh", "-eu", "-c", script) - - -def wsl_path(path): - """Translate a Windows path into the path WSL knows it by - - Done here rather than with wslpath, which is not part of every - distribution's root file system, and whose backslashes would not - survive the trip through wsl.exe's command line anyway. - """ - path = PureWindowsPath(path) - drive = path.drive - assert drive.endswith(":"), f"not an absolute windows path: {path}" - rest = path.as_posix()[len(drive) :].lstrip("/") - translated = f"/mnt/{drive[0].lower()}/{rest}" - # quotes are stripped from wsl.exe's command line before the distribution - # ever sees them, so a path with spaces cannot be passed through it - assert " " not in translated, f"path with spaces: {translated}" - return translated - - -@pytest.fixture(scope="session") -def keycmd_exe(): - """The Windows console script, as WSL users reach it through the PATH""" - path = which("keycmd") - assert path is not None, "the keycmd console script is not on PATH" - return wsl_path(path) - - -def test_wsl_reads_windows_paths(tmp_path): +def test_wsl_reads_windows_paths(wsl, tmp_path): """WSL is reachable, and it sees the Windows file system where expected""" marker = tmp_path / "marker" marker.write_text("hello from windows", encoding="utf-8") - p = wsl_sh(f"cat {wsl_path(marker)}") - assert p.returncode == 0, decode(p.stderr) - assert decode(p.stdout).strip() == "hello from windows" + p = wsl.sh(f"cat {wsl.path(marker)}") + assert p.status == 0, p.output + assert p.stdout.strip() == "hello from windows" -def test_version_from_wsl(keycmd_exe): +def test_version_from_wsl(wsl): """The Windows install runs when it is invoked from a WSL shell""" - p = wsl_sh(f"{keycmd_exe} --version") - assert p.returncode == 0, decode(p.stderr) - assert decode(p.stdout).strip().startswith("keycmd: v") + p = wsl.sh(f"{wsl.keycmd} --version") + assert p.status == 0, p.output + assert p.stdout.strip().startswith("keycmd: v") -def test_credential_manager_from_wsl( - keycmd_exe, ch_tmpdir, local_conf, shell_credentials -): +def test_credential_manager_from_wsl(wsl, ch_tmpdir, local_conf, shell_credentials): """A credential stored on Windows reaches a command run from WSL The command runs back inside the distribution the user typed it in, @@ -98,10 +41,9 @@ def test_credential_manager_from_wsl( # one line and free of quotes, so that the script survives the trip # through wsl.exe intact; the config is picked up from the working # directory, which crosses the boundary as a windows path - script = f"cd {wsl_path(ch_tmpdir)}; {keycmd_exe} --verbose printenv {var}" - p = wsl_sh(script) - output = f"{decode(p.stdout)}\n{decode(p.stderr)}" - assert p.returncode == 0, output - assert f"as environment variable {var}" in output - assert "called from WSL" in output - assert shell_credentials.password in decode(p.stdout) + script = f"cd {wsl.path(ch_tmpdir)}; {wsl.keycmd} --verbose printenv {var}" + p = wsl.sh(script) + assert p.status == 0, p.output + assert f"as environment variable {var}" in p.output + assert "called from WSL" in p.output + assert shell_credentials.password in p.stdout