Skip to content

Commit 866b9cb

Browse files
committed
test: add critical test to preserve symlink paths in environment detection
1 parent e600bba commit 866b9cb

File tree

1 file changed

+61
-0
lines changed

1 file changed

+61
-0
lines changed

crates/pet/src/find.rs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,4 +582,65 @@ mod tests {
582582
"Symlink should point to a valid venv"
583583
);
584584
}
585+
586+
/// CRITICAL TEST: Verify that path().is_dir() does NOT resolve symlinks to their target paths.
587+
/// This ensures we use the symlink path (e.g., ~/envs/myenv) not the deep target path
588+
/// (e.g., /some/deep/path/to/actual/venv).
589+
///
590+
/// This is important because:
591+
/// 1. Users expect to see the symlink path in their environment list
592+
/// 2. We don't want to accidentally traverse into deep filesystem locations
593+
/// 3. The symlink path is the "user-facing" path they configured
594+
#[test]
595+
#[cfg(unix)]
596+
fn test_symlink_path_is_preserved_not_resolved() {
597+
use std::os::unix::fs::symlink;
598+
599+
let tmp = TempDir::new().expect("Failed to create temp dir");
600+
601+
// Create a "deep" target directory structure
602+
let deep_target = tmp.path().join("deep").join("nested").join("path").join("venv");
603+
fs::create_dir_all(&deep_target).expect("Failed to create deep target");
604+
605+
// Create a container with a symlink pointing to the deep target
606+
let container_dir = tmp.path().join("envs");
607+
let symlink_path = container_dir.join("my_venv");
608+
fs::create_dir_all(&container_dir).expect("Failed to create container");
609+
symlink(&deep_target, &symlink_path).expect("Failed to create symlink");
610+
611+
// Get the discovered paths using the same pattern as our fix
612+
let discovered: Vec<PathBuf> = fs::read_dir(&container_dir)
613+
.expect("Failed to read dir")
614+
.filter_map(Result::ok)
615+
.filter(|d| d.path().is_dir()) // This follows symlink to CHECK if it's a dir
616+
.map(|d| d.path()) // But this returns the SYMLINK path, not the target
617+
.collect();
618+
619+
assert_eq!(discovered.len(), 1);
620+
621+
// CRITICAL: The path should be the symlink, NOT the resolved target
622+
assert_eq!(
623+
discovered[0], symlink_path,
624+
"Should return the symlink path, not the deep target"
625+
);
626+
627+
// Verify we did NOT get the deep target path
628+
assert_ne!(
629+
discovered[0], deep_target,
630+
"Should NOT resolve to the deep target path"
631+
);
632+
633+
// The path should NOT contain the deep nested structure
634+
assert!(
635+
!discovered[0].to_string_lossy().contains("deep/nested"),
636+
"Path should not contain the deep nested target structure"
637+
);
638+
639+
// Extra verification: fs::canonicalize WOULD resolve it (showing the difference)
640+
let resolved = fs::canonicalize(&discovered[0]).expect("Should resolve");
641+
assert_eq!(
642+
resolved, deep_target,
643+
"canonicalize() would resolve to target, but path() does not"
644+
);
645+
}
585646
}

0 commit comments

Comments
 (0)