Override id field when specified in junit (#121)

cli/Cargo.toml

@@ -37,12 +37,12 @@ serde = { version = "1.0.130", default-features = false, features = ["derive"] }
 serde_json = "1.0.68"
 zstd = { version = "0.13.0", default-features = false }
 tar = { version = "0.4.30", default-features = false }
-junit-parser = "1.1.0"
 codeowners = { path = "../codeowners" }
 xcresult = { path = "../xcresult" }
 sentry = { version = "0.34.0", features = ["debug-images"] }
 openssl = { version = "0.10.66", features = ["vendored"] }
 uuid = { version = "1.10.0", features = ["v5"] }
+quick-junit = "0.5.0"
 
 [dev-dependencies]
 assert_cmd = "2.0.16"

cli/src/runner.rs

@@ -1,9 +1,10 @@
+use quick_junit::TestCaseStatus;
 use std::process::{Command, Stdio};
 use std::time::SystemTime;
 
 use api::QuarantineConfig;
+use context::junit::parser::JunitParser;
 use context::repo::BundleRepo;
-use junit_parser;
 use tokio_retry::strategy::ExponentialBackoff;
 use tokio_retry::Retry;
 
@@ -125,28 +126,36 @@ pub async fn extract_failed_tests(
                 }
             };
             let reader = std::io::BufReader::new(file);
-            let junitxml = match junit_parser::from_reader(reader) {
+            let mut junitxml = JunitParser::new();
+            match junitxml.parse(reader) {
                 Ok(junitxml) => junitxml,
                 Err(e) => {
                     log::warn!("Error parsing junitxml: {}", e);
                     continue;
                 }
             };
-            for suite in junitxml.suites {
-                let parent_name = suite.name;
-                for case in suite.cases {
-                    let failure = case.status.is_failure();
-                    if failure {
-                        let name = case.original_name;
+            for report in junitxml.reports() {
+                for suite in &report.test_suites {
+                    let parent_name = String::from(suite.name.as_str());
+                    for case in &suite.test_cases {
+                        if !matches!(case.status, TestCaseStatus::NonSuccess { .. }) {
+                            continue;
+                        }
+                        let name = String::from(case.name.as_str());
+                        let xml_string_to_string =
+                            |s: &quick_junit::XmlString| String::from(s.as_str());
+                        let classname = case.classname.as_ref().map(xml_string_to_string);
+                        let file = case.extra.get("file").map(xml_string_to_string);
+                        let id = case.extra.get("id").map(xml_string_to_string);
                         let test = Test::new(
-                            name.clone(),
+                            name,
                             parent_name.clone(),
-                            case.classname.clone(),
-                            case.file.clone(),
+                            classname,
+                            file,
+                            id,
                             org_slug,
                             repo,
                         );
-
                         failures.push(test);
                     }
                 }

cli/src/types.rs

@@ -32,9 +32,20 @@ impl Test {
         parent_name: String,
         class_name: Option<String>,
         file: Option<String>,
+        id: Option<String>,
         org_slug: &str,
         repo: &BundleRepo,
     ) -> Self {
+        if let Some(id) = id {
+            return Test {
+                parent_name,
+                name,
+                class_name,
+                file,
+                id,
+            };
+        }
+        // generate a unique id if not provided
         let repo_full_name = format!("{}/{}/{}", repo.repo.host, repo.repo.owner, repo.repo.name);
         let info_id_input = [
             org_slug,
@@ -231,6 +242,7 @@ mod tests {
             parent_name.clone(),
             class_name.clone(),
             file.clone(),
+            None,
             org_slug,
             &repo,
         );
@@ -239,5 +251,19 @@ mod tests {
         assert_eq!(result.class_name, class_name);
         assert_eq!(result.file, file);
         assert_eq!(result.id, "aad1f138-09ab-5ea9-9c21-af48a03d6edd");
+        let result = Test::new(
+            name.clone(),
+            parent_name.clone(),
+            class_name.clone(),
+            file.clone(),
+            Some(String::from("da5b8893-d6ca-5c1c-9a9c-91f40a2a3649")),
+            org_slug,
+            &repo,
+        );
+        assert_eq!(result.name, name);
+        assert_eq!(result.parent_name, parent_name);
+        assert_eq!(result.class_name, class_name);
+        assert_eq!(result.file, file);
+        assert_eq!(result.id, "da5b8893-d6ca-5c1c-9a9c-91f40a2a3649");
     }
 }

context/Cargo.toml

@@ -24,6 +24,7 @@ serde = { version = "1.0.130", default-features = false, features = ["derive"] }
 serde_json = "1.0.68"
 speedate = "0.14.4"
 thiserror = "1.0.63"
+uuid = { version = "1.10.0", features = ["v5"] }
 wasm-bindgen = { version = "0.2.84", optional = true }
 
 [features]

context/src/junit/parser.rs

@@ -27,6 +27,7 @@ pub mod extra_attrs {
     pub const FILE: &str = "file";
     pub const FILEPATH: &str = "filepath";
     pub const LINE: &str = "line";
+    pub const ID: &str = "id";
 }
 
 #[derive(Error, Debug, Copy, Clone, PartialEq, Eq)]
@@ -274,6 +275,10 @@ impl JunitParser {
                 .insert(extra_attrs::FILEPATH.into(), filepath.into());
         }
 
+        if let Some(id) = parse_attr::id(e) {
+            test_suite.extra.insert(extra_attrs::ID.into(), id.into());
+        }
+
         if let Some(line) = parse_attr::line(e) {
             test_suite
                 .extra
@@ -319,17 +324,25 @@ impl JunitParser {
         }
 
         if let Some(file) = parse_attr::file(e) {
-            test_case.extra.insert("file".into(), file.into());
+            test_case
+                .extra
+                .insert(extra_attrs::FILE.into(), file.into());
         }
 
         if let Some(filepath) = parse_attr::filepath(e) {
-            test_case.extra.insert("filepath".into(), filepath.into());
+            test_case
+                .extra
+                .insert(extra_attrs::FILEPATH.into(), filepath.into());
+        }
+
+        if let Some(id) = parse_attr::id(e) {
+            test_case.extra.insert(extra_attrs::ID.into(), id.into());
         }
 
         if let Some(line) = parse_attr::line(e) {
             test_case
                 .extra
-                .insert("line".into(), line.to_string().into());
+                .insert(extra_attrs::LINE.into(), line.to_string().into());
         }
 
         self.current_test_case = Some(test_case);
@@ -536,6 +549,10 @@ mod parse_attr {
         parse_string_attr(e, extra_attrs::FILEPATH)
     }
 
+    pub fn id<'a>(e: &'a BytesStart<'a>) -> Option<Cow<'a, str>> {
+        parse_string_attr(e, extra_attrs::ID)
+    }
+
     pub fn line<'a>(e: &'a BytesStart<'a>) -> Option<usize> {
         parse_string_attr_into_other_type(e, extra_attrs::LINE)
     }

context/src/junit/validator.rs

@@ -65,6 +65,17 @@ pub fn validate(report: &Report) -> JunitReportValidation {
             }
         };
 
+        if let Some(raw_test_suite_id) = test_suite.extra.get("id") {
+            let test_case_id = uuid::Uuid::parse_str(raw_test_suite_id).unwrap_or_default();
+            if test_case_id.get_version() != Some(uuid::Version::Sha1) {
+                test_suite_validation.add_issue(JunitValidationIssue::Invalid(
+                    JunitTestSuiteValidationIssueInvalid::TestSuiteInvalidId(
+                        raw_test_suite_id.to_string().clone(),
+                    ),
+                ));
+            }
+        }
+
         for test_case in test_suite.test_cases.iter() {
             let mut test_case_validation = JunitTestCaseValidation::default();
 
@@ -82,6 +93,17 @@ pub fn validate(report: &Report) -> JunitReportValidation {
                 }
             };
 
+            if let Some(raw_test_case_id) = test_case.extra.get("id") {
+                let test_case_id = uuid::Uuid::parse_str(raw_test_case_id).unwrap_or_default();
+                if test_case_id.get_version() != Some(uuid::Version::Sha1) {
+                    test_case_validation.add_issue(JunitValidationIssue::Invalid(
+                        JunitTestCaseValidationIssueInvalid::TestCaseInvalidId(
+                            raw_test_case_id.to_string().clone(),
+                        ),
+                    ));
+                }
+            }
+
             match validate_field_len::<MAX_FIELD_LEN, _>(
                 test_case
                     .extra
@@ -295,6 +317,8 @@ pub enum JunitTestSuiteValidationIssueSubOptimal {
 pub enum JunitTestSuiteValidationIssueInvalid {
     #[error("test suite name too short")]
     TestSuiteNameTooShort(String),
+    #[error("test suite id is not a valid uuidv5")]
+    TestSuiteInvalidId(String),
 }
 
 pub type JunitTestCaseValidationIssue = JunitValidationIssue<
@@ -382,4 +406,6 @@ pub enum JunitTestCaseValidationIssueSubOptimal {
 pub enum JunitTestCaseValidationIssueInvalid {
     #[error("test case name too short")]
     TestCaseNameTooShort(String),
+    #[error("test case id is not a valid uuidv5")]
+    TestCaseInvalidId(String),
 }