diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index c1b01427..b7f87cab 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -139,6 +139,9 @@ jobs: steps: - uses: actions/checkout@v4 + - name: Delete huge unnecessary tools folder + run: rm -rf /opt/hostedtoolcache + - name: Setup Rust & Cargo uses: ./.github/actions/setup_rust_cargo diff --git a/cli-tests/src/upload.rs b/cli-tests/src/upload.rs index 1f1a824e..acf730d5 100644 --- a/cli-tests/src/upload.rs +++ b/cli-tests/src/upload.rs @@ -197,7 +197,7 @@ async fn upload_bundle() { assert_eq!(report.test_build_information, None); assert_eq!(report.test_case_runs.len(), 500); let test_case_run = &report.test_case_runs[0]; - assert!(test_case_run.id.is_empty()); + assert!(!test_case_run.id.is_empty()); assert!(!test_case_run.name.is_empty()); assert!(!test_case_run.classname.is_empty()); assert!(!test_case_run.file.is_empty()); @@ -318,7 +318,7 @@ async fn upload_bundle_using_bep() { assert_eq!(test_build_information.label, "//path:test"); let test_case_run = &report.test_case_runs[0]; - assert!(test_case_run.id.is_empty()); + assert!(!test_case_run.id.is_empty()); assert!(!test_case_run.name.is_empty()); assert!(!test_case_run.classname.is_empty()); assert!(!test_case_run.file.is_empty()); @@ -393,7 +393,7 @@ async fn upload_bundle_using_xcresult() { assert_eq!(test_result.test_build_information, None); assert_eq!(test_result.test_case_runs.len(), 17); let test_case_run = &test_result.test_case_runs[0]; - assert!(test_case_run.id.is_empty()); + assert!(!test_case_run.id.is_empty()); assert!(!test_case_run.name.is_empty()); assert!(!test_case_run.classname.is_empty()); assert_eq!(test_case_run.line, 0); diff --git a/context/src/junit/parser.rs b/context/src/junit/parser.rs index a6850409..b48863e5 100644 --- a/context/src/junit/parser.rs +++ b/context/src/junit/parser.rs @@ -334,6 +334,7 @@ impl JunitParser { ) }); test_case_run.is_quarantined = quarantined_test_ids.contains(&test_case_id); + test_case_run.id = test_case_id; test_case_run.file = file; test_case_run.line = test_case @@ -1064,6 +1065,216 @@ mod tests { }) ); assert_eq!(test_case_run1.line, 5); + // Verify that the ID field is set correctly (generated from gen_info_id) + assert_eq!( + test_case_run1.id, + gen_info_id( + org_slug.as_str(), + repo.repo_full_name().as_str(), + Some("test.java"), + Some("test"), + Some("testsuite"), + Some("test_variant_truncation1"), + None, + "", + ) + ); + + let test_case_run2 = &test_case_runs[1]; + assert_eq!(test_case_run2.name, "test_variant_truncation2"); + assert_eq!(test_case_run2.parent_name, "testsuite"); + assert_eq!(test_case_run2.classname, ""); + assert_eq!(test_case_run2.status, TestCaseRunStatus::Failure as i32); + assert_eq!(test_case_run2.status_output_message, "Test failed"); + assert_eq!(test_case_run2.file, "test.java"); + assert_eq!(test_case_run2.attempt_number, 0); + assert!(!test_case_run2.is_quarantined); + // Verify that the ID field is set correctly for test_case_run2 + assert_eq!( + test_case_run2.id, + gen_info_id( + org_slug.as_str(), + repo.repo_full_name().as_str(), + Some("test.java"), + None, // No classname for test_case_run2 + Some("testsuite"), + Some("test_variant_truncation2"), + None, + "", + ) + ); + } + + #[test] + fn test_into_test_case_runs_with_custom_id() { + // Test that custom IDs from xcresult (or other sources) are preserved + 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 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]; + + // Verify that the custom ID from the XML is preserved + assert_eq!(test_case_run.id, "custom-uuid-1234-5678"); + assert_eq!(test_case_run.name, "test_with_custom_id"); + assert_eq!(test_case_run.file, "test.swift"); + assert_eq!(test_case_run.classname, "TestClass"); + } + + #[test] + fn test_into_test_case_runs_mixed_ids() { + // Test mix of custom IDs and generated IDs + 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 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 + let test_case_run1 = &test_case_runs[0]; + assert_eq!(test_case_run1.id, "xcresult-uuid-abcd"); + assert_eq!(test_case_run1.name, "test_with_id"); + + // Second test case should have a generated ID + let test_case_run2 = &test_case_runs[1]; + assert_eq!( + test_case_run2.id, + gen_info_id( + org_slug.as_str(), + repo.repo_full_name().as_str(), + Some("test.swift"), + Some("TestClass"), + Some("testsuite"), + Some("test_without_id"), + None, + "", + ) + ); + assert_eq!(test_case_run2.name, "test_without_id"); + } + + #[test] + fn test_into_test_case_runs_original() { + let mut junit_parser = JunitParser::new(); + let file_contents = r#" + + + + + + but was: ]]> + + + + + + + + "#; + 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 test_case_runs = junit_parser.into_test_case_runs( + None, + &org_slug, + &repo, + &[gen_info_id( + org_slug.as_str(), + repo.repo_full_name().as_str(), + Some("test.java"), + Some("test"), + Some("testsuite"), + Some("test_variant_truncation1"), + None, + "", + )], + ); + assert_eq!(test_case_runs.len(), 2); + let test_case_run1 = &test_case_runs[0]; + assert_eq!(test_case_run1.name, "test_variant_truncation1"); + assert_eq!(test_case_run1.parent_name, "testsuite"); + assert_eq!(test_case_run1.classname, "test"); + assert_eq!(test_case_run1.status, TestCaseRunStatus::Failure as i32); + assert_eq!( + test_case_run1.status_output_message, + "Expected: but was: " + ); + assert_eq!(test_case_run1.file, "test.java"); + assert_eq!(test_case_run1.attempt_number, 0); + assert!(test_case_run1.is_quarantined); + assert_eq!( + test_case_run1.started_at, + Some(Timestamp { + seconds: 1696161600, + nanos: 0 + }) + ); + assert_eq!( + test_case_run1.finished_at, + Some(Timestamp { + seconds: 1696161600, + nanos: 1000000 + }) + ); + assert_eq!(test_case_run1.line, 5); + // Verify that the ID field is set correctly (generated from gen_info_id) + assert_eq!( + test_case_run1.id, + gen_info_id( + org_slug.as_str(), + repo.repo_full_name().as_str(), + Some("test.java"), + Some("test"), + Some("testsuite"), + Some("test_variant_truncation1"), + None, + "", + ) + ); let test_case_run2 = &test_case_runs[1]; assert_eq!(test_case_run2.name, "test_variant_truncation2"); @@ -1074,6 +1285,20 @@ mod tests { assert_eq!(test_case_run2.file, "test.java"); assert_eq!(test_case_run2.attempt_number, 0); assert!(!test_case_run2.is_quarantined); + // Verify that the ID field is set correctly for test_case_run2 + assert_eq!( + test_case_run2.id, + gen_info_id( + org_slug.as_str(), + repo.repo_full_name().as_str(), + Some("test.java"), + None, // No classname for test_case_run2 + Some("testsuite"), + Some("test_variant_truncation2"), + None, + "", + ) + ); assert_eq!( test_case_run2.started_at, Some(Timestamp { diff --git a/xcresult/Cargo.toml b/xcresult/Cargo.toml index 8108dec9..ce59e844 100644 --- a/xcresult/Cargo.toml +++ b/xcresult/Cargo.toml @@ -28,7 +28,7 @@ tracing = "0.1.41" uuid = { version = "1.10.0", features = ["v5"] } [dev-dependencies] -context = { path = "../context" } +context = { path = "../context", features = ["bindings"] } flate2 = "1.0.34" pretty_assertions = "0.6" tar = "0.4.42" diff --git a/xcresult/src/xcresult.rs b/xcresult/src/xcresult.rs index f2e98123..6ff44706 100644 --- a/xcresult/src/xcresult.rs +++ b/xcresult/src/xcresult.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::str; use std::{fs, path::Path, time::Duration}; +use chrono::{DateTime, Utc}; use quick_junit::{NonSuccessKind, Report, TestCase, TestCaseStatus, TestRerun, TestSuite}; use crate::types::{ @@ -9,7 +10,7 @@ use crate::types::{ SWIFT_DEFAULT_TEST_SUITE_NAME, }; use crate::xcresult_legacy::XCResultTestLegacy; -use crate::xcrun::xcresulttool_get_test_results_tests; +use crate::xcrun::{xcresulttool_get_object, xcresulttool_get_test_results_tests}; #[derive(Debug, Clone)] pub struct XCResult { @@ -17,6 +18,7 @@ pub struct XCResult { org_url_slug: String, repo_full_name: String, legacy_xcresult_tests: HashMap, + test_run_started_at: Option>, } impl XCResult { @@ -33,17 +35,61 @@ impl XCResult { e ) })?; - let legacy_xcresult_tests = match XCResultTestLegacy::generate_from_object( - &absolute_path, - use_experimental_failure_summary, - ) { - Ok(tests) => tests, + + // Call xcresulttool_get_object once and use it for both timestamp extraction and legacy tests + let actions_invocation_record = xcresulttool_get_object(&absolute_path); + + // Extract test run start time from the actions invocation record + let test_run_started_at = match &actions_invocation_record { + Ok(record) => { + record + .actions + .as_ref() + .and_then(|arr| arr.values.first()) + .and_then(|action_record| { + action_record.started_time.as_ref().and_then(|date| { + // xcresult uses format like "2024-09-30T12:12:51.159-0700" without colon in timezone + DateTime::parse_from_rfc3339(&date.value) + .or_else(|_| { + DateTime::parse_from_str(&date.value, "%Y-%m-%dT%H:%M:%S%.3f%z") + }) + .ok() + .map(|dt| dt.with_timezone(&Utc)) + }) + }) + } + Err(e) => { + tracing::warn!("Failed to get test run start time from xcresult: {}", e); + None + } + }; + + // Generate legacy test info from the same actions invocation record + let legacy_xcresult_tests = match actions_invocation_record { + Ok(record) => { + match XCResultTestLegacy::generate_from_record( + &absolute_path, + record, + use_experimental_failure_summary, + ) { + Ok(tests) => tests, + Err(e) => { + tracing::warn!( + "Failed to generate legacy XCResultTestLegacy objects: {}", + e + ); + tracing::warn!( + "Attempting to continue without legacy XCResultTestLegacy objects" + ); + HashMap::new() + } + } + } Err(e) => { tracing::warn!( - "Failed to generate legacy XCResultTestLegacy objects: {}", + "Failed to get actions invocation record: {}, continuing without legacy tests", e ); - tracing::warn!("Attempting to continue without legacy XCResultTestLegacy objects"); HashMap::new() } }; @@ -52,6 +98,7 @@ impl XCResult { legacy_xcresult_tests, org_url_slug, repo_full_name, + test_run_started_at, }) } @@ -203,6 +250,11 @@ impl XCResult { test_case.set_time(duration); } + // Set timestamp to test run start time (applies to all tests in the run) + if let Some(started_at) = self.test_run_started_at { + test_case.set_timestamp(started_at); + } + if let Some(node_identifier) = &xcresult_test_case.node_identifier { let id = self.generate_id(node_identifier); test_case.extra.insert("id".into(), id.into()); diff --git a/xcresult/src/xcresult_legacy.rs b/xcresult/src/xcresult_legacy.rs index 6c9582ef..52f47a38 100644 --- a/xcresult/src/xcresult_legacy.rs +++ b/xcresult/src/xcresult_legacy.rs @@ -77,6 +77,18 @@ impl XCResultTestLegacy { use_experimental_failure_summary: bool, ) -> anyhow::Result> { let actions_invocation_record = xcresulttool_get_object(path.as_ref())?; + Self::generate_from_record( + path, + actions_invocation_record, + use_experimental_failure_summary, + ) + } + + pub fn generate_from_record>( + path: T, + actions_invocation_record: legacy_schema::ActionsInvocationRecord, + use_experimental_failure_summary: bool, + ) -> anyhow::Result> { let test_plans = actions_invocation_record .actions .as_ref() diff --git a/xcresult/tests/data/test-ExpectedFailures.junit.xml b/xcresult/tests/data/test-ExpectedFailures.junit.xml index 8f53e676..9bed3ecc 100644 --- a/xcresult/tests/data/test-ExpectedFailures.junit.xml +++ b/xcresult/tests/data/test-ExpectedFailures.junit.xml @@ -1,140 +1,140 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + diff --git a/xcresult/tests/data/test-swift-mix.junit.xml b/xcresult/tests/data/test-swift-mix.junit.xml index c6cff888..964ff917 100644 --- a/xcresult/tests/data/test-swift-mix.junit.xml +++ b/xcresult/tests/data/test-swift-mix.junit.xml @@ -1,16 +1,16 @@ - + - + - + - + diff --git a/xcresult/tests/data/test-swift-without-test-suites.junit.xml b/xcresult/tests/data/test-swift-without-test-suites.junit.xml index 5818a320..9d51f4f3 100644 --- a/xcresult/tests/data/test-swift-without-test-suites.junit.xml +++ b/xcresult/tests/data/test-swift-without-test-suites.junit.xml @@ -1,9 +1,9 @@ - + - + diff --git a/xcresult/tests/data/test-timestamp.xcresult.tar.gz b/xcresult/tests/data/test-timestamp.xcresult.tar.gz new file mode 100644 index 00000000..ce60ce77 Binary files /dev/null and b/xcresult/tests/data/test-timestamp.xcresult.tar.gz differ diff --git a/xcresult/tests/data/test1.junit.xml b/xcresult/tests/data/test1.junit.xml index 4fb34b8a..f7bbce3d 100644 --- a/xcresult/tests/data/test1.junit.xml +++ b/xcresult/tests/data/test1.junit.xml @@ -1,39 +1,39 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + diff --git a/xcresult/tests/data/test4.junit.xml b/xcresult/tests/data/test4.junit.xml index d4b17e45..1b01180b 100644 --- a/xcresult/tests/data/test4.junit.xml +++ b/xcresult/tests/data/test4.junit.xml @@ -1,1115 +1,1115 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + diff --git a/xcresult/tests/xcresult.rs b/xcresult/tests/xcresult.rs index 23963b66..8917ad01 100644 --- a/xcresult/tests/xcresult.rs +++ b/xcresult/tests/xcresult.rs @@ -31,6 +31,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_TIMESTAMP: TempDir = + unpack_archive_to_temp_dir("tests/data/test-timestamp.xcresult.tar.gz"); static ref ORG_URL_SLUG: String = String::from("trunk"); static ref REPO_FULL_NAME: String = RepoUrlParts { host: "github.com".to_string(), @@ -235,3 +237,93 @@ fn test_expected_failures_xcresult_with_valid_path() { ); } } + +#[cfg(target_os = "macos")] +#[test] +fn test_xcresult_to_bindings_report_with_id_and_timestamps() { + use std::io::BufReader; + + 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(); + + 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(); + + // 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())) + .expect("Failed to parse generated JUnit XML"); + + let test_case_runs: Vec = junit_parser + .into_test_case_runs( + None, + &ORG_URL_SLUG.as_str(), + &context::repo::RepoUrlParts { + host: "github.com".to_string(), + owner: "trunk-io".to_string(), + name: "analytics-cli".to_string(), + }, + &[], + ) + .into_iter() + .map(BindingsTestCase::from) + .collect(); + + for test_case in test_case_runs.iter() { + let extra = test_case.extra(); + let id = extra.get("id").expect("ID should be set in extra fields"); + assert!(!id.is_empty(), "ID should not be empty"); + assert!( + id.len() > 10, + "ID should be a valid UUID or hash, got: {}", + id + ); + + let timestamp = test_case.timestamp.expect("timestamp should be set"); + let timestamp_micros = test_case + .timestamp_micros + .expect("timestamp_micros should be set"); + + // Verify timestamp is reasonable (2024-09-30T19:12:51+00:00) + let timestamp_2024_09_30_19_12_51 = 1727723571; // 2024-09-30T19:12:51+00:00 + assert!( + timestamp == timestamp_2024_09_30_19_12_51, + "Timestamp should be 2024-09-30T19:12:51+00:00, got: {} ({})", + timestamp, + chrono::DateTime::from_timestamp(timestamp, 0) + .map(|dt| dt.to_rfc3339()) + .unwrap_or_default() + ); + assert!( + timestamp_micros == 1727723571159000, + "Timestamp micros should be 1727723571159000, got: {} ({})", + timestamp_micros, + chrono::DateTime::from_timestamp(timestamp_micros, 0) + .map(|dt| dt.to_rfc3339()) + .unwrap_or_default() + ); + + assert!(test_case.time.is_some(), "time should be set"); + let time = test_case.time.unwrap(); + assert!(time >= 0.0, "time should be non-negative, got: {}", time); + } +}