From 951a1fa82bc6db649203b8b626507b14c052dda1 Mon Sep 17 00:00:00 2001 From: rhishi99 Date: Thu, 20 Aug 2026 23:22:57 +0530 Subject: [PATCH] test(windows): stop skipping the whole integration suite on Windows Five of the six files in `tests/` open with `#![cfg(unix)]`, so the Windows CI job compiled the binary and then exercised none of it. Every Windows regression the suite could have caught shipped instead. The gate was never really about Unix. Each of these tests shells out to `grep` or `rg` and already skips itself when that engine is missing, so the files run as-is wherever an engine exists -- including Git Bash, scoop, and WSL-adjacent setups, which is most Windows developer machines. Where no engine is installed the tests skip exactly as they already do on a Linux box without ripgrep. Two supporting changes: - `search_error_test` pins `CLAUDE_CONFIG_DIR` at a path that does not exist. Its assertions require empty stderr, which a developer with Claude Code installed but without the rtk hook cannot satisfy. CI passes either way only because a fresh runner has no `~/.claude`. - Renames a `nul.txt` fixture to `nul_bytes.txt`. `nul` is a reserved device name on Windows and cannot be created as a file. Adds `tests/windows_smoke_test.rs` for the behaviours that are genuinely Windows-specific rather than merely untested: CRLF line endings, backslash paths, paths containing spaces, and which shell `rtk summary` hands a command to. Co-Authored-By: Claude Opus 5 --- tests/grep_context_test.rs | 8 +- tests/grep_faithful_format_test.rs | 4 +- tests/search_compress_test.rs | 4 +- tests/search_error_test.rs | 14 ++- tests/search_faithful_test.rs | 4 +- tests/windows_smoke_test.rs | 146 +++++++++++++++++++++++++++++ 6 files changed, 173 insertions(+), 7 deletions(-) create mode 100644 tests/windows_smoke_test.rs diff --git a/tests/grep_context_test.rs b/tests/grep_context_test.rs index 80f5525c48..61c46da034 100644 --- a/tests/grep_context_test.rs +++ b/tests/grep_context_test.rs @@ -1,4 +1,6 @@ -#![cfg(unix)] +//! Runs on every platform: each test that shells out to `grep` or `rg` +//! skips itself when that engine is not installed, so Windows CI exercises +//! the filters instead of compiling an empty test binary. //! Regressions from the #2333 grep overhaul: -A/-B/-C must keep context lines, //! -c/-o must not leak ripgrep's NUL separator. @@ -66,7 +68,7 @@ fn count_output_has_no_nul_separator() { return; } let dir = tempfile::tempdir().unwrap(); - let f = dir.path().join("nul.txt"); + let f = dir.path().join("nul_bytes.txt"); std::fs::write(&f, "TODO one\nnope\nTODO two\n").unwrap(); let out = rtk() .args(["grep", "-c", "TODO", f.to_str().unwrap()]) @@ -149,7 +151,7 @@ fn only_matching_output_has_no_nul_separator() { return; } let dir = tempfile::tempdir().unwrap(); - let f = dir.path().join("nul.txt"); + let f = dir.path().join("nul_bytes.txt"); std::fs::write(&f, "TODO one\nnope\nTODO two\n").unwrap(); let out = rtk() .args(["grep", "-o", "TODO", f.to_str().unwrap()]) diff --git a/tests/grep_faithful_format_test.rs b/tests/grep_faithful_format_test.rs index cf56fa7d29..da9bac6d26 100644 --- a/tests/grep_faithful_format_test.rs +++ b/tests/grep_faithful_format_test.rs @@ -1,4 +1,6 @@ -#![cfg(unix)] +//! Runs on every platform: each test that shells out to `grep` or `rg` +//! skips itself when that engine is not installed, so Windows CI exercises +//! the filters instead of compiling an empty test binary. //! For un-capped searches `rtk grep X` must be byte-identical to `grep X`. //! Every scenario runs twice, with and without `-n`, so both the presence and //! the absence of the line number are pinned: grep prints one only when asked, diff --git a/tests/search_compress_test.rs b/tests/search_compress_test.rs index 74c55aed8c..c00181feaa 100644 --- a/tests/search_compress_test.rs +++ b/tests/search_compress_test.rs @@ -1,4 +1,6 @@ -#![cfg(unix)] +//! Runs on every platform: each test that shells out to `grep` or `rg` +//! skips itself when that engine is not installed, so Windows CI exercises +//! the filters instead of compiling an empty test binary. //! Integration tests for the shared grep/rg compression filter: the GROUP path //! with context flags (-A/-B/-C) and the safety-net passthrough for flags (some //! grep-only like -I, some rg-only like --heading/-p) that break the NUL reparse. diff --git a/tests/search_error_test.rs b/tests/search_error_test.rs index 0d886dcc75..073a66d37d 100644 --- a/tests/search_error_test.rs +++ b/tests/search_error_test.rs @@ -1,4 +1,6 @@ -#![cfg(unix)] +//! Runs on every platform: each test that shells out to `grep` or `rg` +//! skips itself when that engine is not installed, so Windows CI exercises +//! the filters instead of compiling an empty test binary. //! Error/exit faithfulness: rtk surfaces the engine's own stderr and propagates //! its exit code, adding nothing (no synthetic "search failed" line). An engine //! error (exit >=2) must never look like a silent no-match (#2465 / #1436). @@ -8,6 +10,16 @@ use std::process::Command; fn rtk(args: &[&str]) -> std::process::Output { Command::new(env!("CARGO_BIN_EXE_rtk")) .env("LC_ALL", "C") + // Point the hook check at a directory that does not exist, so it reports + // "no Claude Code here" and stays silent. Without this, a developer who + // has Claude Code installed but not the rtk hook gets rtk's once-a-day + // "No hook installed" nag on stderr, and every assertion below that + // requires empty stderr fails. CI passes either way only because a + // fresh runner has no `~/.claude` at all. + .env( + "CLAUDE_CONFIG_DIR", + concat!(env!("CARGO_TARGET_TMPDIR"), "/absent-claude-config"), + ) .args(args) .output() .expect("rtk") diff --git a/tests/search_faithful_test.rs b/tests/search_faithful_test.rs index 08417a42bc..a77607814f 100644 --- a/tests/search_faithful_test.rs +++ b/tests/search_faithful_test.rs @@ -1,4 +1,6 @@ -#![cfg(unix)] +//! Runs on every platform: each test that shells out to `grep` or `rg` +//! skips itself when that engine is not installed, so Windows CI exercises +//! the filters instead of compiling an empty test binary. //! Engine-faithfulness contract: `rtk grep` runs grep and `rtk rg` runs rg, each //! with its own regex dialect and ignore semantics. rtk filters output noise only; //! it never substitutes one engine for the other (the bug this PR removes). diff --git a/tests/windows_smoke_test.rs b/tests/windows_smoke_test.rs new file mode 100644 index 0000000000..5ae563ceb6 --- /dev/null +++ b/tests/windows_smoke_test.rs @@ -0,0 +1,146 @@ +#![cfg(windows)] +//! Windows-only end-to-end coverage. +//! +//! The rest of `tests/` is gated `#![cfg(unix)]`, so before this file the +//! Windows CI target compiled the binary and then exercised none of it. These +//! are smoke tests, not exhaustive ones: they pin the behaviours that are +//! genuinely different on Windows — CRLF line endings, backslash paths, paths +//! containing spaces, and which shell `rtk summary` hands a command to. + +use std::io::Write; +use std::path::Path; +use std::process::{Command, Output, Stdio}; + +/// Run rtk with its tracking DB redirected into `dir` and telemetry off, so a +/// test run never writes to the developer's own history. (`RTK_DATA_DIR` is a +/// compile-time constant, not an environment variable, so the config file +/// location cannot be redirected — these tests only ever read it.) +fn rtk_in(dir: &Path) -> Command { + let mut c = Command::new(env!("CARGO_BIN_EXE_rtk")); + c.env("RTK_DB_PATH", dir.join("history.db")) + .env("RTK_TELEMETRY_DISABLED", "1"); + c +} + +fn stdout_of(out: &Output) -> String { + String::from_utf8_lossy(&out.stdout).into_owned() +} + +fn run_with_stdin(command: &mut Command, input: &[u8]) -> Output { + let mut child = command + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn rtk"); + child + .stdin + .take() + .expect("piped stdin") + .write_all(input) + .expect("write stdin"); + child.wait_with_output().expect("wait for rtk") +} + +// --- basic binary health ------------------------------------------------- + +#[test] +fn version_runs_on_windows() { + let dir = tempfile::tempdir().unwrap(); + let out = rtk_in(dir.path()).arg("--version").output().unwrap(); + assert!(out.status.success(), "rtk --version failed"); + assert!( + stdout_of(&out).contains(env!("CARGO_PKG_VERSION")), + "version output missing crate version: {}", + stdout_of(&out) + ); +} + +// --- CRLF handling ------------------------------------------------------- + +#[test] +fn wc_counts_crlf_lines_from_stdin() { + let dir = tempfile::tempdir().unwrap(); + let out = run_with_stdin(rtk_in(dir.path()).args(["wc", "-l"]), b"alpha\r\nbeta\r\n"); + assert!(out.status.success(), "rtk wc -l failed"); + assert_eq!(stdout_of(&out).trim(), "2"); +} + +#[test] +fn read_does_not_drop_crlf_content() { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("crlf.txt"); + std::fs::write(&file, "first\r\nsecond\r\nthird\r\n").unwrap(); + + let out = rtk_in(dir.path()) + .args(["read", file.to_str().unwrap()]) + .output() + .unwrap(); + assert!(out.status.success(), "rtk read failed"); + + let text = stdout_of(&out); + for expected in ["first", "second", "third"] { + assert!(text.contains(expected), "missing {expected} in:\n{text}"); + } +} + +// --- Windows paths ------------------------------------------------------- + +#[test] +fn ls_handles_backslash_path_with_spaces() { + let dir = tempfile::tempdir().unwrap(); + let spaced = dir.path().join("Program Files Test"); + std::fs::create_dir(&spaced).unwrap(); + std::fs::write(spaced.join("alpha.txt"), "a").unwrap(); + std::fs::write(spaced.join("beta.txt"), "b").unwrap(); + + let arg = spaced.to_str().unwrap(); + assert!(arg.contains('\\'), "temp path was not a backslash path"); + + let out = rtk_in(dir.path()).args(["ls", arg]).output().unwrap(); + assert!(out.status.success(), "rtk ls failed on {arg}"); + + let text = stdout_of(&out); + assert!(text.contains("alpha.txt"), "alpha.txt missing in:\n{text}"); + assert!(text.contains("beta.txt"), "beta.txt missing in:\n{text}"); +} + +// --- shell selection (see src/core/shell.rs) ----------------------------- + +#[test] +fn summary_runs_cmd_builtins() { + let dir = tempfile::tempdir().unwrap(); + let out = rtk_in(dir.path()) + .args(["summary", "echo RTK_CMD_OK"]) + .output() + .unwrap(); + assert!(out.status.success(), "rtk summary of a cmd builtin failed"); + assert!( + stdout_of(&out).contains("RTK_CMD_OK"), + "cmd builtin output missing:\n{}", + stdout_of(&out) + ); +} + +// --- hooks --------------------------------------------------------------- + +#[test] +fn init_show_reports_configuration_without_writing() { + let dir = tempfile::tempdir().unwrap(); + let out = rtk_in(dir.path()) + .current_dir(dir.path()) + // Report on a throwaway config dir rather than the developer's own. + .env("CLAUDE_CONFIG_DIR", dir.path().join(".claude")) + .args(["init", "--show"]) + .output() + .unwrap(); + assert!( + out.status.success(), + "rtk init --show failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + !dir.path().join("CLAUDE.md").exists(), + "init --show must not write CLAUDE.md" + ); +}