[variant] Add missing variant in ID generation

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(""),
                     ));
                 }
             }

context/src/junit/bindings.rs

@@ -1306,6 +1306,7 @@ mod tests {
                 name: "".into(),
             },
             &[],
+            "",
         );
         assert_eq!(test_case_runs.len(), 2);
 

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<TestCaseRun> {
         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#"
+        <xml version="1.0" encoding="UTF-8"?>
+        <testsuites>
+            <testsuite name="testsuite" timestamp="2023-10-01T12:00:00Z" time="0.002">
+                <testcase file="test.java" classname="TestClass" name="test_with_variant" time="0.001">
+                </testcase>
+            </testsuite>
+        </testsuites>
+        "#;
+        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"
+        );
+    }
 }

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<u8> = 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<u8> = 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<BindingsTestCase> = 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<BindingsTestCase> = 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
+        );
+    }
+}