|
1 | 1 | # Lash Development Log |
2 | 2 |
|
| 3 | +## 2025-11-23 - Fix Remaining Windows CI Issues (Clippy Warning + Hardcoded Paths) |
| 4 | + |
| 5 | +### Summary |
| 6 | +Resolved two additional Windows CI issues identified in root cause analysis: a clippy warning from the previous fix and hardcoded Unix paths in config tests. |
| 7 | + |
| 8 | +**Commit:** `8a5ca50` |
| 9 | + |
| 10 | +### Issues Fixed |
| 11 | + |
| 12 | +#### Root Cause #2: Redundant .to_string() After .replace() |
| 13 | +**Problem:** |
| 14 | +- The previous Windows path fix (commit `7b966dc`) introduced a clippy warning |
| 15 | +- Code called `.replace('\\', "/").to_string()` |
| 16 | +- The `.replace()` method already returns a `String`, making `.to_string()` redundant |
| 17 | + |
| 18 | +**Solution:** |
| 19 | +- Simplified to `.replace('\\', "/")` removing the redundant call |
| 20 | +- File: `crates/lash-db/src/walker.rs:671` |
| 21 | + |
| 22 | +**Before:** |
| 23 | +```rust |
| 24 | +.map(|f| { |
| 25 | + f.relative_path |
| 26 | + .to_string_lossy() |
| 27 | + .replace('\\', "/") |
| 28 | + .to_string() // Redundant! |
| 29 | +}) |
| 30 | +``` |
| 31 | + |
| 32 | +**After:** |
| 33 | +```rust |
| 34 | +.map(|f| { |
| 35 | + f.relative_path.to_string_lossy().replace('\\', "/") |
| 36 | +}) |
| 37 | +``` |
| 38 | + |
| 39 | +#### Root Cause #3: Hardcoded /tmp Paths in Config Tests |
| 40 | +**Problem:** |
| 41 | +- Three tests used hardcoded `/tmp` path which doesn't exist on Windows |
| 42 | +- Tests: `test_config_builder`, `test_invalid_max_depth`, `test_invalid_indent_spaces` |
| 43 | +- Windows doesn't have a `/tmp` directory, causing test failures |
| 44 | +- File: `crates/lash-types/src/config.rs` (lines 319, 332, 343) |
| 45 | + |
| 46 | +**Solution:** |
| 47 | +- Replaced hardcoded `/tmp` with `TempDir::new()` from `tempfile` crate |
| 48 | +- Used cross-platform temporary directory creation |
| 49 | +- Pattern already existed in `test_find_project_root` at line 350 |
| 50 | + |
| 51 | +**Example Before:** |
| 52 | +```rust |
| 53 | +#[test] |
| 54 | +fn test_config_builder() { |
| 55 | + let config = ConfigBuilder::new() |
| 56 | + .root("/tmp") // Fails on Windows! |
| 57 | + .max_depth(4) |
| 58 | + .indent_spaces(4) |
| 59 | + .build(); |
| 60 | + // ... |
| 61 | +} |
| 62 | +``` |
| 63 | + |
| 64 | +**Example After:** |
| 65 | +```rust |
| 66 | +#[test] |
| 67 | +fn test_config_builder() { |
| 68 | + let temp_dir = TempDir::new().unwrap(); |
| 69 | + let config = ConfigBuilder::new() |
| 70 | + .root(temp_dir.path()) // Cross-platform! |
| 71 | + .max_depth(4) |
| 72 | + .indent_spaces(4) |
| 73 | + .build(); |
| 74 | + // ... |
| 75 | +} |
| 76 | +``` |
| 77 | + |
| 78 | +### Verification |
| 79 | +- All local tests pass (1,065 tests total) |
| 80 | +- No clippy warnings |
| 81 | +- Changes maintain existing test behavior while adding cross-platform compatibility |
| 82 | +- Pre-commit hooks pass successfully |
| 83 | + |
| 84 | +### Platform Compatibility Impact |
| 85 | +These fixes, combined with the previous path separator fix, should resolve all Windows CI failures: |
| 86 | +- Ubuntu: Already passing |
| 87 | +- macOS: Already passing |
| 88 | +- Windows: Should now pass (pending CI verification) |
| 89 | + |
| 90 | +--- |
| 91 | + |
3 | 92 | ## 2025-11-23 - Fix Windows CI Test Failure (Path Separator Issue) |
4 | 93 |
|
5 | 94 | ### Summary |
|
0 commit comments