feat(xcresult): resolve test files from declarations, and stop dropping nested suites

cli/Cargo.toml

@@ -71,6 +71,7 @@ more-asserts = "0.3.1"
 predicates = "3.0.3"
 pretty_assertions = "0.6"
 prost-wkt-types = { version = "0.5.1", features = ["vendored-protox"] }
+rstest = "0.26.1"
 
 [build-dependencies]
 vergen = { version = "8.3.1", features = [

cli/src/context.rs

@@ -40,7 +40,7 @@ use proto::test_context::test_run::{
 use regex::Regex;
 use tempfile::TempDir;
 #[cfg(target_os = "macos")]
-use xcresult::xcresult::XCResult;
+use xcresult::{test_locations::Limits, xcresult::XCResult};
 
 use crate::error_report::InterruptingError;
 use crate::{
@@ -137,6 +137,8 @@ pub fn gather_initial_test_context(
         pr_number,
         #[cfg(target_os = "macos")]
         use_experimental_failure_summary,
+        #[cfg(target_os = "macos")]
+        use_experimental_xcresult_test_locations,
         ..
     } = upload_args;
 
@@ -151,18 +153,23 @@ pub fn gather_initial_test_context(
     )?;
     tracing::debug!("Found repo state: {:?}", repo);
 
+    #[cfg(target_os = "macos")]
+    let xcresult_options = XCResultOptions {
+        repo: &repo.repo,
+        org_url_slug: &org_url_slug,
+        repo_root: &repo.repo_root,
+        use_experimental_failure_summary,
+        use_experimental_test_locations: use_experimental_xcresult_test_locations,
+    };
+
     let (junit_path_wrappers, bep_result, junit_path_wrappers_temp_dir) =
         coalesce_junit_path_wrappers(
             junit_paths,
             bazel_bep_path,
             #[cfg(target_os = "macos")]
             xcresult_path,
             #[cfg(target_os = "macos")]
-            &repo.repo,
-            #[cfg(target_os = "macos")]
-            org_url_slug.clone(),
-            #[cfg(target_os = "macos")]
-            use_experimental_failure_summary,
+            &xcresult_options,
             test_reports,
             allow_empty_test_results,
         )?;
@@ -620,13 +627,22 @@ fn parse_as_bep(dir: String) -> anyhow::Result<BepParseResult> {
     result
 }
 
+/// What the xcresult conversion needs beyond the bundle, kept together so an option added
+/// later does not thread another `#[cfg]`-gated parameter through three signatures.
+#[cfg(target_os = "macos")]
+struct XCResultOptions<'a> {
+    repo: &'a RepoUrlParts,
+    org_url_slug: &'a str,
+    repo_root: &'a str,
+    use_experimental_failure_summary: bool,
+    use_experimental_test_locations: bool,
+}
+
 fn coalesce_junit_path_wrappers(
     junit_paths: Vec<String>,
     bazel_bep_path: Option<String>,
     #[cfg(target_os = "macos")] xcresult_path: Option<String>,
-    #[cfg(target_os = "macos")] repo: &RepoUrlParts,
-    #[cfg(target_os = "macos")] org_url_slug: String,
-    #[cfg(target_os = "macos")] use_experimental_failure_summary: bool,
+    #[cfg(target_os = "macos")] xcresult_options: &XCResultOptions,
     test_reports: Vec<String>,
     allow_empty_test_results: bool,
 ) -> anyhow::Result<(
@@ -673,13 +689,7 @@ fn coalesce_junit_path_wrappers(
     #[cfg(target_os = "macos")]
     if xcresult_path.is_some() {
         let temp_dir = tempfile::tempdir()?;
-        let temp_paths = handle_xcresult(
-            &temp_dir,
-            xcresult_path,
-            repo,
-            &org_url_slug,
-            use_experimental_failure_summary,
-        )?;
+        let temp_paths = handle_xcresult(&temp_dir, xcresult_path, xcresult_options)?;
         _junit_path_wrappers_temp_dir = Some(temp_dir);
         junit_path_wrappers = [junit_path_wrappers.as_slice(), temp_paths.as_slice()].concat();
         if junit_path_wrappers.is_empty() {
@@ -711,11 +721,7 @@ fn coalesce_junit_path_wrappers(
                 #[cfg(target_os = "macos")]
                 &test_report,
                 #[cfg(target_os = "macos")]
-                repo,
-                #[cfg(target_os = "macos")]
-                &org_url_slug,
-                #[cfg(target_os = "macos")]
-                use_experimental_failure_summary,
+                xcresult_options,
             ) {
                 #[cfg(target_os = "macos")]
                 {
@@ -741,20 +747,12 @@ fn coalesce_junit_path_wrappers(
 
 fn parse_as_xcresult(
     #[cfg(target_os = "macos")] test_report: &String,
-    #[cfg(target_os = "macos")] repo: &RepoUrlParts,
-    #[cfg(target_os = "macos")] org_url_slug: &String,
-    #[cfg(target_os = "macos")] use_experimental_failure_summary: bool,
+    #[cfg(target_os = "macos")] xcresult_options: &XCResultOptions,
 ) -> Option<tempfile::TempDir> {
     #[cfg(target_os = "macos")]
     {
         let temp_dir = tempfile::tempdir().ok()?;
-        let temp_paths = handle_xcresult(
-            &temp_dir,
-            Some(test_report.clone()),
-            repo,
-            &org_url_slug,
-            use_experimental_failure_summary,
-        );
+        let temp_paths = handle_xcresult(&temp_dir, Some(test_report.clone()), xcresult_options);
         if temp_paths.is_ok() {
             return Some(temp_dir);
         } else {
@@ -874,18 +872,28 @@ pub async fn gather_upload_id_context(
 fn handle_xcresult(
     junit_temp_dir: &tempfile::TempDir,
     xcresult_path: Option<String>,
-    repo: &RepoUrlParts,
-    org_url_slug: &String,
-    use_experimental_failure_summary: bool,
+    options: &XCResultOptions,
 ) -> Result<Vec<JunitReportFileWithTestRunnerReport>, anyhow::Error> {
     let mut temp_paths = Vec::new();
     if let Some(xcresult_path) = xcresult_path {
-        let xcresult = XCResult::new(
-            xcresult_path,
-            org_url_slug.clone(),
-            repo.repo_full_name(),
-            use_experimental_failure_summary,
-        )?;
+        let org_url_slug = options.org_url_slug.to_string();
+        let repo_full_name = options.repo.repo_full_name();
+        let xcresult = if options.use_experimental_test_locations {
+            XCResult::new_with_declaration_locations(
+                xcresult_path,
+                org_url_slug,
+                repo_full_name,
+                options.repo_root,
+                Limits::default(),
+            )?
+        } else {
+            XCResult::new(
+                xcresult_path,
+                org_url_slug,
+                repo_full_name,
+                options.use_experimental_failure_summary,
+            )?
+        };
         let junits = xcresult.generate_junits();
         if junits.is_empty() {
             return Err(anyhow::anyhow!(
@@ -997,6 +1005,8 @@ mod tests {
     #[cfg(target_os = "macos")]
     use context::repo::RepoUrlParts;
 
+    #[cfg(target_os = "macos")]
+    use crate::context::XCResultOptions;
     use crate::context::{coalesce_junit_path_wrappers, gather_initial_test_context};
     use crate::upload_command::UploadArgs;
 
@@ -1060,17 +1070,21 @@ mod tests {
             owner: "trunk-io".to_string(),
             name: "analytics-cli".to_string(),
         };
+        #[cfg(target_os = "macos")]
+        let xcresult_options = XCResultOptions {
+            repo: &repo,
+            org_url_slug: "test",
+            repo_root: "test",
+            use_experimental_failure_summary: false,
+            use_experimental_test_locations: false,
+        };
         let result_err = coalesce_junit_path_wrappers(
             vec!["test".into()],
             Some("test".into()),
             #[cfg(target_os = "macos")]
             Some("test".into()),
             #[cfg(target_os = "macos")]
-            &repo,
-            #[cfg(target_os = "macos")]
-            "test".into(),
-            #[cfg(target_os = "macos")]
-            false,
+            &xcresult_options,
             Vec::new(),
             false,
         );
@@ -1081,11 +1095,7 @@ mod tests {
             #[cfg(target_os = "macos")]
             Some("test".into()),
             #[cfg(target_os = "macos")]
-            &repo,
-            #[cfg(target_os = "macos")]
-            "test".into(),
-            #[cfg(target_os = "macos")]
-            false,
+            &xcresult_options,
             Vec::new(),
             true,
         );
@@ -1106,17 +1116,21 @@ mod tests {
             owner: "trunk-io".to_string(),
             name: "analytics-cli".to_string(),
         };
+        #[cfg(target_os = "macos")]
+        let xcresult_options = XCResultOptions {
+            repo: &repo,
+            org_url_slug: "test",
+            repo_root: "test",
+            use_experimental_failure_summary: false,
+            use_experimental_test_locations: false,
+        };
         let result_ok = coalesce_junit_path_wrappers(
             Vec::new(),
             None,
             #[cfg(target_os = "macos")]
             None,
             #[cfg(target_os = "macos")]
-            &repo,
-            #[cfg(target_os = "macos")]
-            "test".into(),
-            #[cfg(target_os = "macos")]
-            false,
+            &xcresult_options,
             vec!["test".into()],
             true,
         );

cli/src/upload_command.rs

@@ -275,6 +275,21 @@ pub struct UploadArgs {
         hide = true
     )]
     pub use_experimental_failure_summary: bool,
+    #[cfg(target_os = "macos")]
+    #[arg(
+        long,
+        env = constants::TRUNK_USE_EXPERIMENTAL_XCRESULT_TEST_LOCATIONS_ENV,
+        help = "Flag to take an xcresult test's file from where a language server says it is declared, rather than from the failure that surfaced it. Reads the bundle with no legacy `xcresulttool get object` calls.",
+        action = ArgAction::Set,
+        required = false,
+        require_equals = true,
+        num_args = 0..=1,
+        default_value = "false",
+        default_missing_value = "true",
+        hide = true,
+        conflicts_with = "use_experimental_failure_summary"
+    )]
+    pub use_experimental_xcresult_test_locations: bool,
     #[arg(
         long,
         env = constants::TRUNK_VALIDATION_REPORT_ENV,

cli/tests/common/command_builder.rs

@@ -452,6 +452,7 @@ pub struct CommandBuilder<'a> {
     command_type: CommandType,
     current_dir: &'a Path,
     paths_state: Option<PathsState>,
+    extra_args: Vec<String>,
 }
 
 impl<'b> CommandBuilder<'b> {
@@ -463,6 +464,7 @@ impl<'b> CommandBuilder<'b> {
             },
             current_dir,
             paths_state: None,
+            extra_args: Vec::new(),
         }
     }
 
@@ -475,6 +477,7 @@ impl<'b> CommandBuilder<'b> {
             },
             current_dir,
             paths_state: None,
+            extra_args: Vec::new(),
         }
     }
 
@@ -486,6 +489,7 @@ impl<'b> CommandBuilder<'b> {
             },
             current_dir,
             paths_state: None,
+            extra_args: Vec::new(),
         }
     }
 
@@ -494,6 +498,11 @@ impl<'b> CommandBuilder<'b> {
         self
     }
 
+    pub fn extra_args(&mut self, args: &[&str]) -> &mut Self {
+        self.extra_args = args.iter().map(|arg| String::from(*arg)).collect();
+        self
+    }
+
     pub fn xcresult_path(&mut self, new_paths: &str) -> &mut Self {
         self.paths_state = Some(PathsState::XCResultPath(String::from(new_paths)));
         self
@@ -634,6 +643,7 @@ impl<'b> CommandBuilder<'b> {
             .into_iter()
             .chain(paths_args)
             .chain(self.command_type.build_args())
+            .chain(self.extra_args.clone())
             .collect()
     }
 

cli/tests/upload.rs

@@ -789,9 +789,14 @@ async fn upload_bundle_without_canonical_test_collection_metadata_keeps_bundle_g
     assert_eq!(bundle_meta.bundle_upload_id_v2, "test-bundle-upload-id-v2");
 }
 
-#[tokio::test(flavor = "multi_thread")]
+// The declaration path is exercised end-to-end here too, so a bundle that uploads cleanly
+// today cannot start failing behind the flag without this noticing.
 #[cfg(target_os = "macos")]
-async fn upload_bundle_using_xcresult() {
+#[rstest::rstest]
+#[case::default_path(&[])]
+#[case::declaration_locations(&["--use-experimental-xcresult-test-locations=true"])]
+#[tokio::test(flavor = "multi_thread")]
+async fn upload_bundle_using_xcresult(#[case] extra_args: &[&str]) {
     let temp_dir = tempdir().unwrap();
     generate_mock_git_repo(&temp_dir);
     unpack_archive_to_dir(
@@ -803,6 +808,7 @@ async fn upload_bundle_using_xcresult() {
 
     let assert = CommandBuilder::upload(temp_dir.path(), state.host.clone())
         .xcresult_path("test1.xcresult")
+        .extra_args(extra_args)
         .command()
         .assert()
         .success()

constants/src/lib.rs

@@ -57,6 +57,8 @@ pub const TRUNK_VALIDATION_REPORT_ENV: &str = "TRUNK_VALIDATION_REPORT";
 pub const TRUNK_SHOW_FAILURE_MESSAGES_ENV: &str = "TRUNK_SHOW_FAILURE_MESSAGES";
 pub const TRUNK_HIDE_TEST_COLLECTION_LINKS_ENV: &str = "TRUNK_HIDE_TEST_COLLECTION_LINKS";
 pub const TRUNK_DEBUG_ENV: &str = "TRUNK_DEBUG";
+pub const TRUNK_USE_EXPERIMENTAL_XCRESULT_TEST_LOCATIONS_ENV: &str =
+    "TRUNK_USE_EXPERIMENTAL_XCRESULT_TEST_LOCATIONS";
 // RSpec-only: when set to "true", aborts the RSpec run if quarantine lookup fails.
 // Handled in rspec-trunk-flaky-tests/lib/trunk_spec_helper.rb, not the CLI.
 pub const TRUNK_QUARANTINE_QUERY_FAILURE_EXIT_ENV: &str = "TRUNK_QUARANTINE_QUERY_FAILURE_EXIT";
@@ -92,6 +94,7 @@ pub const TRUNK_ENVS_TO_CAPTURE: &[&str] = &[
     TRUNK_SHOW_FAILURE_MESSAGES_ENV,
     TRUNK_HIDE_TEST_COLLECTION_LINKS_ENV,
     TRUNK_DEBUG_ENV,
+    TRUNK_USE_EXPERIMENTAL_XCRESULT_TEST_LOCATIONS_ENV,
 ];
 
 pub const ENVS_TO_GET: &[&str] = &[