Skip to content

test: stop tests depending on the contributor's locale and repo - #3826

Merged
KuSh merged 1 commit into
rtk-ai:developfrom
KuSh:fix/tests-locale-independence
Sep 30, 2026
Merged

KuSh merged 1 commit into
rtk-ai:developfrom
KuSh:fix/tests-locale-independence

Conversation

@KuSh

@KuSh KuSh commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Problem

Some tests assert against the machine they run on rather than against RTK.

Hard failure. test_git_status_not_a_repo_exits_nonzero matches git's English not a git repository on stderr. git status is a passthrough, so its message stays in the user's language by design (see git_cmd_c_locale's doc) — the test fails for anyone whose git speaks another language:

$ cargo test --all -- --ignored          # fr_FR.UTF-8
test cmds::git::git::tests::test_git_status_not_a_repo_exits_nonzero ... FAILED
$ LC_ALL=C cargo test --all -- --ignored
ok

Silent weakening, which is worse. search_faithful_test.rs asserts !out.contains("Is a directory") to prove rtk grep reads piped stdin instead of injecting a path. Wherever grep speaks another language the string can never appear, so the assertion passes without testing anything:

$ rtk grep pattern somedir
/usr/bin/grep: somedir: est un dossier          # negative assertion passes vacuously
$ LC_ALL=C rtk grep pattern somedir
/usr/bin/grep: somedir: Is a directory          # what the test was written against

Ambient repository. test_branch_creation_not_swallowed and test_branch_creation_from_commit ran git branch in whatever repository the contributor happened to be in, then deleted the branch again; test_broken_pipe_does_not_crash read that repository's log, and outside a repository git log only errors, so it stopped exercising the pipe.

Change

  • src/core/test_support.rs (new, #[cfg(test)]): rtk_command() / pin_environment() pin LC_ALL + LANGUAGE and neutralize GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM; temp_git_repo() builds a throwaway repo with a local identity, so no global git config is needed at all.
  • The git-status test now asserts what RTK owns — non-zero exit, message on stderr, nothing on stdout — instead of git's wording, and runs in a fresh temp dir rather than a fixed shared /tmp path.
  • The three engine-spawning test helpers (search_faithful, search_compress, grep_context) and the native rg/grep spawns they compare against pin LC_ALL, so both sides speak one language. search_error_test.rs already did this for rtk; its grep side now matches.
  • The two branch tests and the broken-pipe test take a temp_git_repo() and run the built binary with .current_dir().
  • .claude/rules/cli-testing.md gains a "Machine independence" section: never assert on a third-party tool's wording (and watch negative assertions especially), never touch the ambient repo, plus the hostile-environment commands to check against.

Test plan

Full suite including --ignored, all green under each of:

LANG=fr_FR.UTF-8 cargo test --all -- --ignored
LC_ALL=de_DE.UTF-8 cargo test --all -- --ignored
GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null cargo test --all -- --ignored
RIPGREP_CONFIG_PATH=<config with --smart-case -i> cargo test --all -- --ignored

Before this change the first two failed. cargo fmt --all && cargo clippy --all-targets && cargo test --all clean.

Found while reviewing #3681; unrelated to that PR's contents, so it is on its own branch off develop.

🤖 Generated with Claude Code

@KuSh
KuSh force-pushed the fix/tests-locale-independence branch 2 times, most recently from 02f6daf to 541f51c Compare September 2, 2026 07:53
`test_git_status_not_a_repo_exits_nonzero` matched git's English "not a git
repository" on stderr, so it failed in any other language -- `git status` is a
passthrough and keeps the user's locale by design. It now asserts what RTK
actually owns there: a non-zero exit, the message on stderr, nothing on stdout.

The quieter half: `!out.contains("Is a directory")` in search_faithful_test
passed vacuously wherever grep speaks another language, testing nothing. The
engine-spawning helpers pin LC_ALL for both sides of a comparison, so those
assertions mean the same thing everywhere.

The branch tests ran `git branch` in whatever repo the contributor was sitting
in and deleted it afterwards; the broken-pipe test read that repo's log, and
outside a repo it stopped exercising the pipe at all. All three take a
throwaway repo from the new test_support helpers, which also neutralize
GIT_CONFIG_GLOBAL/SYSTEM so local aliases and init.defaultBranch stay out.

Verified against fr_FR, de_DE, an empty git config and an injected
RIPGREP_CONFIG_PATH.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@KuSh
KuSh force-pushed the fix/tests-locale-independence branch from 541f51c to 377177c Compare September 17, 2026 22:37

@aeppling aeppling left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @KuSh , all good to me, thanks for contributing !

@aeppling aeppling self-assigned this Sep 30, 2026
@KuSh
KuSh merged commit 77a0c68 into rtk-ai:develop Sep 30, 2026
11 checks passed
@KuSh
KuSh deleted the fix/tests-locale-independence branch September 30, 2026 16:33
@rtk-release-bot rtk-release-bot Bot mentioned this pull request Sep 30, 2026
KuSh added a commit to KuSh/rtk that referenced this pull request Sep 30, 2026
Running `cargo test` wrote into the contributor's own files and took its
results from their machine. Under a hostile but ordinary environment — deny
rules in ~/.claude/settings.json, a legacy rtk hook in ~/.claude/hooks, an
exported CLAUDE_CONFIG_DIR, RTK_DB_PATH or RTK_NO_TOML, a ~/.gitconfig that
signs commits, GIT_DIR exported by a git hook, a French locale, a global.json
above TMPDIR — `cargo test --all -- --include-ignored` on develop fails 126
tests, and it deletes the legacy hook script, its hash, the Cursor hook and the
OpenCode plugin, writes CLAUDE.md and RTK.md into the exported Claude
directory, fills the exported tracking database and recall store, adds tee and
audit logs, a telemetry salt and a filters.toml, and stages a file into the
repository GIT_DIR names and installs a hook in it. With this change the same
run passes and leaves every one of those files as it was.

In-process, a test build no longer reads the user's locations or environment:

- `core::user_dirs` is the one place that resolves a location: rtk's data and
  config directories and the home (through `dirs`), the working directory, a
  project root found by walking up, and path variables such as
  CLAUDE_CONFIG_DIR. In a test build each answer lies in the process's scratch
  directory: under a root a test can make its own (`test_isolation::with_root`,
  `with_agent_dir`), in the directory it entered (`CwdGuard`, recorded per
  thread), and upward searches stop at the scratch directory.
- `core::user_env` is the one reader of environment variables. In a test build
  it answers only what the calling test set, per thread, so an exported
  RTK_NO_TOML or COMPOSER_BIN_DIR cannot change a result and parallel tests
  cannot see each other's values.
- rtk's own git commands, test-spawned git and rtk children run git with no
  global or system configuration, no inherited GIT_* variable, an empty init
  template, and GIT_CEILING_DIRECTORIES at the temporary directory.
- Process-wide caches that would hold whichever test's configuration was read
  first — `cached_config`, the TOML filter registry's user filters, the
  Composer bin directories — are loaded per call in a test build.

A spawned rtk is built without cfg(test), so the environment is its only
channel. `test_isolation::rtk_command` and its twin `common::rtk_command` clear
every inherited RTK_* variable and the agent, CI and Composer variables rtk
reads, point its data, config and home at the scratch root, start it in an
empty project directory, and isolate its git. The locale is left to each test,
so a child's output and a native tool it is compared with share one.

The machine-independence helpers from rtk-ai#3826 now live in `test_isolation`, so
there is one `rtk_command`, one `temp_git_repo` and one git isolation, and the
tests they serve no longer write to the contributor's rtk data.

Scans keep this from drifting: one refuses a test that names the rtk binary
(CARGO_BIN_EXE_rtk, `Command::new("rtk")`, a target/debug path) outside the two
constructors; one refuses `dirs::`, a direct environment read, or a
working-directory read outside `user_dirs`/`user_env`, with a short list of
exemptions that must each still match; one checks that every variable rtk
reads is pinned or cleared for a child. Each asserts that the files it exempts
were visited, so a run that cannot see the sources fails rather than passes.

Production behaviour is unchanged: every redirect sits behind cfg(test).

Two limits remain. A spawned rtk is production code and searches upward from
its working directory like any run, so with TMPDIR under the home (the Windows
default) it can read settings above it. `rtk hook audit` still reads HOME
directly (rtk-ai#1681).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
KuSh added a commit to KuSh/rtk that referenced this pull request Sep 30, 2026
`cargo test` wrote into the contributor's own files (tracking database,
tee spool, telemetry salt, agent hooks and settings) and took its results
from their machine: agent settings, exported RTK_* and agent variables, git
configuration and repository variables, files above TMPDIR.

A test build now resolves every user location through `core::user_dirs` and
reads the environment only through `core::user_env`, both answering from a
per-process scratch directory and per-thread values a test sets. Spawned rtk
children get the same redirect through the environment, and every git the
suite runs is isolated from the developer's configuration and repositories.
rtk-ai#3826's helpers move into `test_isolation`, so there is one `rtk_command`.

Scans refuse a test that names the rtk binary outside the two constructors,
and a direct `dirs::`, environment or working-directory read outside
`user_dirs`/`user_env`.

Production behaviour is unchanged; every redirect sits behind cfg(test).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants