diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cb5cede05..d00927de45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- (snapshots) Create snapshot builds for selective uploads with no affected images when a complete filename manifest is provided ([#3395](https://github.com/getsentry/sentry-cli/pull/3395)) - (logs) Correct the severity query example ([#3387](https://github.com/getsentry/sentry-cli/pull/3387)) ## 3.6.2 diff --git a/src/commands/snapshots/upload.rs b/src/commands/snapshots/upload.rs index 45681a8042..d9d9461909 100644 --- a/src/commands/snapshots/upload.rs +++ b/src/commands/snapshots/upload.rs @@ -147,10 +147,22 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { debug!("Organization: {org}"); debug!("Project: {project}"); + let all_image_file_names = parse_all_image_file_names(matches)?; + let selective = matches.get_flag("selective") || all_image_file_names.is_some(); + // Collect image files and read their dimensions let images = collect_images(dir_path); - if images.is_empty() { - println!("{} No image files found", style("!").yellow()); + // A complete image name list makes an empty upload a valid selective run: + // the server can reconstruct unaffected base images as skipped. + if images.is_empty() && all_image_file_names.is_none() { + let message = if selective { + "No image files found; no snapshot was created. Pass \ + --all-image-file-names or --all-image-file-names-file to record an empty \ + selective build." + } else { + "No image files found" + }; + println!("{} {message}", style("!").yellow()); return Ok(()); } @@ -163,10 +175,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { validate_image_sizes(&images)?; - let all_image_file_names = parse_all_image_file_names(matches)?; - - let selective = matches.get_flag("selective") || all_image_file_names.is_some(); - if let Some(ref all_names) = all_image_file_names { let all_names_set: HashSet<&str> = all_names.iter().map(|s| s.as_str()).collect(); let mut unknown: Vec = images @@ -190,7 +198,11 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { if images.len() == 1 { "file" } else { "files" } ); - let manifest_entries = upload_images(images, &org, &project)?; + let manifest_entries = if images.is_empty() { + HashMap::new() + } else { + upload_images(images, &org, &project)? + }; // Build manifest from discovered images let diff_threshold = matches.get_one::("diff_threshold").copied(); diff --git a/tests/integration/_cases/snapshots/snapshots-upload-empty-selective-without-names.trycmd b/tests/integration/_cases/snapshots/snapshots-upload-empty-selective-without-names.trycmd new file mode 100644 index 0000000000..42ad85811e --- /dev/null +++ b/tests/integration/_cases/snapshots/snapshots-upload-empty-selective-without-names.trycmd @@ -0,0 +1,5 @@ +``` +$ sentry-cli snapshots upload tests/integration/_fixtures/empty_snapshots --app-id test-app --selective --no-git-metadata +! No image files found; no snapshot was created. Pass --all-image-file-names or --all-image-file-names-file to record an empty selective build. + +``` diff --git a/tests/integration/_fixtures/empty_snapshots/README.txt b/tests/integration/_fixtures/empty_snapshots/README.txt new file mode 100644 index 0000000000..916c90a1f9 --- /dev/null +++ b/tests/integration/_fixtures/empty_snapshots/README.txt @@ -0,0 +1 @@ +This fixture intentionally contains no snapshot images. diff --git a/tests/integration/snapshots.rs b/tests/integration/snapshots.rs index a432a0c843..ed3300730c 100644 --- a/tests/integration/snapshots.rs +++ b/tests/integration/snapshots.rs @@ -2,6 +2,8 @@ use std::io::{Cursor, Write as _}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; +use serde_json::json; + use crate::integration::{AssertCommand, MockEndpointBuilder, TestManager}; fn snapshot_zip_bytes() -> Vec { @@ -128,3 +130,120 @@ fn command_snapshots_upload_renamed_project() { .register_trycmd_test("snapshots/snapshots-upload-renamed-project.trycmd") .with_default_token(); } + +#[test] +fn command_snapshots_upload_empty_selective_with_inline_names() { + let snapshots = tempfile::tempdir().unwrap(); + + TestManager::new() + .mock_endpoint( + MockEndpointBuilder::new( + "POST", + "/api/0/projects/wat-org/wat-project/preprodartifacts/snapshots/", + ) + .expect(1) + .with_response_fn(|request| { + let compressed = request.body().expect("body should be readable"); + let body = zstd::decode_all(Cursor::new(compressed)) + .expect("body should be valid zstd data"); + let manifest: serde_json::Value = + serde_json::from_slice(&body).expect("body should be valid JSON"); + + assert_eq!(manifest["app_id"], "test-app"); + assert_eq!(manifest["images"], json!({})); + assert_eq!(manifest["selective"], true); + assert_eq!( + manifest["all_image_file_names"], + json!(["a.png", "sub/b.jpg"]) + ); + + br#"{"artifactId":"snapshot-id","imageCount":0,"snapshotUrl":null}"#.to_vec() + }), + ) + .assert_cmd(vec![ + "snapshots", + "upload", + snapshots.path().to_str().unwrap(), + "--app-id", + "test-app", + "--all-image-file-names", + "./a.png,sub\\b.jpg", + "--no-git-metadata", + ]) + .with_default_token() + .run_and_assert(AssertCommand::Success); +} + +#[test] +fn command_snapshots_upload_empty_selective_with_names_file() { + let root = tempfile::tempdir().unwrap(); + let snapshots = root.path().join("snapshots"); + let names_file = root.path().join("all-images.txt"); + std::fs::create_dir(&snapshots).unwrap(); + std::fs::write(&names_file, "a.png\nsub/b.png\n").unwrap(); + + TestManager::new() + .mock_endpoint( + MockEndpointBuilder::new( + "POST", + "/api/0/projects/wat-org/wat-project/preprodartifacts/snapshots/", + ) + .expect(1) + .with_response_fn(|request| { + let compressed = request.body().expect("body should be readable"); + let body = zstd::decode_all(Cursor::new(compressed)) + .expect("body should be valid zstd data"); + let manifest: serde_json::Value = + serde_json::from_slice(&body).expect("body should be valid JSON"); + + assert_eq!(manifest["images"], json!({})); + assert_eq!(manifest["selective"], true); + assert_eq!( + manifest["all_image_file_names"], + json!(["a.png", "sub/b.png"]) + ); + + br#"{"artifactId":"snapshot-id","imageCount":0,"snapshotUrl":null}"#.to_vec() + }), + ) + .assert_cmd(vec![ + "snapshots".to_owned(), + "upload".to_owned(), + snapshots.to_string_lossy().into_owned(), + "--app-id".to_owned(), + "test-app".to_owned(), + "--all-image-file-names-file".to_owned(), + names_file.to_string_lossy().into_owned(), + "--no-git-metadata".to_owned(), + ]) + .with_default_token() + .run_and_assert(AssertCommand::Success); +} + +#[test] +fn command_snapshots_upload_empty_selective_without_names_warns() { + TestManager::new() + .register_trycmd_test("snapshots/snapshots-upload-empty-selective-without-names.trycmd"); +} + +#[test] +fn command_snapshots_upload_empty_names_file_fails() { + let root = tempfile::tempdir().unwrap(); + let snapshots = root.path().join("snapshots"); + let names_file = root.path().join("all-images.txt"); + std::fs::create_dir(&snapshots).unwrap(); + std::fs::write(&names_file, " \n\n").unwrap(); + + TestManager::new() + .assert_cmd(vec![ + "snapshots".to_owned(), + "upload".to_owned(), + snapshots.to_string_lossy().into_owned(), + "--app-id".to_owned(), + "test-app".to_owned(), + "--all-image-file-names-file".to_owned(), + names_file.to_string_lossy().into_owned(), + "--no-git-metadata".to_owned(), + ]) + .run_and_assert(AssertCommand::Failure); +}