Repository navigation
Add TDD skill and fix failing tests #35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| --- | ||
| name: rtk-tdd | ||
| description: > | ||
| Enforces TDD (Red-Green-Refactor) for Rust development. Auto-triggers on | ||
| implementation, testing, refactoring, and bug fixing tasks. Provides | ||
| Rust-idiomatic testing patterns with anyhow/thiserror, cfg(test), and | ||
| Arrange-Act-Assert workflow. | ||
| --- | ||
|
|
||
| # Rust TDD Workflow | ||
|
|
||
| ## Three Laws of TDD | ||
|
|
||
| 1. Do NOT write production code without a failing test | ||
| 2. Write only enough test to fail (including compilation failure) | ||
| 3. Write only enough production code to pass the failing test | ||
|
|
||
| Cycle: **RED** (test fails) -> **GREEN** (minimum to pass) -> **REFACTOR** (cleanup, cargo test) | ||
|
|
||
| ## Red-Green-Refactor Steps | ||
|
|
||
| ``` | ||
| 1. Write test in #[cfg(test)] mod tests of the SAME file | ||
| 2. cargo test MODULE::tests::test_name -- must FAIL (red) | ||
| 3. Implement the minimum in the function | ||
| 4. cargo test MODULE::tests::test_name -- must PASS (green) | ||
| 5. Refactor if needed, re-run cargo test (still green) | ||
| 6. cargo fmt && cargo clippy --all-targets && cargo test (final gate) | ||
| ``` | ||
|
|
||
| Never skip step 2. If the test passes immediately, it tests nothing. | ||
|
|
||
| ## Idiomatic Rust Test Patterns | ||
|
|
||
| | Pattern | Usage | When | | ||
| |---------|-------|------| | ||
| | Arrange-Act-Assert | Base structure for every test | Always | | ||
| | `assert_eq!` / `assert!` | Direct comparison / booleans | Deterministic values | | ||
| | `assert!(result.is_err())` | Error path testing | Invalid inputs | | ||
| | `Result<()>` return type | Tests with `?` operator | Fallible functions | | ||
| | `#[should_panic]` | Expected panic | Invariants, preconditions | | ||
| | `tempfile::NamedTempFile` | File/I/O tests | Filesystem-dependent code | | ||
|
|
||
| ## Patterns by Code Type | ||
|
|
||
| | Code Type | Test Pattern | Example | | ||
| |-----------|-------------|---------| | ||
| | Pure function (str -> str) | Input literal -> assert output | `assert_eq!(truncate("hello", 3), "...")` | | ||
| | Parsing/filtering | Raw string -> filter -> contains/not-contains | `assert!(filter(raw).contains("expected"))` | | ||
| | Validation/security | Boundary inputs -> assert bool | `assert!(!is_valid("../etc/passwd"))` | | ||
| | Error handling | Bad input -> `is_err()` | `assert!(parse("garbage").is_err())` | | ||
| | Struct/enum roundtrip | Construct -> serialize -> deserialize -> eq | `assert_eq!(from_str(to_str(x)), x)` | | ||
|
|
||
| ## Naming Convention | ||
|
|
||
| ``` | ||
| test_{function}_{scenario} | ||
| test_{function}_{input_type} | ||
| ``` | ||
|
|
||
| Examples: `test_truncate_edge_case`, `test_parse_invalid_input`, `test_filter_empty_string` | ||
|
|
||
| ## When NOT to Use Pure TDD | ||
|
|
||
| - Functions calling `Command::new()` -> test the parser, not the execution | ||
| - `std::process::exit()` -> refactor to `Result` first, then test the Result | ||
| - Direct I/O (SQLite, network) -> use tempfile/mock or test the pure logic separately | ||
| - Main/CLI wiring -> covered by integration/smoke tests | ||
|
|
||
| ## Pre-Commit Gate | ||
|
|
||
| ```bash | ||
| cargo fmt --all --check | ||
| cargo clippy --all-targets | ||
| cargo test | ||
| ``` | ||
|
|
||
| All 3 must pass. No exceptions. No `#[allow(...)]` without documented justification. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| # RTK Testing Patterns Reference | ||
|
|
||
| ## Untested Modules Backlog | ||
|
|
||
| Prioritized by testability (pure functions first, I/O-heavy last). | ||
|
|
||
| ### High Priority (pure functions, trivial to test) | ||
|
|
||
| | Module | Testable Functions | Notes | | ||
| |--------|-------------------|-------| | ||
| | `diff_cmd.rs` | `compute_diff`, `similarity`, `truncate`, `condense_unified_diff` | 4 pure functions, 0 tests | | ||
| | `env_cmd.rs` | `mask_value`, `is_lang_var`, `is_cloud_var`, `is_tool_var`, `is_interesting_var` | 5 categorization functions | | ||
|
|
||
| ### Medium Priority (need tempfile or parsed input) | ||
|
|
||
| | Module | Testable Functions | Notes | | ||
| |--------|-------------------|-------| | ||
| | `tracking.rs` | `estimate_tokens`, `Tracker::new`, query methods | Use tempfile for SQLite | | ||
| | `config.rs` | `Config::default`, config parsing | Test default values and TOML parsing | | ||
| | `deps.rs` | Dependency file parsing | Test with sample Cargo.toml/package.json strings | | ||
| | `summary.rs` | Output type detection heuristics | Pure string analysis | | ||
|
|
||
| ### Low Priority (heavy I/O, CLI wiring) | ||
|
|
||
| | Module | Testable Functions | Notes | | ||
| |--------|-------------------|-------| | ||
| | `container.rs` | Docker/kubectl output filters | Requires mocking Command output | | ||
| | `find_cmd.rs` | Directory grouping logic | Filesystem-dependent | | ||
| | `wget_cmd.rs` | `compact_url`, `format_size`, `truncate_line`, `extract_filename_from_output` | Some pure helpers worth testing | | ||
| | `gain.rs` | Display formatting | Depends on tracking DB | | ||
| | `init.rs` | CLAUDE.md generation | File I/O | | ||
| | `main.rs` | CLI routing | Covered by smoke tests | | ||
|
|
||
| ## RTK Test Patterns | ||
|
|
||
| ### Pattern 1: Filter Function (most common in RTK) | ||
|
|
||
| ```rust | ||
| #[test] | ||
| fn test_FILTER_happy_path() { | ||
| // Arrange: raw command output as string literal | ||
| let input = r#" | ||
| line of noise | ||
| line with relevant data | ||
| more noise | ||
| "#; | ||
| // Act | ||
| let result = filter_COMMAND(input); | ||
| // Assert: output contains expected, excludes noise | ||
| assert!(result.contains("relevant data")); | ||
| assert!(!result.contains("noise")); | ||
| } | ||
| ``` | ||
|
|
||
| Used in: `git.rs`, `grep_cmd.rs`, `lint_cmd.rs`, `tsc_cmd.rs`, `vitest_cmd.rs`, `pnpm_cmd.rs`, `next_cmd.rs`, `prettier_cmd.rs`, `playwright_cmd.rs`, `prisma_cmd.rs` | ||
|
|
||
| ### Pattern 2: Pure Computation | ||
|
|
||
| ```rust | ||
| #[test] | ||
| fn test_FUNCTION_deterministic() { | ||
| assert_eq!(truncate("hello world", 8), "hello..."); | ||
| assert_eq!(truncate("short", 10), "short"); | ||
| } | ||
| ``` | ||
|
|
||
| Used in: `gh_cmd.rs` (`truncate`), `utils.rs` (`truncate`, `format_tokens`, `format_usd`) | ||
|
|
||
| ### Pattern 3: Validation / Security | ||
|
|
||
| ```rust | ||
| #[test] | ||
| fn test_VALIDATOR_rejects_injection() { | ||
| assert!(!is_valid("malicious; rm -rf /")); | ||
| assert!(!is_valid("../../../etc/passwd")); | ||
| } | ||
| ``` | ||
|
|
||
| Used in: `pnpm_cmd.rs` (`is_valid_package_name`) | ||
|
|
||
| ### Pattern 4: ANSI Stripping | ||
|
|
||
| ```rust | ||
| #[test] | ||
| fn test_strip_ansi() { | ||
| let input = "\x1b[32mgreen\x1b[0m normal"; | ||
| let output = strip_ansi(input); | ||
| assert_eq!(output, "green normal"); | ||
| assert!(!output.contains("\x1b[")); | ||
| } | ||
| ``` | ||
|
|
||
| Used in: `vitest_cmd.rs`, `utils.rs` | ||
|
|
||
| ## Test Skeleton Template | ||
|
|
||
| ```rust | ||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn test_FUNCTION_happy_path() { | ||
| // Arrange | ||
| let input = r#"..."#; | ||
| // Act | ||
| let result = FUNCTION(input); | ||
| // Assert | ||
| assert!(result.contains("expected")); | ||
| assert!(!result.contains("noise")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_FUNCTION_empty_input() { | ||
| let result = FUNCTION(""); | ||
| assert!(...); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_FUNCTION_edge_case() { | ||
| // Boundary conditions: very long input, special chars, unicode | ||
| } | ||
| } | ||
| ``` | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -187,16 +187,25 @@ main.rs:Commands enum | |||||
|
|
||||||
| ## Testing Strategy | ||||||
|
|
||||||
| Tests are embedded in modules using `#[cfg(test)] mod tests`: | ||||||
| - Unit tests validate filtering logic (filter.rs, grep_cmd.rs, etc.) | ||||||
| - Integration tests verify command output transformations (git.rs, runner.rs) | ||||||
| - Security tests ensure proper command sanitization (pnpm validation) | ||||||
| ### TDD Workflow (mandatory) | ||||||
| All code follows Red-Green-Refactor. See `.claude/skills/rtk-tdd/` for the full workflow and Rust-idiomatic patterns. See `.claude/skills/rtk-tdd/references/testing-patterns.md` for RTK-specific patterns and untested module backlog. | ||||||
|
|
||||||
| Run module-specific tests: | ||||||
| ### Test Architecture | ||||||
| - **Unit tests**: Embedded `#[cfg(test)] mod tests` in each module (105+ tests, 25+ files) | ||||||
|
||||||
| - **Unit tests**: Embedded `#[cfg(test)] mod tests` in each module (105+ tests, 25+ files) | |
| - **Unit tests**: Embedded `#[cfg(test)] mod tests` in each module (120+ tests, 25+ files) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -184,3 +184,158 @@ fn condense_unified_diff(diff: &str) -> String { | |
|
|
||
| result.join("\n") | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| // --- similarity --- | ||
|
|
||
| #[test] | ||
| fn test_similarity_identical() { | ||
| assert_eq!(similarity("hello", "hello"), 1.0); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_similarity_completely_different() { | ||
| assert_eq!(similarity("abc", "xyz"), 0.0); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_similarity_empty_strings() { | ||
| // Both empty: union is 0, returns 1.0 by convention | ||
| assert_eq!(similarity("", ""), 1.0); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_similarity_partial_overlap() { | ||
| let s = similarity("abcd", "abef"); | ||
| // Shared: a, b. Union: a, b, c, d, e, f = 6. Jaccard = 2/6 | ||
| assert!((s - 2.0 / 6.0).abs() < f64::EPSILON); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_similarity_threshold_for_modified() { | ||
| // "let x = 1;" vs "let x = 2;" should be > 0.5 (treated as modification) | ||
| assert!(similarity("let x = 1;", "let x = 2;") > 0.5); | ||
| } | ||
|
|
||
| // --- truncate --- | ||
|
|
||
| #[test] | ||
| fn test_truncate_short_string() { | ||
| assert_eq!(truncate("hello", 10), "hello"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_truncate_exact_length() { | ||
| assert_eq!(truncate("hello", 5), "hello"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_truncate_long_string() { | ||
| assert_eq!(truncate("hello world!", 8), "hello..."); | ||
| } | ||
|
Comment on lines
+225
to
+238
|
||
|
|
||
| // --- compute_diff --- | ||
|
|
||
| #[test] | ||
| fn test_compute_diff_identical() { | ||
| let a = vec!["line1", "line2", "line3"]; | ||
| let b = vec!["line1", "line2", "line3"]; | ||
| let result = compute_diff(&a, &b); | ||
| assert_eq!(result.added, 0); | ||
| assert_eq!(result.removed, 0); | ||
| assert_eq!(result.modified, 0); | ||
| assert!(result.changes.is_empty()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_compute_diff_added_lines() { | ||
| let a = vec!["line1"]; | ||
| let b = vec!["line1", "line2", "line3"]; | ||
| let result = compute_diff(&a, &b); | ||
| assert_eq!(result.added, 2); | ||
| assert_eq!(result.removed, 0); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_compute_diff_removed_lines() { | ||
| let a = vec!["line1", "line2", "line3"]; | ||
| let b = vec!["line1"]; | ||
| let result = compute_diff(&a, &b); | ||
| assert_eq!(result.removed, 2); | ||
| assert_eq!(result.added, 0); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_compute_diff_modified_line() { | ||
| // Similar lines (>0.5 similarity) are classified as modified | ||
| let a = vec!["let x = 1;"]; | ||
| let b = vec!["let x = 2;"]; | ||
| let result = compute_diff(&a, &b); | ||
| assert_eq!(result.modified, 1); | ||
| assert_eq!(result.added, 0); | ||
| assert_eq!(result.removed, 0); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_compute_diff_completely_different_line() { | ||
| // Dissimilar lines (<= 0.5 similarity) are added+removed, not modified | ||
| let a = vec!["aaaa"]; | ||
| let b = vec!["zzzz"]; | ||
| let result = compute_diff(&a, &b); | ||
| assert_eq!(result.modified, 0); | ||
| assert_eq!(result.added, 1); | ||
| assert_eq!(result.removed, 1); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_compute_diff_empty_inputs() { | ||
| let result = compute_diff(&[], &[]); | ||
| assert_eq!(result.added, 0); | ||
| assert_eq!(result.removed, 0); | ||
| assert!(result.changes.is_empty()); | ||
| } | ||
|
|
||
| // --- condense_unified_diff --- | ||
|
|
||
| #[test] | ||
| fn test_condense_unified_diff_single_file() { | ||
| let diff = r#"diff --git a/src/main.rs b/src/main.rs | ||
| --- a/src/main.rs | ||
| +++ b/src/main.rs | ||
| @@ -1,3 +1,4 @@ | ||
| fn main() { | ||
| + println!("hello"); | ||
| println!("world"); | ||
| } | ||
| "#; | ||
| let result = condense_unified_diff(diff); | ||
| assert!(result.contains("src/main.rs")); | ||
| assert!(result.contains("+1")); | ||
| assert!(result.contains("println")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_condense_unified_diff_multiple_files() { | ||
| let diff = r#"diff --git a/a.rs b/a.rs | ||
| --- a/a.rs | ||
| +++ b/a.rs | ||
| +added line | ||
| diff --git a/b.rs b/b.rs | ||
| --- a/b.rs | ||
| +++ b/b.rs | ||
| -removed line | ||
| "#; | ||
| let result = condense_unified_diff(diff); | ||
| assert!(result.contains("a.rs")); | ||
| assert!(result.contains("b.rs")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_condense_unified_diff_empty() { | ||
| let result = condense_unified_diff(""); | ||
| assert!(result.is_empty()); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line states that diff_cmd.rs has "4 pure functions, 0 tests", but this PR adds 17 tests to diff_cmd.rs. This entry should be removed from the untested modules backlog or updated to reflect that tests have now been added.
The line should either be deleted entirely or moved to a "Recently Added Coverage" section to track progress.