Skip to content

Commit 4338ee0

Browse files
feat(snapshots): Add --all-image-file-names and --all-image-file-names-file flags (#3312)
Add two mutually exclusive flags to `snapshots upload` that allow selective builds to provide a complete list of image file names from the full test suite. The backend uses this list to distinguish genuinely removed images from images that were simply not included in a selective upload. - `--all-image-file-names <NAMES>` accepts a comma-separated list inline - `--all-image-file-names-file <PATH>` reads one name per line from a file - Either flag implicitly enables `--selective` - CLI validates that every discovered image appears in the list before uploading Companion to [sentry#113006](getsentry/sentry#113006) which adds the backend support for `all_image_file_names`. Follows up on [#3268](#3268) which added the `--selective` flag. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent fafcbbd commit 4338ee0

4 files changed

Lines changed: 167 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
- (snapshots) Add `snapshots diff` command for locally comparing directories of PNG snapshot images using odiff ([#3306](https://github.com/getsentry/sentry-cli/pull/3306))
88
- (snapshots) Add `snapshots download` command for downloading baseline snapshot images from Sentry ([#3310](https://github.com/getsentry/sentry-cli/pull/3310))
9+
- (snapshots) Add `--all-image-file-names` and `--all-image-file-names-file` flags to `snapshots upload` for detecting image removals and renames in selective builds ([#3312](https://github.com/getsentry/sentry-cli/pull/3312))
910

1011
### Fixes
1112

src/api/data_types/snapshots.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ pub struct SnapshotsManifest<'a> {
3333
/// Removals and renames will not be detected on PRs.
3434
#[serde(skip_serializing_if = "std::ops::Not::not")]
3535
pub selective: bool,
36+
#[serde(skip_serializing_if = "Option::is_none")]
37+
pub all_image_file_names: Option<Vec<String>>,
3638
#[serde(flatten)]
3739
pub vcs_info: VcsInfo<'a>,
3840
}
@@ -83,6 +85,7 @@ mod tests {
8385
images: HashMap::new(),
8486
diff_threshold: None,
8587
selective: false,
88+
all_image_file_names: None,
8689
vcs_info: empty_vcs_info(),
8790
};
8891
let json = serde_json::to_value(&manifest).unwrap();
@@ -96,12 +99,44 @@ mod tests {
9699
images: HashMap::new(),
97100
diff_threshold: None,
98101
selective: true,
102+
all_image_file_names: None,
99103
vcs_info: empty_vcs_info(),
100104
};
101105
let json = serde_json::to_value(&manifest).unwrap();
102106
assert_eq!(json["selective"], json!(true));
103107
}
104108

109+
#[test]
110+
fn manifest_omits_all_image_file_names_when_none() {
111+
let manifest = SnapshotsManifest {
112+
app_id: "app".into(),
113+
images: HashMap::new(),
114+
diff_threshold: None,
115+
selective: true,
116+
all_image_file_names: None,
117+
vcs_info: empty_vcs_info(),
118+
};
119+
let json = serde_json::to_value(&manifest).unwrap();
120+
assert!(!json
121+
.as_object()
122+
.unwrap()
123+
.contains_key("all_image_file_names"));
124+
}
125+
126+
#[test]
127+
fn manifest_includes_all_image_file_names_when_some() {
128+
let manifest = SnapshotsManifest {
129+
app_id: "app".into(),
130+
images: HashMap::new(),
131+
diff_threshold: None,
132+
selective: true,
133+
all_image_file_names: Some(vec!["a.png".into(), "b.png".into()]),
134+
vcs_info: empty_vcs_info(),
135+
};
136+
let json = serde_json::to_value(&manifest).unwrap();
137+
assert_eq!(json["all_image_file_names"], json!(["a.png", "b.png"]));
138+
}
139+
105140
#[test]
106141
fn cli_managed_fields_override_sidecar_fields() {
107142
let extra = serde_json::from_value(json!({

src/commands/snapshots/upload.rs

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,30 @@ pub fn make_command(command: Command) -> Command {
7777
Removals and renames cannot be detected on PRs.",
7878
),
7979
)
80+
.arg(
81+
Arg::new("all_image_file_names")
82+
.long("all-image-file-names")
83+
.value_name("NAMES")
84+
.conflicts_with("all_image_file_names_file")
85+
.help(
86+
"Comma-separated list of all image names (including subdirectory paths) \
87+
in the full test suite. \
88+
Used with selective uploads to detect image removals and renames. \
89+
Implicitly enables --selective.",
90+
),
91+
)
92+
.arg(
93+
Arg::new("all_image_file_names_file")
94+
.long("all-image-file-names-file")
95+
.value_name("PATH")
96+
.conflicts_with("all_image_file_names")
97+
.help(
98+
"Path to a file containing all image names (including subdirectory paths), \
99+
one per line. \
100+
Used with selective uploads to detect image removals and renames. \
101+
Implicitly enables --selective.",
102+
),
103+
)
80104
.git_metadata_args()
81105
}
82106

@@ -146,6 +170,26 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
146170

147171
validate_image_sizes(&images)?;
148172

173+
let all_image_file_names = parse_all_image_file_names(matches)?;
174+
175+
let selective = matches.get_flag("selective") || all_image_file_names.is_some();
176+
177+
if let Some(ref all_names) = all_image_file_names {
178+
let all_names_set: HashSet<&str> = all_names.iter().map(|s| s.as_str()).collect();
179+
let mut unknown: Vec<String> = images
180+
.iter()
181+
.map(|img| crate::utils::fs::path_as_url(&img.relative_path))
182+
.filter(|k| !all_names_set.contains(k.as_str()))
183+
.collect();
184+
if !unknown.is_empty() {
185+
unknown.sort();
186+
anyhow::bail!(
187+
"The following uploaded images are not in --all-image-file-names: {}",
188+
unknown.join(", ")
189+
);
190+
}
191+
}
192+
149193
println!(
150194
"{} Processing {} image {}",
151195
style(">").dim(),
@@ -158,13 +202,12 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
158202
// Build manifest from discovered images
159203
let diff_threshold = matches.get_one::<f64>("diff_threshold").copied();
160204

161-
let selective = matches.get_flag("selective");
162-
163205
let manifest = SnapshotsManifest {
164206
app_id: app_id.clone(),
165207
images: manifest_entries,
166208
diff_threshold,
167209
selective,
210+
all_image_file_names,
168211
vcs_info,
169212
};
170213

@@ -199,6 +242,45 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
199242
Ok(())
200243
}
201244

245+
fn split_and_trim(input: &str, separator: char) -> Vec<String> {
246+
input
247+
.split(separator)
248+
.map(|s| s.trim().to_owned())
249+
.filter(|s| !s.is_empty())
250+
.collect()
251+
}
252+
253+
fn normalize_image_names(names: Vec<String>) -> Vec<String> {
254+
names
255+
.into_iter()
256+
.map(|s| s.strip_prefix("./").unwrap_or(&s).replace('\\', "/"))
257+
.collect()
258+
}
259+
260+
fn parse_all_image_file_names(matches: &ArgMatches) -> Result<Option<Vec<String>>> {
261+
if let Some(names_str) = matches.get_one::<String>("all_image_file_names") {
262+
let names = normalize_image_names(split_and_trim(names_str, ','));
263+
if names.is_empty() {
264+
anyhow::bail!("--all-image-file-names must not be empty");
265+
}
266+
return Ok(Some(names));
267+
}
268+
269+
if let Some(file_path) = matches.get_one::<String>("all_image_file_names_file") {
270+
let content = std::fs::read_to_string(file_path)
271+
.with_context(|| format!("Failed to read --all-image-file-names-file: {file_path}"))?;
272+
let names = normalize_image_names(split_and_trim(&content, '\n'));
273+
if names.is_empty() {
274+
anyhow::bail!(
275+
"--all-image-file-names-file is empty or contains only blank lines: {file_path}"
276+
);
277+
}
278+
return Ok(Some(names));
279+
}
280+
281+
Ok(None)
282+
}
283+
202284
fn collect_images(dir: &Path) -> Vec<ImageInfo> {
203285
WalkDir::new(dir)
204286
.follow_links(true)
@@ -505,4 +587,41 @@ mod tests {
505587
let err = validate_image_sizes(&[make_image(8001, 5000)]).unwrap_err();
506588
assert!(err.to_string().contains("exceed the maximum pixel limit"));
507589
}
590+
591+
#[test]
592+
fn test_split_and_trim_comma_separated() {
593+
assert_eq!(
594+
split_and_trim("a.png, b.png, c.png", ','),
595+
vec!["a.png", "b.png", "c.png"]
596+
);
597+
}
598+
599+
#[test]
600+
fn test_split_and_trim_whitespace_and_empty() {
601+
assert_eq!(
602+
split_and_trim(" a.png , b.png , ", ','),
603+
vec!["a.png", "b.png"]
604+
);
605+
}
606+
607+
#[test]
608+
fn test_split_and_trim_newline_separated() {
609+
assert_eq!(
610+
split_and_trim("a.png\nb.png\n\nc.png\n", '\n'),
611+
vec!["a.png", "b.png", "c.png"]
612+
);
613+
}
614+
615+
#[test]
616+
fn test_normalize_image_names_strips_dot_slash() {
617+
let input = vec![
618+
"./img/a.png".to_owned(),
619+
"./img/b.png".to_owned(),
620+
"img/c.png".to_owned(),
621+
];
622+
assert_eq!(
623+
normalize_image_names(input),
624+
vec!["img/a.png", "img/b.png", "img/c.png"]
625+
);
626+
}
508627
}

tests/integration/_cases/snapshots/snapshots-upload-help.trycmd

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,16 @@ Options:
4646
Indicates this upload contains only a subset of images. Removals and renames cannot be
4747
detected on PRs.
4848

49+
--all-image-file-names <NAMES>
50+
Comma-separated list of all image names (including subdirectory paths) in the full test
51+
suite. Used with selective uploads to detect image removals and renames. Implicitly
52+
enables --selective.
53+
54+
--all-image-file-names-file <PATH>
55+
Path to a file containing all image names (including subdirectory paths), one per line.
56+
Used with selective uploads to detect image removals and renames. Implicitly enables
57+
--selective.
58+
4959
--head-sha <head_sha>
5060
The VCS commit sha to use for the upload. If not provided, the current commit sha will be
5161
used.

0 commit comments

Comments
 (0)