diff --git a/api/src/message.rs b/api/src/message.rs index 9539908b..349423f5 100644 --- a/api/src/message.rs +++ b/api/src/message.rs @@ -23,6 +23,47 @@ pub struct CreateBundleUploadResponse { pub key: String, pub test_collection_bundle_meta_id: Option, pub test_collection_bundle_meta_created_at: Option, + /// Repo UUID used to key test collection URLs; absent on older servers. + pub repo_id: Option, + /// Same server-side calculation as `quarantine_resolution_mode`; picks the test URL format. + #[serde(default)] + pub test_collection_migration_state: TestCollectionMigrationState, +} + +#[derive(Debug, Serialize, Clone, Copy, PartialEq, Eq, Default)] +#[serde(rename_all = "snake_case")] +pub enum TestCollectionMigrationState { + Repo, + TestCollection, + #[default] + Unspecified, +} + +impl<'de> Deserialize<'de> for TestCollectionMigrationState { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Ok( + match serde_json::Value::deserialize(deserializer)?.as_str() { + Some("repo") => Self::Repo, + Some("test_collection") => Self::TestCollection, + _ => Self::Unspecified, + }, + ) + } +} + +impl From + for proto::upload_metrics::trunk::TestCollectionMigrationState +{ + fn from(state: TestCollectionMigrationState) -> Self { + match state { + TestCollectionMigrationState::Repo => Self::Repo, + TestCollectionMigrationState::TestCollection => Self::TestCollection, + TestCollectionMigrationState::Unspecified => Self::Unspecified, + } + } } #[derive(Debug, Serialize, Clone, Deserialize, Default)] @@ -33,6 +74,8 @@ pub struct GetQuarantineConfigResponse { pub quarantined_tests: Vec, #[serde(default)] pub quarantine_resolution_mode: QuarantineResolutionMode, + /// Repo UUID used to key test collection URLs; absent on older servers. + pub repo_id: Option, } #[derive(Debug, Serialize, Clone, Deserialize, PartialEq, Eq)] @@ -46,22 +89,6 @@ pub struct GetQuarantineConfigRequest { pub test_collection_short_id: Option, } -#[derive(Debug, Serialize, Clone, Deserialize, PartialEq, Eq)] -#[serde(rename_all = "camelCase")] -pub struct CreateBundleUploadIntentRequest { - pub repo: RepoUrlParts, - pub org_url_slug: String, - pub client_version: String, -} - -#[derive(Debug, Serialize, Clone, Deserialize, PartialEq, Eq)] -#[serde(rename_all = "camelCase")] -pub struct CreateBundleUploadIntentResponse { - pub repo: RepoUrlParts, - pub org_url_slug: String, - pub client_version: String, -} - #[derive(Debug, Serialize, Clone, Deserialize, PartialEq)] pub struct TelemetryUploadMetricsRequest { pub upload_metrics: proto::upload_metrics::trunk::UploadMetrics, diff --git a/api/src/urls.rs b/api/src/urls.rs index 4f1350a1..154448c0 100644 --- a/api/src/urls.rs +++ b/api/src/urls.rs @@ -2,15 +2,30 @@ use bundle::Test; use context::repo::RepoUrlParts; use url::{ParseError, Url, form_urlencoded}; +/// Locates a test collection test page, which is keyed by `{repo_id}_{test_case_id}`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CollectionUrlParts { + pub short_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, + collection: Option<&CollectionUrlParts>, ) -> Result { let mut url = Url::parse(convert_to_app_url(public_api_address).as_str())?; - url.set_path(test_path(org_url_slug, test_case).as_str()); - url.set_query(Some(repo_query(repo).as_str())); + match collection { + Some(collection) => { + url.set_path(collection_test_path(org_url_slug, collection, test_case).as_str()); + } + None => { + url.set_path(test_path(org_url_slug, test_case).as_str()); + url.set_query(Some(repo_query(repo).as_str())); + } + } Ok(url.to_string()) } @@ -22,6 +37,17 @@ fn test_path(org_url_slug: &String, test_case: &Test) -> String { format!("{}/flaky-tests/test/{}", org_url_slug, test_case.id) } +fn collection_test_path( + org_url_slug: &String, + collection: &CollectionUrlParts, + test_case: &Test, +) -> String { + format!( + "{}/flaky-tests/collections/{}/tests/{}_{}", + org_url_slug, collection.short_id, collection.repo_id, test_case.id + ) +} + fn repo_query(repo: &RepoUrlParts) -> String { let value: String = form_urlencoded::byte_serialize(format!("{}/{}", repo.owner, repo.name).as_bytes()) @@ -29,37 +55,68 @@ fn repo_query(repo: &RepoUrlParts) -> String { format!("repo={}", value) } -#[test] -fn test_url_generated() { - let repo = RepoUrlParts { - host: String::from("https://github.com"), - owner: String::from("bad-app"), - name: String::from("ios-app"), - }; +#[cfg(test)] +mod tests { + use super::*; + + fn test_repo() -> RepoUrlParts { + RepoUrlParts { + host: String::from("https://github.com"), + owner: String::from("bad-app"), + name: String::from("ios-app"), + } + } + + fn test_case() -> 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"), + timestamp_millis: None, + is_quarantined: false, + failure_message: None, + variant: None, + } + } + + #[test] + fn test_url_generated() { + let actual = url_for_test_case( + &String::from("https://api.trunk-staging.io"), + &String::from("bad-app-org"), + &test_repo(), + &test_case(), + None, + ); - let 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"), - timestamp_millis: None, - is_quarantined: false, - failure_message: None, - variant: None, - }; + assert_eq!( + actual, + Ok(String::from( + "https://app.trunk-staging.io/bad-app-org/flaky-tests/test/c33a7f64-8f3e-5db9-b37b-2ea870d2441b?repo=bad-app%2Fios-app" + )), + ); + } - let actual = url_for_test_case( - &String::from("https://api.trunk-staging.io"), - &String::from("bad-app-org"), - &repo, - &test, - ); + #[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(&CollectionUrlParts { + short_id: String::from("tc_123"), + repo_id: String::from("12345678-abcd-1234-8678-123456789022"), + }), + ); - assert_eq!( - actual, - Ok(String::from( - "https://app.trunk-staging.io/bad-app-org/flaky-tests/test/c33a7f64-8f3e-5db9-b37b-2ea870d2441b?repo=bad-app%2Fios-app" - )), - ); + assert_eq!( + actual, + Ok(String::from( + "https://app.trunk-staging.io/bad-app-org/flaky-tests/collections/tc_123/tests/12345678-abcd-1234-8678-123456789022_c33a7f64-8f3e-5db9-b37b-2ea870d2441b" + )), + ); + } } diff --git a/cli/src/context.rs b/cli/src/context.rs index 1e5da157..238afcd5 100644 --- a/cli/src/context.rs +++ b/cli/src/context.rs @@ -804,6 +804,7 @@ pub async fn gather_exit_code_and_quarantined_tests_context( org_url_slug: String::default(), fetch_status: QuarantineFetchStatus::FetchSkipped, quarantine_resolution_mode: api::message::QuarantineResolutionMode::Unspecified, + collection_url_parts: None, } } else { // default to success if no test run result (i.e. `upload`) diff --git a/cli/src/context_quarantine.rs b/cli/src/context_quarantine.rs index 9bb400e6..75489bfa 100644 --- a/cli/src/context_quarantine.rs +++ b/cli/src/context_quarantine.rs @@ -3,7 +3,11 @@ use std::{ io::{BufReader, Read}, }; -use api::{client::ApiClient, message::QuarantineResolutionMode, urls::url_for_test_case}; +use api::{ + client::ApiClient, + message::QuarantineResolutionMode, + urls::{CollectionUrlParts, url_for_test_case}, +}; use bundle::{ FileSet, FileSetBuilder, FileSetTestRunnerReport, FileSetType, QuarantineBulkTestStatus, Test, }; @@ -48,6 +52,8 @@ pub struct QuarantineContext { pub org_url_slug: String, pub fetch_status: QuarantineFetchStatus, pub quarantine_resolution_mode: QuarantineResolutionMode, + /// Present when the server resolved quarantining via a test collection and returned the repo UUID. + pub collection_url_parts: Option, } impl QuarantineContext { pub fn skip_fetch(failures: Vec) -> Self { @@ -59,6 +65,7 @@ impl QuarantineContext { org_url_slug: String::default(), fetch_status: QuarantineFetchStatus::FetchSkipped, quarantine_resolution_mode: QuarantineResolutionMode::Unspecified, + collection_url_parts: None, } } @@ -71,6 +78,7 @@ impl QuarantineContext { org_url_slug: String::default(), fetch_status: QuarantineFetchStatus::FetchFailed(error), quarantine_resolution_mode: QuarantineResolutionMode::Unspecified, + collection_url_parts: None, } } } @@ -326,41 +334,57 @@ pub async fn gather_quarantine_context( failures: Vec::default(), fetch_status: QuarantineFetchStatus::FetchSkipped, quarantine_resolution_mode: QuarantineResolutionMode::Unspecified, + collection_url_parts: None, }); } - let (quarantine_config, quarantine_fetch_status) = if !failed_tests_extractor - .failed_tests() - .is_empty() - && file_set_builder - .file_sets() - .iter() - // internal files track quarantine status directly, so we don't need to check them - .any(|file_set| file_set.file_set_type == FileSetType::Junit) - { - tracing::info!("Checking if failed tests can be quarantined"); - match api_client.get_quarantining_config(request).await { - anyhow::Result::Ok(response) => { - if let Some(line) = response.quarantine_resolution_mode.resolution_log_line( - request.test_collection_short_id.as_deref(), - &request.repo, - ) { - tracing::info!("{line}"); + let (quarantine_config, quarantine_fetch_status) = + if !failed_tests_extractor.failed_tests().is_empty() + && file_set_builder + .file_sets() + .iter() + // internal files track quarantine status directly, so we don't need to check them + .any(|file_set| file_set.file_set_type == FileSetType::Junit) + { + tracing::info!("Checking if failed tests can be quarantined"); + match api_client.get_quarantining_config(request).await { + anyhow::Result::Ok(response) => { + if let Some(line) = response.quarantine_resolution_mode.resolution_log_line( + request.test_collection_short_id.as_deref(), + &request.repo, + ) { + tracing::info!("{line}"); + } + (Some(response), QuarantineFetchStatus::FetchSucceeded) } - (Some(response), QuarantineFetchStatus::FetchSucceeded) + anyhow::Result::Err(error) => (None, QuarantineFetchStatus::FetchFailed(error)), } - anyhow::Result::Err(error) => (None, QuarantineFetchStatus::FetchFailed(error)), - } - } else { - tracing::debug!("Skipping quarantine check."); - (None, QuarantineFetchStatus::FetchSkipped) - }; + } else { + tracing::debug!("Skipping quarantine check."); + (None, QuarantineFetchStatus::FetchSkipped) + }; let quarantine_resolution_mode = quarantine_config .as_ref() .map(|config| config.quarantine_resolution_mode) .unwrap_or_default(); + // Collection links need both server confirmation and the repo UUID; otherwise stay legacy. + let collection_url_parts = + if quarantine_resolution_mode == QuarantineResolutionMode::TestCollection { + request + .test_collection_short_id + .clone() + .zip( + quarantine_config + .as_ref() + .and_then(|config| config.repo_id.clone()), + ) + .map(|(short_id, repo_id)| CollectionUrlParts { short_id, repo_id }) + } else { + None + }; + // if quarantining is not enabled, return exit code and empty quarantine status if quarantine_config .as_ref() @@ -379,6 +403,7 @@ pub async fn gather_quarantine_context( org_url_slug: request.org_url_slug.clone(), fetch_status: quarantine_fetch_status, quarantine_resolution_mode, + collection_url_parts, }); } else { // quarantining is enabled, continue with quarantine process and update exit code @@ -413,9 +438,14 @@ pub async fn gather_quarantine_context( quarantined_failures.len(), pluralize("failure", quarantined_failures.len() as isize, false), ); - quarantined_failures - .iter() - .for_each(|quarantined_failure| log_failure(quarantined_failure, request, api_client)); + quarantined_failures.iter().for_each(|quarantined_failure| { + log_failure( + quarantined_failure, + request, + api_client, + collection_url_parts.as_ref(), + ) + }); } if !failures.is_empty() { @@ -424,9 +454,9 @@ pub async fn gather_quarantine_context( failures.len(), pluralize("failure", quarantined_failures.len() as isize, false), ); - failures - .iter() - .for_each(|failure| log_failure(failure, request, api_client)); + failures.iter().for_each(|failure| { + log_failure(failure, request, api_client, collection_url_parts.as_ref()) + }); } let quarantined_failure_count = quarantined_failures.len(); quarantine_results.quarantine_results = quarantined_failures; @@ -466,6 +496,7 @@ pub async fn gather_quarantine_context( org_url_slug: request.org_url_slug.clone(), fetch_status: quarantine_fetch_status, quarantine_resolution_mode, + collection_url_parts, }) } @@ -473,12 +504,14 @@ fn log_failure( failure: &Test, request: &api::message::GetQuarantineConfigRequest, api_client: &ApiClient, + collection_url_parts: Option<&CollectionUrlParts>, ) { let url = match url_for_test_case( &api_client.api_host, &request.org_url_slug, &request.repo, failure, + collection_url_parts, ) { Ok(url) => format!("Learn more > {}", url), Err(_) => String::from(""), @@ -680,6 +713,7 @@ mod tests { org_url_slug: String::new(), fetch_status: QuarantineFetchStatus::FetchSucceeded, quarantine_resolution_mode: QuarantineResolutionMode::Unspecified, + collection_url_parts: None, }; assert_eq!( quarantine_query_result(false, &success_ctx), @@ -710,6 +744,7 @@ mod tests { repo: RepoUrlParts::default(), org_url_slug: String::new(), quarantine_resolution_mode: QuarantineResolutionMode::Unspecified, + collection_url_parts: None, }; assert_eq!( quarantine_query_result(false, &repo_disabled_ctx), diff --git a/cli/src/upload_command.rs b/cli/src/upload_command.rs index 426dca44..cb6844a5 100644 --- a/cli/src/upload_command.rs +++ b/cli/src/upload_command.rs @@ -4,7 +4,11 @@ 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}; +use api::{ + client::get_api_host, + message::TestCollectionMigrationState, + urls::{CollectionUrlParts, url_for_test_case}, +}; use bundle::{BundleMeta, BundlerUtil, QuarantineResolutionMode, Test, unzip_tarball}; use clap::{ArgAction, Args}; use codeowners::OwnersSource; @@ -372,6 +376,8 @@ pub struct UploadRunResult { pub validations: JunitReportValidations, pub validation_report: ValidationReport, pub show_failure_messages: bool, + /// Present when the upload's `test_collection_migration_state` is `test_collection` and the repo UUID is known. + pub collection_url_parts: Option, } pub struct RunUploadOptions { @@ -573,6 +579,11 @@ pub async fn run_upload( .await; let quarantine_query_result = quarantine_query_result_override .unwrap_or_else(|| quarantine_query_result(disable_quarantining, &quarantine_context)); + let test_collection_migration_state = upload_bundle_result + .as_ref() + .ok() + .map(|output| output.test_collection_migration_state) + .unwrap_or_default(); let upload_metrics = proto::upload_metrics::trunk::UploadMetrics { client_version: Some(proto::upload_metrics::trunk::Semver { major: env!("CARGO_PKG_VERSION_MAJOR").parse().unwrap_or_default(), @@ -595,6 +606,11 @@ pub async fn run_upload( quarantine_resolution_mode, ) .into(), + test_collection_migration_state: + proto::upload_metrics::trunk::TestCollectionMigrationState::from( + test_collection_migration_state, + ) + .into(), }; let mut request = api::message::TelemetryUploadMetricsRequest { upload_metrics }; if !upload_args.dry_run { @@ -619,12 +635,16 @@ pub async fn run_upload( if upload_bundle_result.is_err() { tracing::error!("Failed to upload bundle"); } + let collection_url_parts = upload_bundle_result + .as_ref() + .ok() + .and_then(|output| output.collection_url_parts.clone()); let error_report = match upload_bundle_result { - Ok(upload_bundle_result) => { + Ok(upload_bundle_output) => { 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(&upload_bundle_output.bundle_temp_file, &bundle_file)?; } None } @@ -641,9 +661,19 @@ pub async fn run_upload( validations, validation_report: upload_args.validation_report, show_failure_messages: upload_args.show_failure_messages, + collection_url_parts, }) } +#[derive(Debug)] +struct UploadBundleOutput { + bundle_temp_file: PathBuf, + /// Held so the temp dir isn't removed until the bundle is no longer needed. + _bundle_temp_dir: TempDir, + collection_url_parts: Option, + test_collection_migration_state: TestCollectionMigrationState, +} + async fn upload_bundle( meta: &mut BundleMeta, requested_test_collection_short_id: Option, @@ -651,25 +681,27 @@ async fn upload_bundle( bep_result: Option, exit_code: i32, dry_run: bool, -) -> anyhow::Result<(PathBuf, TempDir)> { +) -> anyhow::Result { let upload_result = gather_upload_id_context( meta, - requested_test_collection_short_id, + requested_test_collection_short_id.clone(), api_client, dry_run, ) .await; - let ( - bundle_temp_file, - // directory is removed on drop - bundle_temp_dir, - ) = BundlerUtil::new(meta, bep_result).make_tarball_in_temp_dir()?; + let (bundle_temp_file, bundle_temp_dir) = + BundlerUtil::new(meta, bep_result).make_tarball_in_temp_dir()?; tracing::info!("Flushed temporary tarball to {:?}", bundle_temp_file); if dry_run { tracing::info!("Dry run enabled, not uploading bundle to S3"); - return Ok((bundle_temp_file, bundle_temp_dir)); + return Ok(UploadBundleOutput { + bundle_temp_file, + _bundle_temp_dir: bundle_temp_dir, + collection_url_parts: None, + test_collection_migration_state: TestCollectionMigrationState::Unspecified, + }); } match upload_result { @@ -686,7 +718,23 @@ async fn upload_bundle( ); } - Ok((bundle_temp_file, bundle_temp_dir)) + // `test_collection_migration_state ?? repo` picks the link format; without `repo_id` stay legacy. + let collection_url_parts = if upload.test_collection_migration_state + == TestCollectionMigrationState::TestCollection + { + requested_test_collection_short_id + .zip(upload.repo_id) + .map(|(short_id, repo_id)| CollectionUrlParts { short_id, repo_id }) + } else { + None + }; + + Ok(UploadBundleOutput { + bundle_temp_file, + _bundle_temp_dir: bundle_temp_dir, + collection_url_parts, + test_collection_migration_state: upload.test_collection_migration_state, + }) } Err(e) => { tracing::error!("Failed to gather upload ID: {}", e); @@ -818,6 +866,7 @@ impl EndOutput for UploadRunResult { &self.quarantine_context.org_url_slug, &self.quarantine_context.repo, test, + self.collection_url_parts.as_ref(), )?; let mut link_output = Line::from_iter([ Span::new_unstyled("⤷ ")?, diff --git a/cli/tests/test.rs b/cli/tests/test.rs index 7d163b80..285acfe4 100644 --- a/cli/tests/test.rs +++ b/cli/tests/test.rs @@ -5,7 +5,7 @@ use std::{ use api::message::{ CreateBundleUploadRequest, CreateBundleUploadResponse, GetQuarantineConfigRequest, - GetQuarantineConfigResponse, + GetQuarantineConfigResponse, TestCollectionMigrationState, }; use assert_matches::assert_matches; use axum::{Json, extract::State}; @@ -308,6 +308,9 @@ async fn quarantining_resets_fail_code() { test_collection_bundle_meta_created_at: Some(String::from( "2026-05-10T12:34:56.000Z", )), + repo_id: Some(String::from("6b7e8c9d-1a2b-4c3d-8e5f-9a0b1c2d3e4f")), + test_collection_migration_state: + TestCollectionMigrationState::TestCollection, })) } }, @@ -369,6 +372,9 @@ async fn quarantining_not_active_when_disable_quarantining_set() { test_collection_bundle_meta_created_at: Some(String::from( "2026-05-10T12:34:56.000Z", )), + repo_id: Some(String::from("6b7e8c9d-1a2b-4c3d-8e5f-9a0b1c2d3e4f")), + test_collection_migration_state: + TestCollectionMigrationState::TestCollection, })) } }, @@ -431,6 +437,9 @@ async fn quarantining_not_active_when_disable_true_but_use_true() { test_collection_bundle_meta_created_at: Some(String::from( "2026-05-10T12:34:56.000Z", )), + repo_id: Some(String::from("6b7e8c9d-1a2b-4c3d-8e5f-9a0b1c2d3e4f")), + test_collection_migration_state: + TestCollectionMigrationState::TestCollection, })) } }, diff --git a/cli/tests/upload.rs b/cli/tests/upload.rs index 072c67ab..0dfd23f2 100644 --- a/cli/tests/upload.rs +++ b/cli/tests/upload.rs @@ -5,7 +5,7 @@ use std::{fs, io::BufReader}; use api::message::{ CreateBundleUploadRequest, CreateBundleUploadResponse, GetQuarantineConfigRequest, - GetQuarantineConfigResponse, + GetQuarantineConfigResponse, TestCollectionMigrationState, }; use assert_matches::assert_matches; use axum::body::{Body, Bytes}; @@ -14,7 +14,7 @@ use axum::{Json, extract::State, http::StatusCode, response::Response}; use bundle::{ BundleMeta, FileSetType, INTERNAL_BIN_FILENAME, QuarantineResolutionMode, TestCollectionProps, }; -use chrono::{DateTime, TimeDelta}; +use chrono::{DateTime, TimeDelta, Utc}; use clap::Parser; mod common; @@ -23,7 +23,7 @@ use common::command_builder::CommandBuilder; use common::utils::{ generate_mock_bazel_bep, generate_mock_bazel_bep_no_file_attrs, generate_mock_codeowners, generate_mock_git_repo, generate_mock_invalid_junit_xmls, generate_mock_valid_junit_xmls, - generate_mock_valid_junit_xmls_with_failures, + generate_mock_valid_junit_xmls_with_failures, write_junit_xml_to_dir, }; use constants::EXIT_FAILURE; use context::{ @@ -299,12 +299,32 @@ async fn upload_bundle() { )); } +/// The v5 UUID the CLI derives for `failing-test` in [`write_fixed_failing_junit_xml`]'s report. +const FIXED_FAILING_TEST_ID: &str = "327b018d-d7bb-51f9-9f85-bdca020cc810"; + +fn write_fixed_failing_junit_xml>(directory: T) { + let timestamp = (Utc::now() - TimeDelta::minutes(1)).to_rfc3339(); + write_junit_xml_to_dir( + &format!( + r#" + + + + + + +"# + ), + directory, + ); +} + // NOTE: must be multi threaded to start a mock server #[tokio::test(flavor = "multi_thread")] async fn upload_bundle_records_quarantine_resolution_mode() { let temp_dir = tempdir().unwrap(); generate_mock_git_repo(&temp_dir); - generate_mock_valid_junit_xmls(&temp_dir); + write_fixed_failing_junit_xml(&temp_dir); let mut mock_server_builder = MockServerBuilder::new(); mock_server_builder.set_get_quarantining_config_handler( @@ -313,6 +333,7 @@ async fn upload_bundle_records_quarantine_resolution_mode() { is_disabled: false, quarantined_tests: Vec::new(), quarantine_resolution_mode: QuarantineResolutionMode::TestCollection, + repo_id: Some(String::from("6b7e8c9d-1a2b-4c3d-8e5f-9a0b1c2d3e4f")), }) }, ); @@ -323,9 +344,13 @@ async fn upload_bundle_records_quarantine_resolution_mode() { .arg("--test-collection-id") .arg("tc_123") .assert() - // the mock JUnit reports contain unquarantined failures, so the upload itself succeeds + // the JUnit report contains an unquarantined failure, so the upload itself succeeds // while the command exits non-zero - .failure(); + .failure() + .stderr(predicate::str::contains(format!( + "{}/test-org/flaky-tests/collections/tc_123/tests/6b7e8c9d-1a2b-4c3d-8e5f-9a0b1c2d3e4f_{FIXED_FAILING_TEST_ID}", + state.host + ))); let requests = state.requests.lock().unwrap().clone(); let tar_extract_directory = requests @@ -355,6 +380,54 @@ async fn upload_bundle_records_quarantine_resolution_mode() { upload_metrics.quarantine_resolution_mode, i32::from(proto::upload_metrics::trunk::QuarantineResolutionMode::TestCollection) ); + assert_eq!( + upload_metrics.test_collection_migration_state, + i32::from(proto::upload_metrics::trunk::TestCollectionMigrationState::TestCollection) + ); +} + +// NOTE: must be multi threaded to start a mock server +#[tokio::test(flavor = "multi_thread")] +async fn upload_falls_back_to_legacy_test_links_without_repo_id() { + let temp_dir = tempdir().unwrap(); + generate_mock_git_repo(&temp_dir); + write_fixed_failing_junit_xml(&temp_dir); + + let mut mock_server_builder = MockServerBuilder::new(); + mock_server_builder.set_create_bundle_handler( + |State(state): State, + Json(_): Json| async move { + let host = &state.host; + Ok::, String>(Json(CreateBundleUploadResponse { + id: String::from("test-bundle-upload-id"), + id_v2: String::from("test-bundle-upload-id-v2"), + url: format!("{host}/s3upload"), + key: String::from("unused"), + test_collection_bundle_meta_id: Some(String::from( + "82c6a6e5-f8ea-4d93-9a26-b8ab6ff8f6bc", + )), + test_collection_bundle_meta_created_at: Some(String::from( + "2026-05-10T12:34:56.000Z", + )), + repo_id: None, + test_collection_migration_state: TestCollectionMigrationState::TestCollection, + })) + }, + ); + let state = mock_server_builder.spawn_mock_server().await; + + CommandBuilder::upload(temp_dir.path(), state.host.clone()) + .command() + .arg("--test-collection-id") + .arg("tc_123") + .assert() + .failure() + // collection mode without a repo UUID must fall back to the legacy repo-name link + .stderr(predicate::str::contains(format!( + "{}/test-org/flaky-tests/test/{FIXED_FAILING_TEST_ID}?repo=trunk-io%2Fanalytics-cli", + state.host + ))) + .stderr(predicate::str::contains("/flaky-tests/collections/").not()); } // NOTE: must be multi threaded to start a mock server @@ -628,6 +701,8 @@ async fn upload_bundle_without_canonical_test_collection_metadata_keeps_bundle_g key: String::from("unused"), test_collection_bundle_meta_id: None, test_collection_bundle_meta_created_at: None, + repo_id: None, + test_collection_migration_state: TestCollectionMigrationState::Repo, })) }, ); @@ -1472,6 +1547,8 @@ async fn quarantines_tests_regardless_of_upload() { test_collection_bundle_meta_created_at: Some(String::from( "2026-05-10T12:34:56.000Z", )), + repo_id: Some(String::from("6b7e8c9d-1a2b-4c3d-8e5f-9a0b1c2d3e4f")), + test_collection_migration_state: TestCollectionMigrationState::TestCollection, }) .into_response() } @@ -2211,6 +2288,9 @@ async fn do_not_quarantines_tests_when_quarantine_disabled_set() { test_collection_bundle_meta_created_at: Some(String::from( "2026-05-10T12:34:56.000Z", )), + repo_id: Some(String::from("6b7e8c9d-1a2b-4c3d-8e5f-9a0b1c2d3e4f")), + test_collection_migration_state: + TestCollectionMigrationState::TestCollection, })) } }; diff --git a/proto/proto/upload_metrics.proto b/proto/proto/upload_metrics.proto index 2a14c8c9..b02fb9d5 100644 --- a/proto/proto/upload_metrics.proto +++ b/proto/proto/upload_metrics.proto @@ -27,6 +27,13 @@ enum QuarantineResolutionMode { QUARANTINE_RESOLUTION_MODE_TEST_COLLECTION = 2; } +// Whether the server resolved the upload to a test collection or the repo +enum TestCollectionMigrationState { + TEST_COLLECTION_MIGRATION_STATE_UNSPECIFIED = 0; + TEST_COLLECTION_MIGRATION_STATE_REPO = 1; + TEST_COLLECTION_MIGRATION_STATE_TEST_COLLECTION = 2; +} + message UploadMetrics { Semver client_version = 1; Repo repo = 2; @@ -37,6 +44,7 @@ message UploadMetrics { string failure_reason = 7; QuarantineQueryResult quarantine_query_result = 8; QuarantineResolutionMode quarantine_resolution_mode = 9; + TestCollectionMigrationState test_collection_migration_state = 10; } // Used by the analytics-uploader, kept here to avoid collisions in the future. diff --git a/test_utils/src/mock_server.rs b/test_utils/src/mock_server.rs index ee014bb8..54a1fa74 100644 --- a/test_utils/src/mock_server.rs +++ b/test_utils/src/mock_server.rs @@ -8,7 +8,7 @@ use std::{ use api::message::{ CreateBundleUploadRequest, CreateBundleUploadResponse, GetQuarantineConfigRequest, - GetQuarantineConfigResponse, + GetQuarantineConfigResponse, TestCollectionMigrationState, }; use axum::{ Json, Router, @@ -180,6 +180,8 @@ pub async fn create_bundle_handler( key: String::from("unused"), test_collection_bundle_meta_id: Some(String::from("82c6a6e5-f8ea-4d93-9a26-b8ab6ff8f6bc")), test_collection_bundle_meta_created_at: Some(String::from("2026-05-10T12:34:56.000Z")), + repo_id: Some(String::from("6b7e8c9d-1a2b-4c3d-8e5f-9a0b1c2d3e4f")), + test_collection_migration_state: TestCollectionMigrationState::TestCollection, }) }