Skip to content

Commit 7d0de48

Browse files
max-sixtyClaude
andauthored
Fix source path in snapshots for non-child workspaces (#778)
Adds a helper function to compute relative paths between arbitrary directories. This is used to ensure the `source` field in snapshots is relative to the workspace root, even when the project is not a direct child of the workspace directory. This resolves issue #777 where the source path would become absolute in such setups. Includes tests for the fix. Co-authored-by: Claude <[email protected]> Co-authored-by: Claude <[email protected]>
1 parent 0eea607 commit 7d0de48

File tree

4 files changed

+284
-3
lines changed

4 files changed

+284
-3
lines changed

Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cargo-insta/tests/functional/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ use tempfile::TempDir;
7171
mod binary;
7272
mod delete_pending;
7373
mod inline;
74+
mod test_workspace_source_path;
7475
mod unreferenced;
7576
mod workspace;
7677

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
use crate::TestFiles;
2+
use std::fs;
3+
4+
/// Test for issue #777: Insta "source" in snapshot is full absolute path when workspace is not parent
5+
#[test]
6+
fn test_workspace_source_path_issue_777() {
7+
// Create a workspace structure where project is not a child of workspace
8+
// This reproduces the exact issue from #777
9+
let test_project = TestFiles::new()
10+
.add_file(
11+
"workspace/Cargo.toml",
12+
r#"
13+
[workspace]
14+
resolver = "2"
15+
members = ["../project1"]
16+
"#
17+
.to_string(),
18+
)
19+
.add_file(
20+
"project1/Cargo.toml",
21+
r#"
22+
[package]
23+
name = "project1"
24+
version = "0.1.0"
25+
edition = "2021"
26+
27+
workspace = "../workspace"
28+
29+
[dependencies]
30+
insta = { path = '$PROJECT_PATH', features = ["yaml"] }
31+
"#
32+
.to_string(),
33+
)
34+
.add_file(
35+
"project1/src/lib.rs",
36+
r#"
37+
#[test]
38+
fn test_something() {
39+
insta::assert_yaml_snapshot!(vec![1, 2, 3]);
40+
}
41+
"#
42+
.to_string(),
43+
)
44+
.create_project();
45+
46+
// Run test to create snapshot from within project1 directory
47+
// This should trigger the issue where source path becomes absolute
48+
let output = test_project
49+
.insta_cmd()
50+
.current_dir(test_project.workspace_dir.join("project1"))
51+
// Set workspace root to the actual workspace directory
52+
.env(
53+
"INSTA_WORKSPACE_ROOT",
54+
test_project.workspace_dir.join("workspace"),
55+
)
56+
.args(["test", "--accept"])
57+
.output()
58+
.unwrap();
59+
60+
assert!(output.status.success());
61+
62+
// Read the generated snapshot
63+
let snapshot_path = test_project
64+
.workspace_dir
65+
.join("project1/src/snapshots/project1__something.snap");
66+
67+
let snapshot_content = fs::read_to_string(&snapshot_path).unwrap();
68+
69+
// Parse the snapshot to check the source field
70+
let source_line = snapshot_content
71+
.lines()
72+
.find(|line| line.starts_with("source:"))
73+
.expect("source line not found");
74+
75+
let source_path = source_line
76+
.strip_prefix("source: ")
77+
.expect("invalid source line")
78+
.trim()
79+
.trim_matches('"');
80+
81+
// The source path should be relative and start with ../ (since workspace and project are siblings)
82+
assert!(
83+
source_path.starts_with("../"),
84+
"Source path should be relative starting with '../', but got: {}",
85+
source_path
86+
);
87+
88+
// The path should be exactly ../project1/src/lib.rs
89+
assert_eq!(
90+
source_path, "../project1/src/lib.rs",
91+
"Expected simplified relative path"
92+
);
93+
}
94+
95+
/// Test that the fix works with a more complex workspace structure
96+
#[test]
97+
fn test_workspace_source_path_complex() {
98+
// Create a complex workspace structure
99+
let test_project = TestFiles::new()
100+
.add_file(
101+
"code/workspace/Cargo.toml",
102+
r#"
103+
[workspace]
104+
resolver = "2"
105+
members = ["../../projects/app1", "../../projects/app2"]
106+
"#
107+
.to_string(),
108+
)
109+
.add_file(
110+
"projects/app1/Cargo.toml",
111+
r#"
112+
[package]
113+
name = "app1"
114+
version = "0.1.0"
115+
edition = "2021"
116+
117+
workspace = "../../code/workspace"
118+
119+
[dependencies]
120+
insta = { path = '$PROJECT_PATH', features = ["yaml"] }
121+
"#
122+
.to_string(),
123+
)
124+
.add_file(
125+
"projects/app1/src/lib.rs",
126+
r#"
127+
#[test]
128+
fn test_app1() {
129+
insta::assert_yaml_snapshot!(vec!["app1"]);
130+
}
131+
"#
132+
.to_string(),
133+
)
134+
.add_file(
135+
"projects/app2/Cargo.toml",
136+
r#"
137+
[package]
138+
name = "app2"
139+
version = "0.1.0"
140+
edition = "2021"
141+
142+
workspace = "../../code/workspace"
143+
144+
[dependencies]
145+
insta = { path = '$PROJECT_PATH', features = ["yaml"] }
146+
"#
147+
.to_string(),
148+
)
149+
.add_file(
150+
"projects/app2/src/lib.rs",
151+
r#"
152+
#[test]
153+
fn test_app2() {
154+
insta::assert_yaml_snapshot!(vec!["app2"]);
155+
}
156+
"#
157+
.to_string(),
158+
)
159+
.create_project();
160+
161+
// Run tests for both projects
162+
let output1 = test_project
163+
.insta_cmd()
164+
.current_dir(test_project.workspace_dir.join("projects/app1"))
165+
.args(["test", "--accept"])
166+
.output()
167+
.unwrap();
168+
169+
assert!(output1.status.success());
170+
171+
let output2 = test_project
172+
.insta_cmd()
173+
.current_dir(test_project.workspace_dir.join("projects/app2"))
174+
.args(["test", "--accept"])
175+
.output()
176+
.unwrap();
177+
178+
assert!(output2.status.success());
179+
180+
// Check both snapshots
181+
let snapshot1_path = test_project
182+
.workspace_dir
183+
.join("projects/app1/src/snapshots/app1__app1.snap");
184+
let snapshot1_content = fs::read_to_string(&snapshot1_path).unwrap();
185+
186+
let snapshot2_path = test_project
187+
.workspace_dir
188+
.join("projects/app2/src/snapshots/app2__app2.snap");
189+
let snapshot2_content = fs::read_to_string(&snapshot2_path).unwrap();
190+
191+
// Neither snapshot should contain absolute paths
192+
assert!(
193+
!snapshot1_content.contains(&test_project.workspace_dir.to_string_lossy().to_string()),
194+
"App1 snapshot contains absolute path"
195+
);
196+
assert!(
197+
!snapshot2_content.contains(&test_project.workspace_dir.to_string_lossy().to_string()),
198+
"App2 snapshot contains absolute path"
199+
);
200+
201+
// Both should have relative paths
202+
assert!(
203+
snapshot1_content.contains("source: \"../../projects/app1/src/lib.rs\""),
204+
"App1 snapshot source is not the expected relative path. Got:\n{}",
205+
snapshot1_content
206+
);
207+
assert!(
208+
snapshot2_content.contains("source: \"../../projects/app2/src/lib.rs\""),
209+
"App2 snapshot source is not the expected relative path. Got:\n{}",
210+
snapshot2_content
211+
);
212+
}

insta/src/runtime.rs

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,27 @@ impl<'a> SnapshotAssertionContext<'a> {
436436
self.module_path.replace("::", "__"),
437437
self.snapshot_name.as_ref().map(|x| x.to_string()),
438438
Settings::with(|settings| MetaData {
439-
source: Some(path_to_storage(Path::new(self.assertion_file))),
439+
source: {
440+
let source_path = Path::new(self.assertion_file);
441+
// We need to compute a relative path from the workspace to the source file.
442+
// This is necessary for workspace setups where the project is not a direct
443+
// child of the workspace root (e.g., when workspace and project are siblings).
444+
// We canonicalize paths first to properly handle symlinks.
445+
let canonicalized_base = self.workspace.canonicalize().ok();
446+
let canonicalized_path = source_path.canonicalize().ok();
447+
448+
let relative = if let (Some(base), Some(path)) =
449+
(canonicalized_base, canonicalized_path)
450+
{
451+
path_relative_from(&path, &base)
452+
.unwrap_or_else(|| source_path.to_path_buf())
453+
} else {
454+
// If canonicalization fails, try with original paths
455+
path_relative_from(source_path, self.workspace)
456+
.unwrap_or_else(|| source_path.to_path_buf())
457+
};
458+
Some(path_to_storage(&relative))
459+
},
440460
assertion_line: Some(self.assertion_line),
441461
description: settings.description().map(Into::into),
442462
expression: if settings.omit_expression() {
@@ -685,6 +705,54 @@ impl<'a> SnapshotAssertionContext<'a> {
685705
}
686706
}
687707

708+
/// Computes a relative path from `base` to `path`, returning a path with `../` components
709+
/// if necessary.
710+
///
711+
/// This function is vendored from the old Rust standard library implementation
712+
/// (pre-1.0, removed in RFC 474) and is distributed under the same terms as the
713+
/// Rust project (MIT/Apache-2.0 dual license).
714+
///
715+
/// Unlike `Path::strip_prefix`, this function can handle cases where `path` is not
716+
/// a descendant of `base`, making it suitable for finding relative paths between
717+
/// arbitrary directories (e.g., between sibling directories in a workspace).
718+
fn path_relative_from(path: &Path, base: &Path) -> Option<PathBuf> {
719+
use std::path::Component;
720+
721+
if path.is_absolute() != base.is_absolute() {
722+
if path.is_absolute() {
723+
Some(PathBuf::from(path))
724+
} else {
725+
None
726+
}
727+
} else {
728+
let mut ita = path.components();
729+
let mut itb = base.components();
730+
let mut comps: Vec<Component> = vec![];
731+
loop {
732+
match (ita.next(), itb.next()) {
733+
(None, None) => break,
734+
(Some(a), None) => {
735+
comps.push(a);
736+
comps.extend(ita.by_ref());
737+
break;
738+
}
739+
(None, _) => comps.push(Component::ParentDir),
740+
(Some(a), Some(b)) if comps.is_empty() && a == b => {}
741+
(Some(a), Some(_b)) => {
742+
comps.push(Component::ParentDir);
743+
for _ in itb {
744+
comps.push(Component::ParentDir);
745+
}
746+
comps.push(a);
747+
comps.extend(ita.by_ref());
748+
break;
749+
}
750+
}
751+
}
752+
Some(comps.iter().map(|c| c.as_os_str()).collect())
753+
}
754+
}
755+
688756
fn prevent_inline_duplicate(function_name: &str, assertion_file: &str, assertion_line: u32) {
689757
let key = format!("{}|{}|{}", function_name, assertion_file, assertion_line);
690758
let mut set = INLINE_DUPLICATES.lock().unwrap();

0 commit comments

Comments
 (0)