diff --git a/bundle/src/types.rs b/bundle/src/types.rs index c7c6d7d6..bd2d4799 100644 --- a/bundle/src/types.rs +++ b/bundle/src/types.rs @@ -104,6 +104,8 @@ pub struct QuarantineBulkTestStatus { pub group_is_quarantined: bool, #[serde(rename = "quarantineResults")] pub quarantine_results: Vec, + #[serde(rename = "quarantiningDisabledForRepo")] + pub quarantining_disabled_for_repo: bool, } #[derive(Debug, Serialize, Clone)] diff --git a/cli/src/context_quarantine.rs b/cli/src/context_quarantine.rs index 4ecd8747..597680c1 100644 --- a/cli/src/context_quarantine.rs +++ b/cli/src/context_quarantine.rs @@ -21,6 +21,7 @@ use context::{ }; use pluralizer::pluralize; use prost::Message; +use proto::upload_metrics::trunk::QuarantineQueryResult; #[derive(Debug)] pub enum QuarantineFetchStatus { @@ -71,6 +72,21 @@ impl QuarantineContext { } } +pub fn quarantine_query_result( + disable_quarantining: bool, + ctx: &QuarantineContext, +) -> proto::upload_metrics::trunk::QuarantineQueryResult { + if disable_quarantining || ctx.quarantine_status.quarantining_disabled_for_repo { + return QuarantineQueryResult::Disabled; + } + + match &ctx.fetch_status { + QuarantineFetchStatus::FetchFailed(_) => QuarantineQueryResult::Failure, + QuarantineFetchStatus::FetchSkipped => QuarantineQueryResult::Skipped, + QuarantineFetchStatus::FetchSucceeded => QuarantineQueryResult::Success, + } +} + fn convert_case_to_test + ToString>( repo: &RepoUrlParts, org_slug: T, @@ -334,7 +350,10 @@ pub async fn gather_quarantine_context( tracing::info!("Quarantining is not enabled, not quarantining any tests"); return Ok(QuarantineContext { exit_code, - quarantine_status: QuarantineBulkTestStatus::default(), + quarantine_status: QuarantineBulkTestStatus { + quarantining_disabled_for_repo: true, + ..Default::default() + }, failures: failed_tests_extractor.failed_tests().to_vec(), repo: request.repo.clone(), org_url_slug: request.org_url_slug.clone(), @@ -627,6 +646,53 @@ mod tests { assert_eq!(multi_failures[0].name, "Hello"); } + #[test] + fn test_quarantine_query_result_mapper() { + use proto::upload_metrics::trunk::QuarantineQueryResult; + + let success_ctx = QuarantineContext { + exit_code: 0, + quarantine_status: QuarantineBulkTestStatus::default(), + failures: vec![], + repo: RepoUrlParts::default(), + org_url_slug: String::new(), + fetch_status: QuarantineFetchStatus::FetchSucceeded, + }; + assert_eq!( + quarantine_query_result(false, &success_ctx), + QuarantineQueryResult::Success + ); + assert_eq!( + quarantine_query_result(true, &success_ctx), + QuarantineQueryResult::Disabled + ); + + let skipped_ctx = QuarantineContext { + fetch_status: QuarantineFetchStatus::FetchSkipped, + ..success_ctx + }; + assert_eq!( + quarantine_query_result(false, &skipped_ctx), + QuarantineQueryResult::Skipped + ); + + let repo_disabled_ctx = QuarantineContext { + quarantine_status: QuarantineBulkTestStatus { + quarantining_disabled_for_repo: true, + ..Default::default() + }, + fetch_status: QuarantineFetchStatus::FetchSucceeded, + exit_code: 0, + failures: vec![], + repo: RepoUrlParts::default(), + org_url_slug: String::new(), + }; + assert_eq!( + quarantine_query_result(false, &repo_disabled_ctx), + QuarantineQueryResult::Disabled + ); + } + #[test] fn test_convert_case_to_test_populates_failure_message() { use quick_junit::{NonSuccessKind, TestCase, TestCaseStatus, TestSuite}; diff --git a/cli/src/main.rs b/cli/src/main.rs index 31f7abd6..91b911da 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -16,7 +16,7 @@ use trunk_analytics_cli::{ context::gather_debug_props, error_report::ErrorReport, test_command::{TestArgs, run_test}, - upload_command::{UploadArgs, UploadRunResult, run_upload}, + upload_command::{RunUploadOptions, UploadArgs, UploadRunResult, run_upload}, validate_command::{ValidateArgs, ValidateRunResult, run_validate}, }; @@ -192,7 +192,14 @@ async fn run(cli: Cli, render_sender: Sender) -> anyhow::Result< ); match cli.command { Commands::Upload(upload_args) => { - let result = run_upload(upload_args, None, None, Some(render_sender)).await?; + let result = run_upload( + upload_args, + RunUploadOptions { + render_sender: Some(render_sender), + ..Default::default() + }, + ) + .await?; Ok(RunResult::Upload(result)) } Commands::Test(test_args) => { diff --git a/cli/src/test_command.rs b/cli/src/test_command.rs index 0d359a0b..5f370b73 100644 --- a/cli/src/test_command.rs +++ b/cli/src/test_command.rs @@ -1,7 +1,7 @@ use std::{ env, process::{Command, Stdio}, - sync::{mpsc::Sender, Arc}, + sync::{Arc, mpsc::Sender}, time::SystemTime, }; @@ -9,16 +9,16 @@ use clap::Args; use constants::EXIT_FAILURE; use display::{ end_output::EndOutput, - message::{send_message, DisplayMessage}, + message::{DisplayMessage, send_message}, }; use superconsole::{ - style::{Attribute, Stylize}, Line, Span, + style::{Attribute, Stylize}, }; use crate::{ context::{gather_debug_props, gather_initial_test_context}, - upload_command::{run_upload, UploadArgs, UploadRunResult}, + upload_command::{RunUploadOptions, UploadArgs, UploadRunResult, run_upload}, }; enum RunOutput { @@ -133,9 +133,11 @@ pub async fn run_test( let upload_run_result = run_upload( upload_args, - Some(test_context), - Some(test_run_result.clone()), - None, + RunUploadOptions { + pre_test_context: Some(test_context), + test_run_result: Some(test_run_result.clone()), + ..Default::default() + }, ) .await; match upload_run_result { diff --git a/cli/src/upload_command.rs b/cli/src/upload_command.rs index 51096bf9..9f30bcce 100644 --- a/cli/src/upload_command.rs +++ b/cli/src/upload_command.rs @@ -18,7 +18,7 @@ use superconsole::{ }; use tempfile::TempDir; -use crate::context_quarantine::QuarantineContext; +use crate::context_quarantine::{QuarantineContext, quarantine_query_result}; use crate::validate_command::JunitReportValidations; use crate::{ context::{ @@ -355,12 +355,35 @@ pub struct UploadRunResult { pub show_failure_messages: bool, } +pub struct RunUploadOptions { + pub pre_test_context: Option, + pub test_run_result: Option, + pub render_sender: Option>, + pub quarantine_query_result_override: + Option, +} + +impl Default for RunUploadOptions { + fn default() -> Self { + Self { + pre_test_context: None, + test_run_result: None, + render_sender: None, + quarantine_query_result_override: None, + } + } +} + pub async fn run_upload( upload_args: UploadArgs, - pre_test_context: Option, - test_run_result: Option, - render_sender: Option>, + options: RunUploadOptions, ) -> anyhow::Result { + let RunUploadOptions { + pre_test_context, + test_run_result, + render_sender, + quarantine_query_result_override, + } = options; // grab the exec start if provided (`test` subcommand) or use the current time let cli_started_at = if let Some(test_run_result) = test_run_result.as_ref() { test_run_result @@ -509,6 +532,8 @@ pub async fn run_upload( upload_args.dry_run, ) .await; + let quarantine_query_result = quarantine_query_result_override + .unwrap_or_else(|| quarantine_query_result(disable_quarantining, &quarantine_context)); 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(), @@ -526,6 +551,7 @@ pub async fn run_upload( upload_finished_at: Some(chrono::Utc::now().into()), failed: false, failure_reason: "".into(), + quarantine_query_result: quarantine_query_result.into(), }; let mut request = api::message::TelemetryUploadMetricsRequest { upload_metrics }; if !upload_args.dry_run { diff --git a/cli/tests/upload.rs b/cli/tests/upload.rs index ae99132f..b38d7ac8 100644 --- a/cli/tests/upload.rs +++ b/cli/tests/upload.rs @@ -34,6 +34,7 @@ use lazy_static::lazy_static; use predicates::prelude::*; use pretty_assertions::assert_eq; use prost::Message; +use proto::upload_metrics::trunk::QuarantineQueryResult; use tempfile::tempdir; #[cfg(target_os = "macos")] use test_utils::inputs::unpack_archive_to_dir; @@ -1277,6 +1278,10 @@ async fn telemetry_upload_metrics_on_upload_failure() { assert_eq!(telemetry_request_repo.owner, "trunk-io"); assert_eq!(telemetry_request_repo.name, "analytics-cli"); assert_eq!(telemetry_request.failure_reason, "400_bad_request"); + assert_eq!( + QuarantineQueryResult::try_from(telemetry_request.quarantine_query_result).unwrap(), + QuarantineQueryResult::Disabled + ); // HINT: View CLI output with `cargo test -- --nocapture` println!("{assert}"); @@ -1308,11 +1313,126 @@ async fn telemetry_upload_metrics_on_upload_success() { assert_eq!(telemetry_request_repo.owner, "trunk-io"); assert_eq!(telemetry_request_repo.name, "analytics-cli"); assert_eq!(telemetry_request.failure_reason, ""); + assert_eq!( + QuarantineQueryResult::try_from(telemetry_request.quarantine_query_result).unwrap(), + QuarantineQueryResult::Disabled + ); // HINT: View CLI output with `cargo test -- --nocapture` println!("{assert}"); } +#[tokio::test(flavor = "multi_thread")] +async fn telemetry_upload_metrics_quarantine_query_result_success() { + let temp_dir = tempdir().unwrap(); + generate_mock_git_repo(&temp_dir); + generate_mock_valid_junit_xmls_with_failures(&temp_dir); + + let state = MockServerBuilder::new().spawn_mock_server().await; + + CommandBuilder::upload(temp_dir.path(), state.host.clone()) + .command() + .assert() + .failure(); + + let requests = state.requests.lock().unwrap().clone(); + let quarantine_query_result = requests + .iter() + .rev() + .find_map(|req| match req { + RequestPayload::TelemetryUploadMetrics(ur) => Some(ur.quarantine_query_result), + _ => None, + }) + .expect("telemetry upload metrics request"); + assert_eq!( + QuarantineQueryResult::try_from(quarantine_query_result).unwrap(), + QuarantineQueryResult::Success + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn telemetry_upload_metrics_quarantine_query_result_failure() { + let temp_dir = tempdir().unwrap(); + generate_mock_git_repo(&temp_dir); + generate_mock_valid_junit_xmls_with_failures(&temp_dir); + + let mut mock_server_builder = MockServerBuilder::new(); + mock_server_builder.set_get_quarantining_config_handler( + |Json(_): Json| async { + Err::, StatusCode>(StatusCode::INTERNAL_SERVER_ERROR) + }, + ); + let state = mock_server_builder.spawn_mock_server().await; + + CommandBuilder::upload(temp_dir.path(), state.host.clone()) + .command() + .assert() + .failure(); + + let requests = state.requests.lock().unwrap().clone(); + let quarantine_query_result = requests + .iter() + .rev() + .find_map(|req| match req { + RequestPayload::TelemetryUploadMetrics(ur) => Some(ur.quarantine_query_result), + _ => None, + }) + .expect("telemetry upload metrics request"); + assert_eq!( + QuarantineQueryResult::try_from(quarantine_query_result).unwrap(), + QuarantineQueryResult::Failure + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn telemetry_upload_metrics_quarantine_query_result_skipped() { + let temp_dir = tempdir().unwrap(); + generate_mock_git_repo(&temp_dir); + + // All-passing JUnit so quarantine lookup is skipped (no failed tests). + let test_case_options = junit_mock::TestCaseOptions { + test_case_names: Some(vec![String::from("test_case")]), + test_case_classnames: Some(vec![String::from("TestClass")]), + test_case_random_count: 0usize, + test_case_sys_out_percentage: 0u8, + test_case_sys_err_percentage: 0u8, + test_case_duration_range: vec![Duration::new(10, 0).into(), Duration::new(20, 0).into()], + test_case_success_to_skip_to_fail_to_error_percentage: vec![vec![100u8, 0u8, 0u8, 0u8]], + }; + let options = junit_mock::Options { + global: junit_mock::GlobalOptions::try_parse_from([""]).unwrap(), + report: junit_mock::ReportOptions::try_parse_from([""]).unwrap(), + test_suite: junit_mock::TestSuiteOptions::try_parse_from([""]).unwrap(), + test_case: test_case_options, + test_rerun: junit_mock::TestRerunOptions::try_parse_from([""]).unwrap(), + }; + let mut mock = JunitMock::new(options); + let reports = mock.generate_reports(); + mock.write_reports_to_file(temp_dir.as_ref(), reports) + .unwrap(); + + let state = MockServerBuilder::new().spawn_mock_server().await; + + CommandBuilder::upload(temp_dir.path(), state.host.clone()) + .command() + .assert() + .success(); + + let requests = state.requests.lock().unwrap().clone(); + let quarantine_query_result = requests + .iter() + .rev() + .find_map(|req| match req { + RequestPayload::TelemetryUploadMetrics(ur) => Some(ur.quarantine_query_result), + _ => None, + }) + .expect("telemetry upload metrics request"); + assert_eq!( + QuarantineQueryResult::try_from(quarantine_query_result).unwrap(), + QuarantineQueryResult::Skipped + ); +} + #[tokio::test(flavor = "multi_thread")] async fn telemetry_does_not_impact_return() { let temp_dir = tempdir().unwrap(); diff --git a/constants/src/lib.rs b/constants/src/lib.rs index 365f7828..c0bde789 100644 --- a/constants/src/lib.rs +++ b/constants/src/lib.rs @@ -21,6 +21,7 @@ pub const CODEOWNERS_LOCATIONS: &[&str] = &[".github", ".bitbucket", ".", "docs" pub const DEFAULT_ORIGIN: &str = "https://api.trunk.io"; pub const DEFAULT_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS: u64 = 5 * 60; // 5 minutes pub const CACHE_DIR: &str = "trunk-flaky-tests"; +pub const TRUNK_QUARANTINE_DISK_CACHE_DIR_ENV: &str = "TRUNK_QUARANTINE_DISK_CACHE_DIR"; pub const TRUNK_PUBLIC_API_ADDRESS_ENV: &str = "TRUNK_PUBLIC_API_ADDRESS"; pub const TRUNK_API_CLIENT_RETRY_COUNT_ENV: &str = "TRUNK_API_CLIENT_RETRY_COUNT"; @@ -52,6 +53,7 @@ pub const TRUNK_DEBUG_ENV: &str = "TRUNK_DEBUG"; // TRUNK_* environment variables to capture in bundle metadata for debugging. // TRUNK_API_TOKEN_ENV is intentionally omitted. pub const TRUNK_ENVS_TO_CAPTURE: &[&str] = &[ + TRUNK_QUARANTINE_DISK_CACHE_DIR_ENV, TRUNK_PUBLIC_API_ADDRESS_ENV, TRUNK_API_CLIENT_RETRY_COUNT_ENV, TRUNK_ORG_URL_SLUG_ENV, diff --git a/context/src/junit/bindings/test_case.rs b/context/src/junit/bindings/test_case.rs index daa081f7..ead74dec 100644 --- a/context/src/junit/bindings/test_case.rs +++ b/context/src/junit/bindings/test_case.rs @@ -332,7 +332,7 @@ impl TryInto for BindingsTestCase { properties, bazel_run_information: _, } = self; - // donotland: anything here? + let mut test_case = TestCase::new(name, status.try_into()?); test_case.classname = classname.map(|c| c.into()); test_case.assertions = assertions; diff --git a/proto/proto/upload_metrics.proto b/proto/proto/upload_metrics.proto index 701168f7..68e1999b 100644 --- a/proto/proto/upload_metrics.proto +++ b/proto/proto/upload_metrics.proto @@ -5,6 +5,21 @@ package trunk.oss.flakytests_cli.v1; import "common.proto"; import "google/protobuf/timestamp.proto"; +enum QuarantineQueryResult { + // Default; should not be sent from new CLI versions in practice. + QUARANTINE_QUERY_RESULT_UNSPECIFIED = 0; + // Successfully retrieved quarantined test results from the API. + QUARANTINE_QUERY_RESULT_SUCCESS = 1; + // Query failed (includes auth errors like 403/404; not differentiated yet). + QUARANTINE_QUERY_RESULT_FAILURE = 2; + // Disabled via CLI flag/env var or repo setting from the DB. + QUARANTINE_QUERY_RESULT_DISABLED = 3; + // No tests failed; quarantine lookup was not needed. + QUARANTINE_QUERY_RESULT_SKIPPED = 4; + // RSpec only: quarantined results served from disk cache. + QUARANTINE_QUERY_RESULT_CACHED = 5; +} + message UploadMetrics { Semver client_version = 1; Repo repo = 2; @@ -13,6 +28,7 @@ message UploadMetrics { google.protobuf.Timestamp upload_finished_at = 5; bool failed = 6; string failure_reason = 7; + QuarantineQueryResult quarantine_query_result = 8; } // Used by the analytics-uploader, kept here to avoid collisions in the future. diff --git a/test_report/src/report.rs b/test_report/src/report.rs index 80b38f73..0fa6f490 100644 --- a/test_report/src/report.rs +++ b/test_report/src/report.rs @@ -22,8 +22,17 @@ use proto::test_context::test_run::{ use serde::{Deserialize, Serialize}; use third_party::sentry; use tracing_subscriber::{filter::FilterFn, prelude::*}; -use trunk_analytics_cli::{context::gather_initial_test_context, upload_command::run_upload}; +use trunk_analytics_cli::{ + context::gather_initial_test_context, + upload_command::{RunUploadOptions, run_upload}, +}; use uuid::Uuid; + +pub fn quarantine_disk_cache_dir() -> PathBuf { + env::var(constants::TRUNK_QUARANTINE_DISK_CACHE_DIR_ENV) + .map(PathBuf::from) + .unwrap_or_else(|_| env::temp_dir().join(constants::CACHE_DIR)) +} #[cfg(feature = "wasm")] use wasm_bindgen::prelude::wasm_bindgen; @@ -40,12 +49,21 @@ struct QuarantineConfigDiskCacheEntry { cached_at_secs: u64, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum QuarantineLookupSource { + Api, + DiskCache, + Failed, + Disabled, +} + #[derive(Debug, Clone, PartialEq)] pub struct TestReport { test_report: TestReportProto, command: String, started_at: SystemTime, quarantine_config: Option, + quarantine_lookup_source: Option, quarantined_tests_disk_cache_ttl: Duration, codeowners: Option, variant: Option, @@ -219,6 +237,7 @@ impl MutTestReport { command, started_at, quarantine_config: None, + quarantine_lookup_source: None, quarantined_tests_disk_cache_ttl, codeowners, repo, @@ -226,6 +245,24 @@ impl MutTestReport { })) } + fn record_quarantine_lookup_source(&self, source: QuarantineLookupSource) { + self.0.borrow_mut().quarantine_lookup_source = Some(source); + } + + fn quarantine_query_result_for_telemetry( + &self, + ) -> proto::upload_metrics::trunk::QuarantineQueryResult { + use proto::upload_metrics::trunk::QuarantineQueryResult; + + match self.0.borrow().quarantine_lookup_source { + Some(QuarantineLookupSource::DiskCache) => QuarantineQueryResult::Cached, + Some(QuarantineLookupSource::Api) => QuarantineQueryResult::Success, + Some(QuarantineLookupSource::Failed) => QuarantineQueryResult::Failure, + Some(QuarantineLookupSource::Disabled) => QuarantineQueryResult::Disabled, + None => QuarantineQueryResult::Skipped, + } + } + fn serialize_test_report(&self) -> Vec { prost::Message::encode_to_vec(&self.0.borrow().test_report) } @@ -391,9 +428,7 @@ impl MutTestReport { .to_string(); let quarantine_config_cache_file_name = format!("quarantine_config_{cache_key}.json"); - env::temp_dir() - .join(constants::CACHE_DIR) - .join(quarantine_config_cache_file_name) + quarantine_disk_cache_dir().join(quarantine_config_cache_file_name) } fn load_quarantine_config_from_disk_cache( @@ -493,6 +528,7 @@ impl MutTestReport { self.load_quarantine_config_from_disk_cache(&org_url_slug, &repo_url) { self.0.borrow_mut().quarantine_config = Some(cache_entry.quarantine_config); + self.record_quarantine_lookup_source(QuarantineLookupSource::DiskCache); return; } @@ -514,6 +550,11 @@ impl MutTestReport { for quarantined_test_id in response.quarantined_tests.iter() { quarantined_tests.insert(quarantined_test_id.clone(), true); } + if is_disabled { + self.record_quarantine_lookup_source(QuarantineLookupSource::Disabled); + } else { + self.record_quarantine_lookup_source(QuarantineLookupSource::Api); + } (is_disabled, false) } Err(err) => { @@ -523,6 +564,7 @@ impl MutTestReport { "Error fetching quarantined tests: {:?}", err ); + self.record_quarantine_lookup_source(QuarantineLookupSource::Failed); (false, true) } }; @@ -673,9 +715,14 @@ impl MutTestReport { .unwrap() .block_on(run_upload( upload_args, - Some(pre_test_context), - Some(test_run_result), - None, + RunUploadOptions { + pre_test_context: Some(pre_test_context), + test_run_result: Some(test_run_result), + quarantine_query_result_override: Some( + self.quarantine_query_result_for_telemetry(), + ), + ..Default::default() + }, )) { Ok(upload_result) => { if let Some(upload_bundle_error) = diff --git a/test_report/tests/common/env.rs b/test_report/tests/common/env.rs index 6ea7f7b1..f115cd61 100644 --- a/test_report/tests/common/env.rs +++ b/test_report/tests/common/env.rs @@ -4,10 +4,13 @@ use constants::{ TRUNK_ALLOW_EMPTY_TEST_RESULTS_ENV, TRUNK_API_CLIENT_RETRY_COUNT_ENV, TRUNK_API_TOKEN_ENV, TRUNK_CODEOWNERS_PATH_ENV, TRUNK_DISABLE_QUARANTINING_ENV, TRUNK_DRY_RUN_ENV, TRUNK_ORG_URL_SLUG_ENV, TRUNK_PR_NUMBER_ENV, TRUNK_PUBLIC_API_ADDRESS_ENV, - TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS_ENV, TRUNK_REPO_HEAD_AUTHOR_NAME_ENV, - TRUNK_REPO_HEAD_BRANCH_ENV, TRUNK_REPO_HEAD_COMMIT_EPOCH_ENV, TRUNK_REPO_HEAD_SHA_ENV, - TRUNK_REPO_ROOT_ENV, TRUNK_REPO_URL_ENV, TRUNK_USE_UNCLONED_REPO_ENV, TRUNK_VARIANT_ENV, + TRUNK_QUARANTINE_DISK_CACHE_DIR_ENV, TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS_ENV, + TRUNK_REPO_HEAD_AUTHOR_NAME_ENV, TRUNK_REPO_HEAD_BRANCH_ENV, TRUNK_REPO_HEAD_COMMIT_EPOCH_ENV, + TRUNK_REPO_HEAD_SHA_ENV, TRUNK_REPO_ROOT_ENV, TRUNK_REPO_URL_ENV, TRUNK_USE_UNCLONED_REPO_ENV, + TRUNK_VARIANT_ENV, }; +use tempfile::TempDir; +use test_report::report::quarantine_disk_cache_dir; /// Cleans up all TRUNK_* and CI-related environment variables to avoid test interference. pub fn cleanup_env_vars() { @@ -32,12 +35,20 @@ pub fn cleanup_env_vars() { env::remove_var("GITHUB_JOB"); env::remove_var(TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS_ENV); env::remove_var(TRUNK_API_CLIENT_RETRY_COUNT_ENV); + env::remove_var(TRUNK_QUARANTINE_DISK_CACHE_DIR_ENV); + } +} + +pub fn setup_quarantine_disk_cache_dir(temp_dir: &TempDir) { + let cache_dir = temp_dir.path().join(constants::CACHE_DIR); + unsafe { + env::set_var(TRUNK_QUARANTINE_DISK_CACHE_DIR_ENV, cache_dir); } } #[allow(dead_code)] pub fn clean_up_cache_files() { - let cache_dir = env::temp_dir().join(constants::CACHE_DIR); + let cache_dir = quarantine_disk_cache_dir(); if let Ok(entries) = std::fs::read_dir(&cache_dir) { for entry in entries.flatten() { let _ = std::fs::remove_file(entry.path()); diff --git a/test_report/tests/common/mod.rs b/test_report/tests/common/mod.rs index e49cedf9..394ce8e7 100644 --- a/test_report/tests/common/mod.rs +++ b/test_report/tests/common/mod.rs @@ -2,4 +2,4 @@ mod env; -pub use env::{clean_up_cache_files, cleanup_env_vars}; +pub use env::{clean_up_cache_files, cleanup_env_vars, setup_quarantine_disk_cache_dir}; diff --git a/test_report/tests/publish.rs b/test_report/tests/publish.rs index 5e58ea00..c5b21980 100644 --- a/test_report/tests/publish.rs +++ b/test_report/tests/publish.rs @@ -4,7 +4,7 @@ use std::{env, fs, io::BufReader, path::Path, thread}; use assert_matches::assert_matches; use bundle::{BundleMeta, FileSetType}; -use common::cleanup_env_vars; +use common::{cleanup_env_vars, setup_quarantine_disk_cache_dir}; use constants::{ TRUNK_ALLOW_EMPTY_TEST_RESULTS_ENV, TRUNK_API_TOKEN_ENV, TRUNK_CODEOWNERS_PATH_ENV, TRUNK_DISABLE_QUARANTINING_ENV, TRUNK_DRY_RUN_ENV, TRUNK_ORG_URL_SLUG_ENV, TRUNK_PR_NUMBER_ENV, @@ -35,6 +35,7 @@ fn generate_mock_codeowners>(directory: T) { async fn publish_uploads_test_report() { cleanup_env_vars(); let temp_dir = tempdir().unwrap(); + setup_quarantine_disk_cache_dir(&temp_dir); let repo_setup_res = setup_repo_with_commit(&temp_dir); generate_mock_codeowners(&temp_dir); assert!(repo_setup_res.is_ok()); @@ -253,6 +254,7 @@ async fn publish_uploads_test_report() { fn publish_try_save() { cleanup_env_vars(); let temp_dir = tempdir().unwrap(); + setup_quarantine_disk_cache_dir(&temp_dir); let report = MutTestReport::new( "test-origin".into(), "test-command".into(), @@ -277,6 +279,7 @@ fn publish_try_save() { async fn publish_environment_variable_overrides() { cleanup_env_vars(); let temp_dir = tempdir().unwrap(); + setup_quarantine_disk_cache_dir(&temp_dir); generate_mock_codeowners(&temp_dir); // Set current directory to a safe location first (in case previous test left it in a bad state) @@ -415,6 +418,7 @@ async fn publish_environment_variable_overrides() { async fn publish_variant_priority_constructor_over_env() { cleanup_env_vars(); let temp_dir = tempdir().unwrap(); + setup_quarantine_disk_cache_dir(&temp_dir); generate_mock_codeowners(&temp_dir); // Set current directory to a safe location first (in case previous test left it in a bad state) diff --git a/test_report/tests/quarantine.rs b/test_report/tests/quarantine.rs index a955f08b..ee5f148f 100644 --- a/test_report/tests/quarantine.rs +++ b/test_report/tests/quarantine.rs @@ -5,7 +5,7 @@ use std::{env, fs, thread}; use api::message::{GetQuarantineConfigRequest, GetQuarantineConfigResponse}; use axum::{Json, extract::State, http::StatusCode}; use bundle::Test; -use common::{clean_up_cache_files, cleanup_env_vars}; +use common::{clean_up_cache_files, cleanup_env_vars, setup_quarantine_disk_cache_dir}; use constants::{ TRUNK_API_CLIENT_RETRY_COUNT_ENV, TRUNK_API_TOKEN_ENV, TRUNK_ORG_URL_SLUG_ENV, TRUNK_PUBLIC_API_ADDRESS_ENV, TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS_ENV, @@ -25,6 +25,7 @@ use test_utils::mock_server::{MockServerBuilder, RequestPayload, SharedMockServe async fn quarantine_variant_impacts_quarantining() { cleanup_env_vars(); let temp_dir = tempdir().unwrap(); + setup_quarantine_disk_cache_dir(&temp_dir); let repo_setup_res = setup_repo_with_commit(&temp_dir); assert!(repo_setup_res.is_ok()); let _ = env::set_current_dir(&temp_dir); @@ -295,9 +296,11 @@ async fn quarantine_variant_impacts_quarantining() { #[serial] async fn quarantine_disk_cache() { cleanup_env_vars(); - clean_up_cache_files(); let temp_dir = tempdir().unwrap(); + setup_quarantine_disk_cache_dir(&temp_dir); + clean_up_cache_files(); + let _ = env::set_current_dir(&temp_dir); let repo_url_1 = "https://github.com/test-org/test-repo-1.git"; @@ -534,6 +537,7 @@ async fn quarantine_disabled_for_repo() { cleanup_env_vars(); let temp_dir = tempdir().unwrap(); + setup_quarantine_disk_cache_dir(&temp_dir); let _ = env::set_current_dir(&temp_dir); let repo_url = "https://github.com/test-org/test-repo.git"; @@ -607,9 +611,11 @@ async fn quarantine_disabled_for_repo() { #[serial] async fn quarantine_lookup_failed_when_endpoint_fails() { cleanup_env_vars(); - clean_up_cache_files(); let temp_dir = tempdir().unwrap(); + setup_quarantine_disk_cache_dir(&temp_dir); + clean_up_cache_files(); + let _ = env::set_current_dir(&temp_dir); let repo_url = "https://github.com/test-org/test-repo.git"; diff --git a/test_report/tests/telemetry.rs b/test_report/tests/telemetry.rs new file mode 100644 index 00000000..d0cd6c68 --- /dev/null +++ b/test_report/tests/telemetry.rs @@ -0,0 +1,284 @@ +mod common; + +use std::{env, thread}; + +use api::message::{GetQuarantineConfigRequest, GetQuarantineConfigResponse}; +use axum::{Json, extract::State, http::StatusCode}; +use common::{clean_up_cache_files, cleanup_env_vars, setup_quarantine_disk_cache_dir}; +use constants::{ + TRUNK_API_CLIENT_RETRY_COUNT_ENV, TRUNK_API_TOKEN_ENV, TRUNK_ORG_URL_SLUG_ENV, + TRUNK_PUBLIC_API_ADDRESS_ENV, TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS_ENV, + TRUNK_REPO_HEAD_AUTHOR_NAME_ENV, TRUNK_REPO_HEAD_BRANCH_ENV, TRUNK_REPO_HEAD_SHA_ENV, + TRUNK_REPO_URL_ENV, TRUNK_USE_UNCLONED_REPO_ENV, +}; +use proto::upload_metrics::trunk::QuarantineQueryResult; +use serial_test::serial; +use tempfile::tempdir; +use test_report::report::{MutTestReport, Status}; +use test_utils::mock_git_repo::setup_repo_with_commit; +use test_utils::mock_server::{MockServerBuilder, RequestPayload, SharedMockServerState}; + +fn assert_telemetry_quarantine_query_result( + requests: &[RequestPayload], + expected: QuarantineQueryResult, +) { + let quarantine_query_result = requests + .iter() + .rev() + .find_map(|req| match req { + RequestPayload::TelemetryUploadMetrics(ur) => Some(ur.quarantine_query_result), + _ => None, + }) + .expect("expected telemetry upload metrics request"); + assert_eq!( + QuarantineQueryResult::try_from(quarantine_query_result).unwrap(), + expected + ); +} + +fn set_mock_api_env(api_host: &str) { + unsafe { + env::set_var(TRUNK_PUBLIC_API_ADDRESS_ENV, api_host); + env::set_var(TRUNK_API_TOKEN_ENV, "test-token"); + } +} + +fn set_uncloned_repo_publish_env(repo_url: &str) { + unsafe { + env::set_var(TRUNK_ORG_URL_SLUG_ENV, "test-org"); + env::set_var(TRUNK_REPO_URL_ENV, repo_url); + env::set_var(TRUNK_USE_UNCLONED_REPO_ENV, "true"); + env::set_var(TRUNK_REPO_HEAD_SHA_ENV, ""); + env::set_var(TRUNK_REPO_HEAD_BRANCH_ENV, ""); + env::set_var(TRUNK_REPO_HEAD_AUTHOR_NAME_ENV, ""); + env::set_var("CI", "1"); + env::set_var("GITHUB_JOB", "test-job"); + } +} + +fn publish_minimal_success_test(test_report: &MutTestReport) { + test_report.add_test( + Some("1".into()), + "test-name".into(), + "test-classname".into(), + "test-file".into(), + "test-parent-name".into(), + None, + Status::Success, + 0, + 1000, + 1001, + String::new(), + String::new(), + false, + ); + assert!(test_report.publish()); +} + +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn telemetry_query_result_success_on_publish() { + cleanup_env_vars(); + let temp_dir = tempdir().unwrap(); + setup_quarantine_disk_cache_dir(&temp_dir); + setup_repo_with_commit(&temp_dir).unwrap(); + env::set_current_dir(&temp_dir).unwrap(); + let state = MockServerBuilder::new().spawn_mock_server().await; + set_mock_api_env(&state.host); + unsafe { + env::set_var(TRUNK_ORG_URL_SLUG_ENV, "test-org"); + env::set_var(TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS_ENV, "0"); + } + + thread::spawn(|| { + let test_report = MutTestReport::new( + "test".into(), + "telemetry-success".into(), + Some("test-variant".into()), + ); + test_report.is_quarantined( + Some("2".into()), + Some("test-name".into()), + Some("test-parent-name".into()), + Some("test-classname".into()), + Some("test-file".into()), + ); + publish_minimal_success_test(&test_report); + }) + .join() + .unwrap(); + + let requests = state.requests.lock().unwrap().clone(); + assert_telemetry_quarantine_query_result(&requests, QuarantineQueryResult::Success); +} + +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn telemetry_query_result_skipped_without_lookup_on_publish() { + cleanup_env_vars(); + let temp_dir = tempdir().unwrap(); + setup_quarantine_disk_cache_dir(&temp_dir); + env::set_current_dir(&temp_dir).unwrap(); + + let state = MockServerBuilder::new().spawn_mock_server().await; + set_mock_api_env(&state.host); + set_uncloned_repo_publish_env("https://github.com/test-org/test-repo-skipped-telemetry.git"); + + thread::spawn(|| { + let test_report = MutTestReport::new("test".into(), "telemetry-skipped".into(), None); + publish_minimal_success_test(&test_report); + }) + .join() + .unwrap(); + + let requests = state.requests.lock().unwrap().clone(); + assert_telemetry_quarantine_query_result(&requests, QuarantineQueryResult::Skipped); +} + +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn telemetry_query_result_cached_on_publish() { + cleanup_env_vars(); + + let temp_dir = tempdir().unwrap(); + setup_quarantine_disk_cache_dir(&temp_dir); + clean_up_cache_files(); + + env::set_current_dir(&temp_dir).unwrap(); + + set_uncloned_repo_publish_env("https://github.com/test-org/test-repo-cache-telemetry.git"); + + let state = MockServerBuilder::new().spawn_mock_server().await; + set_mock_api_env(&state.host); + + thread::spawn(|| { + let warm_cache = MutTestReport::new("test".into(), "warm-cache".into(), None); + warm_cache.is_quarantined( + None, + Some("warm".into()), + Some("warm-parent".into()), + Some("Warm".into()), + Some("warm.rb".into()), + ); + + let test_report = MutTestReport::new("test".into(), "cached-publish".into(), None); + test_report.is_quarantined( + None, + Some("cached".into()), + Some("cached-parent".into()), + Some("Cached".into()), + Some("cached.rb".into()), + ); + publish_minimal_success_test(&test_report); + }) + .join() + .unwrap(); + + let requests = state.requests.lock().unwrap().clone(); + let get_quarantine_config_count = requests + .iter() + .filter(|req| matches!(req, RequestPayload::GetQuarantineConfig(_))) + .count(); + assert_eq!( + get_quarantine_config_count, 1, + "second report should use disk cache during suite" + ); + // Checks the last telemetry call + assert_telemetry_quarantine_query_result(&requests, QuarantineQueryResult::Cached); +} + +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn telemetry_query_result_disabled_on_publish() { + cleanup_env_vars(); + + let temp_dir = tempdir().unwrap(); + setup_quarantine_disk_cache_dir(&temp_dir); + env::set_current_dir(&temp_dir).unwrap(); + + set_uncloned_repo_publish_env("https://github.com/test-org/test-repo-disabled-telemetry.git"); + + let state = { + let mut builder = MockServerBuilder::new(); + builder.set_get_quarantining_config_handler( + |_state: State, _req: Json| async move { + Json(GetQuarantineConfigResponse { + is_disabled: true, + quarantined_tests: vec![], + }) + }, + ); + builder.spawn_mock_server().await + }; + + set_mock_api_env(&state.host); + unsafe { + env::set_var(TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS_ENV, "0"); + } + + thread::spawn(|| { + let test_report = MutTestReport::new("test".into(), "disabled-publish".into(), None); + test_report.is_quarantined( + None, + Some("test-name".into()), + Some("test-parent-name".into()), + Some("TestClass".into()), + Some("test_file.rs".into()), + ); + publish_minimal_success_test(&test_report); + }) + .join() + .unwrap(); + + let requests = state.requests.lock().unwrap().clone(); + assert_telemetry_quarantine_query_result(&requests, QuarantineQueryResult::Disabled); +} + +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn telemetry_query_result_failure_on_publish() { + cleanup_env_vars(); + + let temp_dir = tempdir().unwrap(); + setup_quarantine_disk_cache_dir(&temp_dir); + clean_up_cache_files(); + + env::set_current_dir(&temp_dir).unwrap(); + + set_uncloned_repo_publish_env("https://github.com/test-org/test-repo-failure-telemetry.git"); + + let state = { + let mut builder = MockServerBuilder::new(); + builder.set_get_quarantining_config_handler( + |Json(_): Json| async { + Err::, StatusCode>( + StatusCode::INTERNAL_SERVER_ERROR, + ) + }, + ); + builder.spawn_mock_server().await + }; + + set_mock_api_env(&state.host); + unsafe { + env::set_var(TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS_ENV, "0"); + env::set_var(TRUNK_API_CLIENT_RETRY_COUNT_ENV, "0"); + } + + thread::spawn(|| { + let test_report = MutTestReport::new("test".into(), "failure-publish".into(), None); + test_report.is_quarantined( + None, + Some("test-name".into()), + Some("test-parent-name".into()), + Some("TestClass".into()), + Some("test_file.rs".into()), + ); + publish_minimal_success_test(&test_report); + }) + .join() + .unwrap(); + + let requests = state.requests.lock().unwrap().clone(); + assert_telemetry_quarantine_query_result(&requests, QuarantineQueryResult::Failure); +}