diff --git a/Cargo.lock b/Cargo.lock index bd1deb94..2a121009 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1568,6 +1568,12 @@ version = "0.3.31" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f90f7dce0722e95104fcb095585910c0977252f286e354b5e3bd38902cd99988" +[[package]] +name = "futures-timer" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f288b0a4f20f9a56b5d1da57e2227c661b7b16168e2f72365f57b63326e29b24" + [[package]] name = "futures-util" version = "0.3.31" @@ -1708,7 +1714,7 @@ dependencies = [ "gix-utils", "itoa", "thiserror 2.0.17", - "winnow", + "winnow 0.7.13", ] [[package]] @@ -1763,7 +1769,7 @@ dependencies = [ "smallvec", "thiserror 2.0.17", "unicode-bom", - "winnow", + "winnow 0.7.13", ] [[package]] @@ -1916,7 +1922,7 @@ dependencies = [ "itoa", "smallvec", "thiserror 2.0.17", - "winnow", + "winnow 0.7.13", ] [[package]] @@ -1999,7 +2005,7 @@ dependencies = [ "gix-utils", "maybe-async", "thiserror 2.0.17", - "winnow", + "winnow 0.7.13", ] [[package]] @@ -2031,7 +2037,7 @@ dependencies = [ "gix-validate", "memmap2", "thiserror 2.0.17", - "winnow", + "winnow 0.7.13", ] [[package]] @@ -4028,6 +4034,15 @@ dependencies = [ "syn 2.0.110", ] +[[package]] +name = "proc-macro-crate" +version = "3.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e67ba7e9b2b56446f1d419b1d807906278ffa1a658a8a5d8a39dcb1f5a78614f" +dependencies = [ + "toml_edit 0.25.6+spec-1.1.0", +] + [[package]] name = "proc-macro2" version = "1.0.103" @@ -4585,6 +4600,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "relative-path" +version = "1.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba39f3699c378cd8970968dcbff9c43159ea4cfbd88d43c00b22f2ef10a435d2" + [[package]] name = "reqwest" version = "0.12.24" @@ -4644,6 +4665,35 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rstest" +version = "0.26.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f5a3193c063baaa2a95a33f03035c8a72b83d97a54916055ba22d35ed3839d49" +dependencies = [ + "futures-timer", + "futures-util", + "rstest_macros", +] + +[[package]] +name = "rstest_macros" +version = "0.26.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9c845311f0ff7951c5506121a9ad75aec44d083c31583b2ea5a30bcb0b0abba0" +dependencies = [ + "cfg-if", + "glob", + "proc-macro-crate", + "proc-macro2", + "quote", + "regex", + "relative-path", + "rustc_version", + "syn 2.0.110", + "unicode-ident", +] + [[package]] name = "rustc-demangle" version = "0.1.26" @@ -5831,8 +5881,8 @@ checksum = "dc1beb996b9d83529a9e75c17a1686767d148d70663143c7854d8b4a09ced362" dependencies = [ "serde", "serde_spanned", - "toml_datetime", - "toml_edit", + "toml_datetime 0.6.11", + "toml_edit 0.22.27", ] [[package]] @@ -5844,6 +5894,15 @@ dependencies = [ "serde", ] +[[package]] +name = "toml_datetime" +version = "1.1.1+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3165f65f62e28e0115a00b2ebdd37eb6f3b641855f9d636d3cd4103767159ad7" +dependencies = [ + "serde_core", +] + [[package]] name = "toml_edit" version = "0.22.27" @@ -5853,9 +5912,30 @@ dependencies = [ "indexmap 2.12.0", "serde", "serde_spanned", - "toml_datetime", + "toml_datetime 0.6.11", "toml_write", - "winnow", + "winnow 0.7.13", +] + +[[package]] +name = "toml_edit" +version = "0.25.6+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0db3bae107c9522f86d361697dee1d7386a2ddcf659d5aea5159819a21a3c4a7" +dependencies = [ + "indexmap 2.12.0", + "toml_datetime 1.1.1+spec-1.1.0", + "toml_parser", + "winnow 1.0.3", +] + +[[package]] +name = "toml_parser" +version = "1.1.2+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a2abe9b86193656635d2411dc43050282ca48aa31c2451210f4202550afb7526" +dependencies = [ + "winnow 1.0.3", ] [[package]] @@ -6911,6 +6991,15 @@ dependencies = [ "memchr", ] +[[package]] +name = "winnow" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0592e1c9d151f854e6fd382574c3a0855250e1d9b2f99d9281c6e6391af352f1" +dependencies = [ + "memchr", +] + [[package]] name = "wit-bindgen" version = "0.46.0" @@ -6949,6 +7038,7 @@ dependencies = [ "proc-macro2", "quick-junit", "regex", + "rstest", "schemars", "serde", "serde_json", diff --git a/xcresult/Cargo.toml b/xcresult/Cargo.toml index 75163bc8..2cf90e27 100644 --- a/xcresult/Cargo.toml +++ b/xcresult/Cargo.toml @@ -30,6 +30,7 @@ uuid = { version = "1.10.0", features = ["v5"] } context = { path = "../context", features = ["bindings"] } flate2 = "1.0.34" pretty_assertions = "0.6" +rstest = "0.26.1" tar = "0.4.45" temp_testdir = "0.2.3" diff --git a/xcresult/src/xcresult_legacy.rs b/xcresult/src/xcresult_legacy.rs index 59dafadd..7fdcea4b 100644 --- a/xcresult/src/xcresult_legacy.rs +++ b/xcresult/src/xcresult_legacy.rs @@ -34,44 +34,112 @@ impl XCResultTestLegacy { // grab the first failure summary if there are multiple failure_summaries.values.first() }) - .and_then(|failure_summary| { + .and_then(Self::find_file_in_failure_summary) + }) + } + + fn find_file_in_failure_summary( + failure_summary: &legacy_schema::ActionTestFailureSummary, + ) -> Option { + Self::normalize_file_path(failure_summary.file_name.as_ref().map(|file| &file.value)) + .or_else(|| { + Self::normalize_file_path( failure_summary .source_code_context .as_ref() - .and_then(|source_code_context| { - source_code_context - .call_stack - .as_ref() - .and_then(|call_stack| { - call_stack - .values - .iter() - .filter_map(|call_stack| { - call_stack - .symbol_info - .as_ref() - .and_then(|symbol_info| { - symbol_info.location.as_ref().and_then( - |location| location.file_path.as_ref(), - ) - }) - .map(|file_path| { - file_path.value.replace(' ', "%20") - }) - }) - .filter(|file_path| { - std::path::Path::new(&file_path) - .extension() - .map(|ext| ext == "swift" || ext == "m") - .unwrap_or(false) - }) - // use the last valid swift / obj-c file-path in the stack - .last() - }) - }) - }) - }) + .and_then(|source_code_context| source_code_context.location.as_ref()) + .and_then(|location| location.file_path.as_ref()) + .map(|file_path| &file_path.value), + ) + }) + .or_else(|| { + failure_summary + .source_code_context + .as_ref() + .and_then(Self::find_file_in_source_code_context_call_stack) + }) + } + + fn find_file_in_source_code_context_call_stack( + source_code_context: &legacy_schema::SourceCodeContext, + ) -> Option { + source_code_context + .call_stack + .as_ref() + .and_then(|call_stack| { + call_stack + .values + .iter() + .filter_map(|call_stack| { + call_stack + .symbol_info + .as_ref() + .and_then(|symbol_info| { + symbol_info + .location + .as_ref() + .and_then(|location| location.file_path.as_ref()) + }) + .and_then(|file_path| Self::normalize_file_path(Some(&file_path.value))) + }) + .filter(|file_path| { + std::path::Path::new(&file_path) + .extension() + .map(|ext| ext == "swift" || ext == "m") + .unwrap_or(false) + }) + // use the last valid swift / obj-c file-path in the stack + .last() + }) + } + + fn normalize_file_path(file_path: Option<&String>) -> Option { + file_path.map(|file_path| file_path.replace(' ', "%20")) + } + + fn fallback_file_from_failure_issue_summary( + failure_summary: &legacy_schema::TestFailureIssueSummary, + ) -> Option<(Option<&str>, String)> { + failure_summary + .document_location_in_creating_workspace + .as_ref() + .and_then(|document_location_in_creating_workspace| { + document_location_in_creating_workspace.url.as_ref() + }) + .map(|file| { + let file = file + .value + .replace("file://", "") + .split('#') + .next() + .unwrap_or_default() + .into(); + let producing_target = failure_summary + .producing_target + .as_ref() + .map(|x| x.value.as_ref()); + if producing_target.is_some() { + return (producing_target, file); + } + let test_case_name = failure_summary + .test_case_name + .as_ref() + .map(|x| x.value.as_ref()); + (test_case_name, file) + }) + } + + fn find_fallback_file<'a>( + files: &HashMap, String>, + test_suite_name: Option<&str>, + formatted_test_case_name: &str, + ) -> Option { + files + .get(&test_suite_name) + .or_else(|| files.get(&Some(formatted_test_case_name))) + .cloned() } + pub fn generate_from_object>( path: T, use_experimental_failure_summary: bool, @@ -234,34 +302,7 @@ impl XCResultTestLegacy { failure_summaries .values .iter() - .flat_map(|failure_summary| { - failure_summary - .document_location_in_creating_workspace - .as_ref() - .and_then(|document_location_in_creating_workspace| { - document_location_in_creating_workspace.url.as_ref() - }) - .map(|file| { - let mut file = file.value.clone(); - file = file - .replace("file://", "") - .split('#') - .collect::>()[0] - .into(); - let producing_target = failure_summary - .producing_target - .as_ref() - .map(|x| x.value.as_ref()); - if producing_target.is_some() { - return (producing_target, file); - } - let test_case_name = failure_summary - .test_case_name - .as_ref() - .map(|x| x.value.as_ref()); - (test_case_name, file) - }) - }) + .flat_map(Self::fallback_file_from_failure_issue_summary) .collect::>() }); leafs @@ -308,10 +349,11 @@ impl XCResultTestLegacy { }; if file.is_none() { file = files.as_ref().and_then(|files| { - files - .get(&test_suite_name) - .or_else(|| files.get(&Some(&formatted_test_case_name))) - .cloned() + Self::find_fallback_file( + files, + test_suite_name, + &formatted_test_case_name, + ) }) } @@ -502,3 +544,139 @@ impl<'a> XCResultTestLegacyNodeTree<'a> { } } } + +#[cfg(test)] +mod tests { + use rstest::rstest; + use serde_json::{Value, json}; + + use super::*; + + fn xc_string(value: &str) -> Value { + json!({ "_value": value }) + } + + #[rstest] + #[case::file_name_wins( + Some("/repo/Tests/My Test.swift"), + Some("/repo/Tests/Other.swift"), + &[], + Some("/repo/Tests/My%20Test.swift") + )] + #[case::location_before_call_stack( + None, + Some("/repo/Tests/Assertion.swift"), + &["/repo/Packages/SnapshotTesting/SnapshotsTestTrait.swift"], + Some("/repo/Tests/Assertion.swift") + )] + #[case::last_swift_or_objc_stack_frame( + None, + None, + &[ + "/repo/Tests/Generated.cc", + "/repo/Tests/First.swift", + "/repo/Tests/Second.m", + "/repo/Tests/Readme.md", + ], + Some("/repo/Tests/Second.m") + )] + #[case::no_usable_file( + None, + None, + &["/repo/Tests/Generated.cc", "/repo/Tests/Readme.md"], + None + )] + fn failure_summary_file_sources( + #[case] file_name: Option<&str>, + #[case] location: Option<&str>, + #[case] stack: &[&str], + #[case] expected: Option<&str>, + ) { + let summary = serde_json::from_value(json!({ + "fileName": file_name.map(xc_string), + "sourceCodeContext": { + "location": { "filePath": location.map(xc_string) }, + "callStack": { "_values": stack.iter().map(|path| { + let stack_frame = json!({ + "symbolInfo": { + "location": { + "filePath": xc_string(path) + } + } + }); + stack_frame + }).collect::>() } + } + })) + .unwrap(); + let file = XCResultTestLegacy::find_file_in_failure_summary(&summary); + assert_eq!(file, expected.map(String::from)); + } + + #[rstest] + #[case::producing_target_key( + Some("file:///repo/Tests/Test.swift#EndingLineNumber=8"), + Some("SnapshotReproTests"), + Some("SnapshotReproTests.failingSnapshot()"), + Some((Some("SnapshotReproTests"), "/repo/Tests/Test.swift")) + )] + #[case::test_case_key( + Some("file:///repo/Tests/Test.swift"), + None, + Some("SnapshotReproTests.failingSnapshot()"), + Some((Some("SnapshotReproTests.failingSnapshot()"), "/repo/Tests/Test.swift")) + )] + #[case::missing_document_location( + None, + None, + Some("SnapshotReproTests.failingSnapshot()"), + None + )] + fn fallback_issue_summary_cleans_url_and_selects_key( + #[case] url: Option<&str>, + #[case] producing_target: Option<&str>, + #[case] test_case_name: Option<&str>, + #[case] expected: Option<(Option<&str>, &str)>, + ) { + let summary = serde_json::from_value(json!({ + "documentLocationInCreatingWorkspace": { "url": url.map(xc_string) }, + "producingTarget": producing_target.map(xc_string), + "testCaseName": test_case_name.map(xc_string) + })) + .unwrap(); + let file = XCResultTestLegacy::fallback_file_from_failure_issue_summary(&summary) + .map(|(key, file)| (key.map(String::from), file)); + assert_eq!( + file, + expected.map(|(key, file)| (key.map(String::from), file.to_string())) + ); + } + + #[rstest] + #[case::suite_key_wins(true, Some("/repo/Tests/Suite.swift"))] + #[case::formatted_case_fallback(false, Some("/repo/Tests/TestCase.swift"))] + fn fallback_file_lookup_prefers_suite_then_formatted_case( + #[case] include_suite_file: bool, + #[case] expected: Option<&str>, + ) { + let mut files = HashMap::new(); + if include_suite_file { + files.insert( + Some("SnapshotReproTests"), + "/repo/Tests/Suite.swift".to_string(), + ); + } + files.insert( + Some("SnapshotReproTests.failingSnapshot()"), + "/repo/Tests/TestCase.swift".to_string(), + ); + assert_eq!( + XCResultTestLegacy::find_fallback_file( + &files, + Some("SnapshotReproTests"), + "SnapshotReproTests.failingSnapshot()", + ), + expected.map(String::from) + ); + } +} diff --git a/xcresult/tests/data/test-swift-snapshot-testing.junit.xml b/xcresult/tests/data/test-swift-snapshot-testing.junit.xml new file mode 100644 index 00000000..a5d03dce --- /dev/null +++ b/xcresult/tests/data/test-swift-snapshot-testing.junit.xml @@ -0,0 +1,23 @@ + + + + + + + + diff --git a/xcresult/tests/data/test-swift-snapshot-testing.xcresult.tar.gz b/xcresult/tests/data/test-swift-snapshot-testing.xcresult.tar.gz new file mode 100644 index 00000000..f438ad44 Binary files /dev/null and b/xcresult/tests/data/test-swift-snapshot-testing.xcresult.tar.gz differ diff --git a/xcresult/tests/xcresult.rs b/xcresult/tests/xcresult.rs index 8260043c..8a3f7612 100644 --- a/xcresult/tests/xcresult.rs +++ b/xcresult/tests/xcresult.rs @@ -3,6 +3,7 @@ use std::{fs::File, path::Path}; use context::repo::RepoUrlParts; use flate2::read::GzDecoder; use lazy_static::lazy_static; +use rstest::rstest; use tar::Archive; use temp_testdir::TempDir; use xcresult::xcresult::XCResult; @@ -31,6 +32,8 @@ lazy_static! { unpack_archive_to_temp_dir("tests/data/test-swift-without-test-suites.xcresult.tar.gz"); static ref TEMP_DIR_TEST_SWIFT_MIX: TempDir = unpack_archive_to_temp_dir("tests/data/test-swift-mix.xcresult.tar.gz"); + static ref TEMP_DIR_TEST_SWIFT_SNAPSHOT_TESTING: TempDir = + unpack_archive_to_temp_dir("tests/data/test-swift-snapshot-testing.xcresult.tar.gz"); static ref TEMP_DIR_TEST_TIMESTAMP: TempDir = unpack_archive_to_temp_dir("tests/data/test-timestamp.xcresult.tar.gz"); static ref TEMP_DIR_TEST_VARIANT: TempDir = @@ -212,6 +215,36 @@ fn test_xcresult_with_valid_path_invalid_os() { ); } +#[cfg(target_os = "macos")] +#[rstest] +#[case::experimental_failure_summary(true)] +#[case::legacy_fallback(false)] +fn test_swift_snapshot_testing_trait_failure_uses_assertion_file( + #[case] use_experimental_failure_summary: bool, +) { + let path = TEMP_DIR_TEST_SWIFT_SNAPSHOT_TESTING + .as_ref() + .join("SnapshotRepro.xcresult"); + let path_str = path.to_str().unwrap(); + let xcresult = XCResult::new( + path_str, + ORG_URL_SLUG.clone(), + REPO_FULL_NAME.clone(), + use_experimental_failure_summary, + ); + assert!(xcresult.is_ok()); + + let mut junits = xcresult.unwrap().generate_junits(); + assert_eq!(junits.len(), 1); + let junit = junits.pop().unwrap(); + let mut junit_writer: Vec = Vec::new(); + junit.serialize(&mut junit_writer).unwrap(); + pretty_assertions::assert_eq!( + String::from_utf8(junit_writer).unwrap(), + include_str!("data/test-swift-snapshot-testing.junit.xml") + ); +} + #[cfg(target_os = "macos")] #[test] fn test_expected_failures_xcresult_with_valid_path() {