Cover macOS, Linux and WSL in the test suite and CI matrix - #32
Merged
Conversation
The test suite needs an OS keyring that unlocks without user interaction, which used to limit CI to Windows runners. Both other platforms can provide one too: - macOS: create a throwaway keychain and make it the default - Linux: run the tests in a d-bus session with an unlocked gnome-keyring CI now tests Linux, macOS and Windows on every supported Python version. The tests that need a keyring skip themselves with a message when none is available, so contributors are not blocked; KEYCMD_REQUIRE_OS_KEYRING turns those skips into failures, and CI sets it so that a broken keyring setup cannot quietly gut a run. Everything platform specific now lives in a shared conftest, which also removes the duplicated fixtures. The suite exercises every shell of the platform that is installed instead of only the one that happens to have invoked pytest, and the shells that are not installed are covered by asserting on the command line keycmd builds for them. New tests cover the posix process replacement path, the shell detection fallbacks, the verbose logging, and the error paths of the cli and of get_env, taking coverage to 100% (CI gates at 95%). Also stop test_find_file from creating a file in the real home folder. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PYvtsyJgsswPf278ytw2Zd
WSL needs no job of its own: inside WSL keycmd is a posix process the Linux job already covers, and calling the Windows install from a WSL shell runs keycmd as a Windows process the Windows job already covers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PYvtsyJgsswPf278ytw2Zd
The half of the WSL story that can actually break is the interop boundary: keycmd installed on windows, invoked from a shell inside WSL, reading the windows credential manager. Cover it end to end with a credential in the credential manager, a shell inside WSL, and the windows install of keycmd in between, in a CI job that installs WSL. The tests are opt in through KEYCMD_TEST_WSL, since installing WSL is too expensive to put in front of every run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PYvtsyJgsswPf278ytw2Zd
run_cmd logs the command through pformat, which wraps the list over several lines when the shell path is long, so a plain repr of the list is not in the output. Caught by powershell.EXE on the windows runners, whose path is long enough to wrap where pwsh's is not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PYvtsyJgsswPf278ytw2Zd
Backslashes do not survive wsl.exe's command line, which turned the path of the console script into D:akeycmdkeycmd.venvScriptskeycmd.EXE before wslpath ever saw it. Hand wslpath forward slashes instead, and keep the script one line so that it crosses the same boundary unharmed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PYvtsyJgsswPf278ytw2Zd
The install step took 44 seconds on one run and was still going twenty minutes later on the next, so drop what makes it heavy: WSLv1 needs no virtual machine, and the Alpine root file system is a fraction of the Ubuntu one from the store. That root file system ships neither bash nor wslpath, so run the scripts with plain sh and translate windows paths in the test itself, which is also one less thing to lose backslashes on the way through wsl.exe. The smoke test now reads a file it created on the windows side, so that the translation is verified before the tests that depend on it run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PYvtsyJgsswPf278ytw2Zd
wsl.exe strips quotes from its command line, so sh expanded the $env: prefix of the powershell spelling as an unset variable, and keycmd echoed ":KEYCMD_TEST". Ask cmd to print the environment instead: it works whichever shell keycmd detects on the windows side, needs no quoting, and lets the test assert on the value of the variable rather than on whatever the shell echoed. Also assert paths translated for WSL have no spaces, since quoting them is not an option. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PYvtsyJgsswPf278ytw2Zd
Python 3.14 deprecated the single argument form of TOMLDecodeError, and the structured one it wants instead does not exist before 3.14, so name the offending file by rewriting the message of the original error rather than raising a new one. That keeps the type, the message and the traceback the cli and the tests already rely on. Turn warnings into errors while here, so the next deprecation fails the suite instead of scrolling past in the log. Trim the matrix to the latest python on all three platforms plus a single job on the oldest supported one, which is what the type checker is already pinned to. Add a CLAUDE.md covering the commands, the shape of the configuration, credential and shell layers, and the platform traps in the test suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PYvtsyJgsswPf278ytw2Zd
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The test suite needs an OS keyring that unlocks without user interaction, which is why CI only ran on Windows. Every other platform can provide one too, so CI now tests Linux, macOS and Windows on every supported Python version, plus WSL reaching the Windows credential manager.
Keyring setup per platform
gnome-keyringand run the tests inside adbus-run-sessionwith the daemon unlocked, since the secret service is bound to a d-bus session.The tests that need a keyring skip themselves with a message when none is available, so contributors are not blocked on a headless machine.
KEYCMD_REQUIRE_OS_KEYRING=1turns those skips into failures, and CI sets it so a broken keyring setup cannot quietly gut a run rather than fail it.WSL
The half of the WSL setup that can actually break is the interop boundary: keycmd installed on Windows, invoked from a shell inside WSL, reading the Windows credential manager.
tests/test_wsl.pycovers exactly that, in a job of its own that installs WSL, and it passes: keycmd finds its config, reads the credential out of the credential manager, and hands it to the command.Working inside WSL needs no separate job, since there keycmd is a posix process talking to whichever keyring backend the distro provides, which is what the Linux job covers, daemon and all.
Notes on how that job is set up, both learned the hard way on this branch:
wsl.exestrips quotes and eats backslashes on its way to the distribution, which mangled the path of the console script and swallowed the$env:prefix of a variable reference. Paths are therefore translated in the test rather than bywslpath(which the Alpine root file system does not ship anyway), passed unquoted, and asserted to be free of spaces. The environment is printed withcmd /c set, which needs no quoting and works whichever shell keycmd detects on the Windows side.Test suite
Everything platform specific moved into a new
tests/conftest.py, which also removes the fixtures that were duplicated across three modules.sh,bash,zshandcmd,powershell,pwsh) instead of only the shell that happened to invoke pytest. Shells that are not installed are still covered by asserting on the command line keycmd builds for them, which is what covers thecmd/Cbranch on posix runners.execvpepath (including exit status propagation and environment inheritance, verified in real subprocesses), the three shell detection fallbacks,--shell,--verbose, the missing command and invalid TOML errors,expose, the missing credential and missing alias errors, and the config walk stopping at the home folder and at the root of the file system.test_find_fileno longer creates a file in the real home folder.Coverage goes from 79% on Linux, with failures, to 100% on Linux, macOS and Windows alike. CI gates at 95%.
Unrelated and pre-existing: on Python 3.14
keycmd/conf.py:43emits aDeprecationWarningabout free-formTOMLDecodeErrorarguments. Left alone, since it is a source change rather than test or CI scope.