diff --git a/cli/src/context.rs b/cli/src/context.rs index 2c36cd77..369c0ad1 100644 --- a/cli/src/context.rs +++ b/cli/src/context.rs @@ -375,6 +375,7 @@ pub fn generate_internal_file_from_bep( org_slug, repo, quarantined_test_ids, + variant.as_deref().unwrap_or(""), ); for test_case_run in &mut xml_test_case_runs { test_case_run.attempt_number = xml_file.attempt; @@ -403,6 +404,7 @@ pub fn generate_internal_file_from_bep( org_slug, repo, quarantined_test_ids, + variant.as_deref().unwrap_or(""), )); } } @@ -474,6 +476,7 @@ pub fn generate_internal_file( org_slug, repo, quarantined_test_ids, + variant.as_deref().unwrap_or(""), )); } } diff --git a/context/src/junit/bindings.rs b/context/src/junit/bindings.rs index 26572425..8448fa5f 100644 --- a/context/src/junit/bindings.rs +++ b/context/src/junit/bindings.rs @@ -1306,6 +1306,7 @@ mod tests { name: "".into(), }, &[], + "", ); assert_eq!(test_case_runs.len(), 2); diff --git a/context/src/junit/parser.rs b/context/src/junit/parser.rs index b48863e5..d5153210 100644 --- a/context/src/junit/parser.rs +++ b/context/src/junit/parser.rs @@ -25,7 +25,7 @@ use wasm_bindgen::prelude::*; use super::date_parser::JunitDateParser; #[cfg(feature = "bindings")] use crate::junit::bindings::BindingsReport; -use crate::{meta::id::gen_info_id, repo::RepoUrlParts}; +use crate::{meta::id::generate_info_id_variant_wrapper, repo::RepoUrlParts}; const TAG_REPORT: &[u8] = b"testsuites"; const TAG_TEST_SUITE: &[u8] = b"testsuite"; @@ -203,6 +203,7 @@ impl JunitParser { org_slug: &T, repo: &RepoUrlParts, quarantined_test_ids: &[String], + variant: &str, ) -> Vec { let mut test_case_runs = Vec::new(); for report in self.reports { @@ -317,22 +318,23 @@ impl JunitParser { } } - let test_case_id = test_case - .extra - .get(extra_attrs::ID) - .map(|v| v.to_string()) - .unwrap_or_else(|| { - gen_info_id( - org_slug.as_ref(), - repo.repo_full_name().as_str(), - Some(file.as_str()), - test_case.classname.map(|v| v.to_string()).as_deref(), - Some(test_case_run.parent_name.as_str()), - Some(test_case_run.name.as_str()), - None, - "", - ) - }); + let existing_id = test_case.extra.get(extra_attrs::ID).map(|v| v.as_str()); + + // trunk-ignore(clippy/unnecessary_unwrap) + let test_case_id = if existing_id.is_some() && variant.is_empty() { + existing_id.unwrap().to_string() + } else { + generate_info_id_variant_wrapper( + org_slug.as_ref(), + repo.repo_full_name().as_str(), + Some(file.as_str()), + test_case.classname.map(|v| v.to_string()).as_deref(), + Some(test_case_run.parent_name.as_str()), + Some(test_case_run.name.as_str()), + existing_id, + variant, + ) + }; test_case_run.is_quarantined = quarantined_test_ids.contains(&test_case_id); test_case_run.id = test_case_id; @@ -993,7 +995,11 @@ mod tests { use prost_wkt_types::Timestamp; use proto::test_context::test_run::TestCaseRunStatus; - use crate::{junit::parser::JunitParser, meta::id::gen_info_id, repo::RepoUrlParts}; + use crate::{ + junit::parser::JunitParser, + meta::id::{gen_info_id, generate_info_id_variant_wrapper}, + repo::RepoUrlParts, + }; #[test] fn test_into_test_case_runs() { let mut junit_parser = JunitParser::new(); @@ -1036,6 +1042,7 @@ mod tests { None, "", )], + "", ); assert_eq!(test_case_runs.len(), 2); let test_case_run1 = &test_case_runs[0]; @@ -1128,7 +1135,7 @@ mod tests { name: "repo-name".into(), }; - let test_case_runs = junit_parser.into_test_case_runs(None, &org_slug, &repo, &[]); + let test_case_runs = junit_parser.into_test_case_runs(None, &org_slug, &repo, &[], ""); assert_eq!(test_case_runs.len(), 1); let test_case_run = &test_case_runs[0]; @@ -1164,7 +1171,7 @@ mod tests { name: "repo-name".into(), }; - let test_case_runs = junit_parser.into_test_case_runs(None, &org_slug, &repo, &[]); + let test_case_runs = junit_parser.into_test_case_runs(None, &org_slug, &repo, &[], ""); assert_eq!(test_case_runs.len(), 2); // First test case should have the custom ID @@ -1232,6 +1239,7 @@ mod tests { None, "", )], + "", ); assert_eq!(test_case_runs.len(), 2); let test_case_run1 = &test_case_runs[0]; @@ -1413,6 +1421,7 @@ mod tests { name: "test-repo".into(), }, &[], + "", ); assert_eq!(test_case_runs.len(), 1); @@ -1448,4 +1457,83 @@ mod tests { time ); } + + #[test] + fn test_into_test_case_runs_with_variant() { + let mut junit_parser = JunitParser::new(); + let file_contents = r#" + + + + + + + + "#; + let parsed_results = junit_parser.parse(BufReader::new(file_contents.as_bytes())); + assert!(parsed_results.is_ok()); + + let org_slug = "org-url-slug".to_string(); + let repo = RepoUrlParts { + host: "repo-host".into(), + owner: "repo-owner".into(), + name: "repo-name".into(), + }; + let variant = "test-variant"; + + let test_case_runs_with_variant = + junit_parser + .clone() + .into_test_case_runs(None, &org_slug, &repo, &[], variant); + + assert_eq!(test_case_runs_with_variant.len(), 1); + let test_case_with_variant = &test_case_runs_with_variant[0]; + + let expected_id_with_variant = generate_info_id_variant_wrapper( + org_slug.as_str(), + repo.repo_full_name().as_str(), + Some("test.java"), + Some("TestClass"), + Some("testsuite"), + Some("test_with_variant"), + None, + variant, + ); + + assert_eq!( + test_case_with_variant.id, expected_id_with_variant, + "ID should be generated with variant included" + ); + + let mut junit_parser_no_variant = JunitParser::new(); + junit_parser_no_variant + .parse(BufReader::new(file_contents.as_bytes())) + .unwrap(); + let test_case_runs_no_variant = + junit_parser_no_variant.into_test_case_runs(None, &org_slug, &repo, &[], ""); + + assert_eq!(test_case_runs_no_variant.len(), 1); + let test_case_no_variant = &test_case_runs_no_variant[0]; + + let expected_id_no_variant = gen_info_id( + org_slug.as_str(), + repo.repo_full_name().as_str(), + Some("test.java"), + Some("TestClass"), + Some("testsuite"), + Some("test_with_variant"), + None, + "", + ); + + assert_eq!( + test_case_no_variant.id, expected_id_no_variant, + "ID should be generated without variant" + ); + + assert_ne!( + test_case_with_variant.id, test_case_no_variant.id, + "IDs with and without variant should be different" + ); + } } diff --git a/xcresult/tests/data/test-variant.xcresult.tar.gz b/xcresult/tests/data/test-variant.xcresult.tar.gz new file mode 100644 index 00000000..ce60ce77 Binary files /dev/null and b/xcresult/tests/data/test-variant.xcresult.tar.gz differ diff --git a/xcresult/tests/xcresult.rs b/xcresult/tests/xcresult.rs index 8917ad01..ad600362 100644 --- a/xcresult/tests/xcresult.rs +++ b/xcresult/tests/xcresult.rs @@ -33,6 +33,8 @@ lazy_static! { unpack_archive_to_temp_dir("tests/data/test-swift-mix.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 = + unpack_archive_to_temp_dir("tests/data/test-variant.xcresult.tar.gz"); static ref ORG_URL_SLUG: String = String::from("trunk"); static ref REPO_FULL_NAME: String = RepoUrlParts { host: "github.com".to_string(), @@ -246,7 +248,6 @@ fn test_xcresult_to_bindings_report_with_id_and_timestamps() { use context::junit::bindings::BindingsTestCase; use context::junit::parser::JunitParser; - // Generate JUnit from xcresult let path = TEMP_DIR_TEST_TIMESTAMP.as_ref().join("test1.xcresult"); let path_str = path.to_str().unwrap(); @@ -262,12 +263,10 @@ fn test_xcresult_to_bindings_report_with_id_and_timestamps() { assert_eq!(junits.len(), 1); let junit = junits.pop().unwrap(); - // Serialize to XML let mut junit_writer: Vec = Vec::new(); junit.serialize(&mut junit_writer).unwrap(); let junit_xml = String::from_utf8(junit_writer).unwrap(); - // Parse the JUnit XML back let mut junit_parser = JunitParser::new(); junit_parser .parse(BufReader::new(junit_xml.as_bytes())) @@ -283,6 +282,7 @@ fn test_xcresult_to_bindings_report_with_id_and_timestamps() { name: "analytics-cli".to_string(), }, &[], + "", ) .into_iter() .map(BindingsTestCase::from) @@ -327,3 +327,107 @@ fn test_xcresult_to_bindings_report_with_id_and_timestamps() { assert!(time >= 0.0, "time should be non-negative, got: {}", time); } } + +#[cfg(target_os = "macos")] +#[test] +fn test_xcresult_with_variant_id_generation() { + use std::io::BufReader; + + use context::junit::bindings::BindingsTestCase; + use context::junit::parser::JunitParser; + + // Generate JUnit from xcresult + let path = TEMP_DIR_TEST_VARIANT.as_ref().join("test1.xcresult"); + let path_str = path.to_str().unwrap(); + + let xcresult = XCResult::new( + path_str, + ORG_URL_SLUG.clone(), + REPO_FULL_NAME.clone(), + false, + ) + .unwrap(); + + let mut junits = xcresult.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(); + let junit_xml = String::from_utf8(junit_writer).unwrap(); + + let repo_parts = context::repo::RepoUrlParts { + host: "github.com".to_string(), + owner: "trunk-io".to_string(), + name: "analytics-cli".to_string(), + }; + + // Parse WITHOUT variant + let mut junit_parser_no_variant = JunitParser::new(); + junit_parser_no_variant + .parse(BufReader::new(junit_xml.as_bytes())) + .expect("Failed to parse generated JUnit XML"); + + let test_case_runs_no_variant: Vec = junit_parser_no_variant + .into_test_case_runs(None, &ORG_URL_SLUG.as_str(), &repo_parts, &[], "") + .into_iter() + .map(BindingsTestCase::from) + .collect(); + + // Parse WITH variant + let variant = "ios-simulator"; + let mut junit_parser_with_variant = JunitParser::new(); + junit_parser_with_variant + .parse(BufReader::new(junit_xml.as_bytes())) + .expect("Failed to parse generated JUnit XML"); + + let test_case_runs_with_variant: Vec = junit_parser_with_variant + .into_test_case_runs(None, &ORG_URL_SLUG.as_str(), &repo_parts, &[], variant) + .into_iter() + .map(BindingsTestCase::from) + .collect(); + + assert!( + !test_case_runs_no_variant.is_empty(), + "Should have test cases without variant" + ); + assert!( + !test_case_runs_with_variant.is_empty(), + "Should have test cases with variant" + ); + assert_eq!( + test_case_runs_no_variant.len(), + test_case_runs_with_variant.len(), + "Should have same number of test cases" + ); + + for (test_no_variant, test_with_variant) in test_case_runs_no_variant + .iter() + .zip(test_case_runs_with_variant.iter()) + { + let extra_no_variant = test_no_variant.extra(); + let id_no_variant = extra_no_variant + .get("id") + .expect("ID should be set for test without variant"); + + let extra_with_variant = test_with_variant.extra(); + let id_with_variant = extra_with_variant + .get("id") + .expect("ID should be set for test with variant"); + + assert!( + !id_no_variant.is_empty(), + "ID without variant should not be empty" + ); + assert!( + !id_with_variant.is_empty(), + "ID with variant should not be empty" + ); + + assert_ne!( + id_no_variant, id_with_variant, + "ID should be different when variant '{}' is applied. Test: {}", + variant, test_no_variant.name + ); + } +}