trunk-merge/pr-1196/ac3ad0eb-08e7-417e-a9be-2acd690ca7d7

api/Cargo.toml

@@ -27,6 +27,7 @@ tokio-retry = { version = "0.3", default-features = false }
 constants = { version = "0.0.0", path = "../constants" }
 tracing = "0.1.41"
 url = "2.5.4"
+uuid = "1.10.0"
 proto = { version = "0.0.0", path = "../proto" }
 prost = "0.12.6"
 

api/src/message.rs

@@ -23,6 +23,10 @@ pub struct CreateBundleUploadResponse {
     pub key: String,
     pub test_collection_bundle_meta_id: Option<String>,
     pub test_collection_bundle_meta_created_at: Option<String>,
+    #[serde(default)]
+    pub repo_id: Option<String>,
+    #[serde(default)]
+    pub test_collection_id: Option<String>,
 }
 
 #[derive(Debug, Serialize, Clone, Deserialize, Default)]

api/src/urls.rs

@@ -2,18 +2,39 @@ use anyhow::Context;
 use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD};
 use bundle::Test;
 use chrono::DateTime;
-use context::repo::RepoUrlParts;
+use context::{meta::id::gen_test_case_guid, repo::RepoUrlParts};
 use serde::Serialize;
 use url::{ParseError, Url, form_urlencoded};
+use uuid::Uuid;
+
+/// The server-resolved half of the test-case GUID tuple, from `CreateBundleUploadResponse`.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct TestCaseGuidScope {
+    pub test_collection_id: String,
+    pub repo_id: String,
+}
 
 pub fn url_for_test_case(
     public_api_address: &str,
     org_url_slug: &String,
     repo: &RepoUrlParts,
     test_case: &Test,
     test_collection_short_id: Option<&str>,
+    guid_scope: Option<&TestCaseGuidScope>,
 ) -> Result<String, ParseError> {
     let mut url = Url::parse(convert_to_app_url(public_api_address).as_str())?;
+
+    let guid_path = match (test_collection_short_id, guid_scope) {
+        (Some(short_id), Some(ids)) => test_case_guid(ids, test_case)
+            .map(|guid| collection_test_guid_path(org_url_slug, short_id, &guid)),
+        _ => None,
+    };
+    // No `?repo=`: the GUID resolves the whole identity tuple, unlike both short-link forms.
+    if let Some(path) = guid_path {
+        url.set_path(path.as_str());
+        return Ok(url.to_string());
+    }
+
     let path = match test_collection_short_id {
         Some(short_id) => collection_test_path(org_url_slug, short_id, test_case),
         None => test_path(org_url_slug, test_case),
@@ -23,6 +44,14 @@ pub fn url_for_test_case(
     Ok(url.to_string())
 }
 
+fn test_case_guid(ids: &TestCaseGuidScope, test_case: &Test) -> Option<Uuid> {
+    Some(gen_test_case_guid(
+        Uuid::parse_str(&ids.test_collection_id).ok()?,
+        Uuid::parse_str(&ids.repo_id).ok()?,
+        Uuid::parse_str(&test_case.id).ok()?,
+    ))
+}
+
 /// Serialized to match trunk2's `encodeBundleMetaKey` byte for byte — hence the field order
 /// and a `Serialize` struct rather than a `format!`.
 #[derive(Serialize)]
@@ -69,6 +98,10 @@ fn test_path(org_url_slug: &String, test_case: &Test) -> String {
     format!("{}/flaky-tests/test/{}", org_url_slug, test_case.id)
 }
 
+fn collection_test_guid_path(org_url_slug: &str, short_id: &str, guid: &Uuid) -> String {
+    format!("{org_url_slug}/flaky-tests/collections/{short_id}/tests/{guid}")
+}
+
 // Short-link form: the webapp resolves the repo query param to a repo id and
 // redirects to the canonical collections/<short_id>/tests/<repo_id>_<test_case_id> page.
 fn collection_test_path(org_url_slug: &String, short_id: &str, test_case: &Test) -> String {
@@ -98,31 +131,52 @@ mod tests {
     }
 
     fn test_case() -> Test {
+        test_case_with_id("c33a7f64-8f3e-5db9-b37b-2ea870d2441b")
+    }
+
+    fn test_case_with_id(id: &str) -> Test {
         Test {
             name: String::from("can math"),
             parent_name: String::from("basic suite"),
             class_name: None,
             file: None,
-            id: String::from("c33a7f64-8f3e-5db9-b37b-2ea870d2441b"),
+            id: String::from(id),
             timestamp_millis: None,
             is_quarantined: false,
             failure_message: None,
             variant: None,
         }
     }
 
-    #[test]
-    fn test_url_generated() {
-        let actual = url_for_test_case(
+    const COLLECTION_ID: &str = "018f6d3a-6f2e-4c4a-9b1e-2f3a4b5c6d7e";
+    const REPO_ID: &str = "7a1f0e3d-2b4c-4d5e-8f90-123456789abc";
+
+    fn test_guid_scope() -> TestCaseGuidScope {
+        TestCaseGuidScope {
+            test_collection_id: String::from(COLLECTION_ID),
+            repo_id: String::from(REPO_ID),
+        }
+    }
+
+    fn test_case_url(
+        test_case: &Test,
+        short_id: Option<&str>,
+        guid_scope: Option<&TestCaseGuidScope>,
+    ) -> Result<String, ParseError> {
+        url_for_test_case(
             &String::from("https://api.trunk-staging.io"),
             &String::from("bad-app-org"),
             &test_repo(),
-            &test_case(),
-            None,
-        );
+            test_case,
+            short_id,
+            guid_scope,
+        )
+    }
 
+    #[test]
+    fn test_url_generated() {
         assert_eq!(
-            actual,
+            test_case_url(&test_case(), None, None),
             Ok(String::from(
                 "https://app.trunk-staging.io/bad-app-org/flaky-tests/test/c33a7f64-8f3e-5db9-b37b-2ea870d2441b?repo=bad-app%2Fios-app"
             )),
@@ -173,19 +227,44 @@ mod tests {
 
     #[test]
     fn test_collection_url_generated() {
-        let actual = url_for_test_case(
-            &String::from("https://api.trunk-staging.io"),
-            &String::from("bad-app-org"),
-            &test_repo(),
-            &test_case(),
-            Some("tc_123"),
+        assert_eq!(
+            test_case_url(&test_case(), Some("tc_123"), None),
+            Ok(String::from(
+                "https://app.trunk-staging.io/bad-app-org/flaky-tests/collections/tc_123/t/c33a7f64-8f3e-5db9-b37b-2ea870d2441b?repo=bad-app%2Fios-app"
+            )),
         );
+    }
+
+    #[test]
+    fn test_collection_guid_url_generated() {
+        let test_case = test_case_with_id("88e5353c-190c-5dce-9d06-0e66c3e062b1");
 
         assert_eq!(
-            actual,
+            test_case_url(&test_case, Some("tc_123"), Some(&test_guid_scope())),
             Ok(String::from(
-                "https://app.trunk-staging.io/bad-app-org/flaky-tests/collections/tc_123/t/c33a7f64-8f3e-5db9-b37b-2ea870d2441b?repo=bad-app%2Fios-app"
+                "https://app.trunk-staging.io/bad-app-org/flaky-tests/collections/tc_123/tests/bfeebcf4-72d1-887d-8bcd-788d0dec7f97"
             )),
         );
     }
+
+    #[test]
+    fn test_collection_guid_url_falls_back_for_an_unparseable_server_id() {
+        let ids = TestCaseGuidScope {
+            test_collection_id: String::from("tc_123"),
+            repo_id: String::from(REPO_ID),
+        };
+
+        assert_eq!(
+            test_case_url(&test_case(), Some("tc_123"), Some(&ids)),
+            test_case_url(&test_case(), Some("tc_123"), None),
+        );
+    }
+
+    #[test]
+    fn test_repo_scoped_url_ignores_the_guid_scope() {
+        assert_eq!(
+            test_case_url(&test_case(), None, Some(&test_guid_scope())),
+            test_case_url(&test_case(), None, None),
+        );
+    }
 }

cli/src/context_quarantine.rs

@@ -484,12 +484,14 @@ fn log_failure(
         .test_collection_short_id
         .as_deref()
         .filter(|_| !hide_test_collection_links);
+    // createBundleUpload has not run yet, so there are no ids to mint a GUID from.
     let url = match url_for_test_case(
         &api_client.api_host,
         &request.org_url_slug,
         &request.repo,
         failure,
         test_collection_short_id,
+        None,
     ) {
         Ok(url) => format!("Learn more > {}", url),
         Err(_) => String::from(""),

cli/src/upload_command.rs

@@ -4,10 +4,7 @@ use std::path::PathBuf;
 use std::sync::mpsc::Sender;
 
 use api::client::{ApiClient, ApiErrorEndpoint};
-use api::{
-    client::get_api_host,
-    urls::{url_for_test_case, url_for_upload},
-};
+use api::urls::{TestCaseGuidScope, url_for_test_case, url_for_upload};
 use bundle::{BundleMeta, BundlerUtil, QuarantineResolutionMode, Test, unzip_tarball};
 use clap::{ArgAction, Args};
 use codeowners::OwnersSource;
@@ -471,6 +468,7 @@ pub struct UploadRunResult {
     pub test_collection_short_id: Option<String>,
     pub hide_test_collection_links: bool,
     pub api_address: String,
+    pub guid_scope: Option<TestCaseGuidScope>,
 }
 
 pub struct RunUploadOptions {
@@ -719,21 +717,32 @@ pub async fn run_upload(
     if upload_bundle_result.is_err() {
         tracing::error!("Failed to upload bundle");
     }
-    let error_report = match upload_bundle_result {
-        Ok(upload_bundle_result) => {
+    let (guid_scope, error_report) = match upload_bundle_result {
+        Ok(uploaded) => {
             if upload_args.dry_run {
                 let curr_dir = env::current_dir()?;
                 let bundle_file = curr_dir.join(DRY_RUN_OUTPUT_DIR);
-                unzip_tarball(&upload_bundle_result.0, &bundle_file)?;
+                unzip_tarball(&uploaded.tarball, &bundle_file)?;
             }
-            None
+            (uploaded.guid_scope, None)
         }
-        Err(e) => Some(ErrorReport::new(
-            e,
-            upload_args.org_url_slug.clone(),
-            Some("There was an unexpected error that occurred while uploading test results".into()),
-        )),
+        Err(e) => (
+            None,
+            Some(ErrorReport::new(
+                e,
+                upload_args.org_url_slug.clone(),
+                Some(
+                    "There was an unexpected error that occurred while uploading test results"
+                        .into(),
+                ),
+            )),
+        ),
     };
+    if guid_scope.is_none() && upload_args.test_collection_short_id.is_some() {
+        tracing::debug!(
+            "No test collection ids returned for this upload; test links will use the short-link form"
+        );
+    }
     Ok(UploadRunResult {
         quarantine_context,
         error_report,
@@ -746,17 +755,25 @@ pub async fn run_upload(
             .filter(|id| !id.is_empty()),
         hide_test_collection_links: upload_args.hide_test_collection_links,
         api_address: api_client.api_host.clone(),
+        guid_scope,
     })
 }
 
+struct UploadedBundle {
+    tarball: PathBuf,
+    // directory is removed on drop
+    _temp_dir: TempDir,
+    guid_scope: Option<TestCaseGuidScope>,
+}
+
 async fn upload_bundle(
     meta: &mut BundleMeta,
     requested_test_collection_short_id: Option<String>,
     api_client: &ApiClient,
     bep_result: Option<BepParseResult>,
     exit_code: i32,
     dry_run: bool,
-) -> anyhow::Result<(PathBuf, TempDir)> {
+) -> anyhow::Result<UploadedBundle> {
     let upload_result = gather_upload_id_context(
         meta,
         requested_test_collection_short_id,
@@ -774,7 +791,11 @@ async fn upload_bundle(
 
     if dry_run {
         tracing::info!("Dry run enabled, not uploading bundle to S3");
-        return Ok((bundle_temp_file, bundle_temp_dir));
+        return Ok(UploadedBundle {
+            tarball: bundle_temp_file,
+            _temp_dir: bundle_temp_dir,
+            guid_scope: None,
+        });
     }
 
     match upload_result {
@@ -791,7 +812,16 @@ async fn upload_bundle(
                 );
             }
 
-            Ok((bundle_temp_file, bundle_temp_dir))
+            Ok(UploadedBundle {
+                tarball: bundle_temp_file,
+                _temp_dir: bundle_temp_dir,
+                guid_scope: upload.test_collection_id.zip(upload.repo_id).map(
+                    |(test_collection_id, repo_id)| TestCaseGuidScope {
+                        test_collection_id,
+                        repo_id,
+                    },
+                ),
+            })
         }
         Err(e) => {
             tracing::error!("Failed to gather upload ID: {}", e);
@@ -911,6 +941,10 @@ impl EndOutput for UploadRunResult {
             .test_collection_short_id
             .as_deref()
             .filter(|_| !self.hide_test_collection_links);
+        let guid_scope = self
+            .guid_scope
+            .as_ref()
+            .filter(|_| !self.hide_test_collection_links);
 
         // Helper closure to render the test table
         let render_test_table = |tests: &[Test]| -> anyhow::Result<Lines> {
@@ -955,11 +989,12 @@ impl EndOutput for UploadRunResult {
                     test_line.pad_left(2);
                     output.push(test_line);
                     let link = url_for_test_case(
-                        &get_api_host(),
+                        &self.api_address,
                         &self.quarantine_context.org_url_slug,
                         &self.quarantine_context.repo,
                         test,
                         test_collection_short_id,
+                        guid_scope,
                     )?;
                     let mut link_output = Line::from_iter([
                         Span::new_unstyled("⤷ ")?,