From 2c9d55a167205bf59c5471db4713fab5ed07fce2 Mon Sep 17 00:00:00 2001 From: Tyler Jang Date: Tue, 16 Jun 2026 04:23:37 +0000 Subject: [PATCH] WIP --- Cargo.lock | 1 + api/src/urls.rs | 8 +- cli/Cargo.toml | 1 + cli/src/context.rs | 39 +- cli/src/lib.rs | 1 + cli/src/summary_output.rs | 429 ++++++++++++++++++ cli/src/upload_command.rs | 77 +++- cli/tests/common/command_builder.rs | 26 ++ cli/tests/upload.rs | 50 ++ constants/src/lib.rs | 2 + .../lib/trunk_spec_helper.rb | 1 + test_report/src/report.rs | 5 + 12 files changed, 612 insertions(+), 28 deletions(-) create mode 100644 cli/src/summary_output.rs diff --git a/Cargo.lock b/Cargo.lock index 2a121009..aca8da07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6153,6 +6153,7 @@ dependencies = [ "reqwest", "sentry", "sentry-tracing", + "serde", "serde_json", "superconsole", "tempfile", diff --git a/api/src/urls.rs b/api/src/urls.rs index 4f1350a1..cbf82bee 100644 --- a/api/src/urls.rs +++ b/api/src/urls.rs @@ -4,7 +4,7 @@ use url::{ParseError, Url, form_urlencoded}; pub fn url_for_test_case( public_api_address: &str, - org_url_slug: &String, + org_url_slug: &str, repo: &RepoUrlParts, test_case: &Test, ) -> Result { @@ -18,7 +18,7 @@ fn convert_to_app_url(public_api_address: &str) -> String { public_api_address.replace("https://api.", "https://app.") } -fn test_path(org_url_slug: &String, test_case: &Test) -> String { +fn test_path(org_url_slug: &str, test_case: &Test) -> String { format!("{}/flaky-tests/test/{}", org_url_slug, test_case.id) } @@ -50,8 +50,8 @@ fn test_url_generated() { }; let actual = url_for_test_case( - &String::from("https://api.trunk-staging.io"), - &String::from("bad-app-org"), + "https://api.trunk-staging.io", + "bad-app-org", &repo, &test, ); diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 9723a264..2c07d11e 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -49,6 +49,7 @@ sentry-tracing = "0.36.0" openssl = { version = "0.10.78", features = ["vendored"] } openssl-src = { version = "=300.3.1", optional = true } quick-junit = "0.5.0" +serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" clap-verbosity-flag = "3.0.2" proto = { path = "../proto" } diff --git a/cli/src/context.rs b/cli/src/context.rs index abbd4f1e..c6524ab8 100644 --- a/cli/src/context.rs +++ b/cli/src/context.rs @@ -33,6 +33,7 @@ use context::{ }; use github_actions::extract_github_external_id; use lazy_static::lazy_static; +use prost::Message; use proto::test_context::test_run::test_case_run::TestRunnerInformation; use proto::test_context::test_run::{ BazelAttemptNumber, BazelBuildInformation, BazelRunInformation, TestBuildResult, TestReport, @@ -325,10 +326,10 @@ fn write_test_report_to_file( test_results: Vec, variant: Option, temp_dir: &TempDir, -) -> anyhow::Result { +) -> anyhow::Result<(BundledFile, TestReport)> { let test_report = TestReport { uploader_metadata: Some(UploaderMetadata { - variant: variant.unwrap_or_default(), + variant: variant.clone().unwrap_or_default(), ..Default::default() }), test_results, @@ -339,14 +340,17 @@ fn write_test_report_to_file( let test_report_path = temp_dir.path().join(INTERNAL_BIN_FILENAME); std::fs::write(&test_report_path, buf)?; - Ok(BundledFile { - original_path: test_report_path.to_string_lossy().to_string(), - original_path_rel: None, - owners: vec![], - path: INTERNAL_BIN_FILENAME.to_string(), - // last_modified_epoch_ns does not serialize so the compiler complains it does not exist - ..Default::default() - }) + Ok(( + BundledFile { + original_path: test_report_path.to_string_lossy().to_string(), + original_path_rel: None, + owners: vec![], + path: INTERNAL_BIN_FILENAME.to_string(), + // last_modified_epoch_ns does not serialize so the compiler complains it does not exist + ..Default::default() + }, + test_report, + )) } struct BuildResultFromBep { @@ -392,6 +396,7 @@ pub fn generate_internal_file_from_bep( use_bazel_target_for_codeowners: bool, ) -> anyhow::Result<( BundledFile, + TestReport, BTreeMap>, )> { let mut junit_validations = BTreeMap::new(); @@ -498,9 +503,9 @@ pub fn generate_internal_file_from_bep( )); } - let bundled_file = write_test_report_to_file(test_results, variant, temp_dir)?; + let (bundled_file, test_report) = write_test_report_to_file(test_results, variant, temp_dir)?; - Ok((bundled_file, junit_validations)) + Ok((bundled_file, test_report, junit_validations)) } pub fn generate_internal_file( @@ -515,6 +520,7 @@ pub fn generate_internal_file( use_bazel_target_for_codeowners: bool, ) -> anyhow::Result<( BundledFile, + Option, BTreeMap>, )> { let mut junit_validations = BTreeMap::new(); @@ -530,8 +536,9 @@ pub fn generate_internal_file( "Internal file set contains more than one file" )); } - // Internal file set, we should just use that directly and assume it's valid - return Ok((file_set.files[0].clone(), BTreeMap::new())); + // Internal file sets are pre-encoded proto files; pass through as-is. + let bundled_file = file_set.files[0].clone(); + return Ok((bundled_file, None, BTreeMap::new())); } else { for file in &file_set.files { if file.original_path.ends_with(".xml") { @@ -574,9 +581,9 @@ pub fn generate_internal_file( None, // No build information for non-BEP files )]; - let bundled_file = write_test_report_to_file(test_results, variant, temp_dir)?; + let (bundled_file, test_report) = write_test_report_to_file(test_results, variant, temp_dir)?; - Ok((bundled_file, junit_validations)) + Ok((bundled_file, Some(test_report), junit_validations)) } pub fn fall_back_to_binary_parse( diff --git a/cli/src/lib.rs b/cli/src/lib.rs index 0c7bd3ba..9a9f05c9 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -3,6 +3,7 @@ pub mod context_quarantine; pub mod error_report; pub mod print; mod report_limiting; +pub mod summary_output; pub mod test_command; pub mod upload_command; pub mod validate_command; diff --git a/cli/src/summary_output.rs b/cli/src/summary_output.rs new file mode 100644 index 00000000..f5a7447c --- /dev/null +++ b/cli/src/summary_output.rs @@ -0,0 +1,429 @@ +use std::path::Path; + +use api::{client::get_api_host, urls::url_for_test_case}; +use bundle::Test; +use context::{meta::id::gen_info_id, repo::RepoUrlParts}; +use proto::test_context::test_run::{ + TestCaseRun, TestCaseRunStatus, TestReport, + test_case_run::TestRunnerInformation, +}; +use serde::{Deserialize, Serialize}; + +use crate::upload_command::UploadRunResult; + +const SCHEMA_VERSION: u32 = 1; + +#[derive(Debug, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct SummaryReport { + pub schema_version: u32, + pub summary: SummaryCounts, + pub failures: Vec, +} + +#[derive(Debug, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct SummaryCounts { + pub total: usize, + pub pass: usize, + pub fail: usize, + pub quarantined: usize, + #[serde(skip_serializing_if = "Option::is_none")] + pub pass_ratio: Option, +} + +#[derive(Debug, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct SummaryFailure { + #[serde(skip_serializing_if = "Option::is_none")] + pub file: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub line_number: Option, + pub name: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub class_name: Option, + pub suite_name: String, + pub trunk_url: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub duration: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub failure_message: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub bazel_target: Option, + pub is_quarantined: bool, +} + +pub fn build_summary_report( + result: &UploadRunResult, + test_report_override: Option<&TestReport>, +) -> anyhow::Result { + let org_url_slug = &result.quarantine_context.org_url_slug; + let repo = &result.quarantine_context.repo; + let variant = result.meta.variant.clone().unwrap_or_default(); + + let mut failures = match test_report_override.or(result.test_report.as_ref()) { + Some(test_report) => { + failures_from_test_report(test_report, org_url_slug, repo, &variant)? + } + None => { + tracing::warn!("No test report available for summary output; writing empty failures list"); + Vec::new() + } + }; + failures.sort_by_key(|failure| failure.is_quarantined); + + let (failure_count, quarantined) = failures.iter().fold((0, 0), |(fail, quar), failure| { + if failure.is_quarantined { + (fail, quar + 1) + } else { + (fail + 1, quar) + } + }); + let total_tests = result.meta.junit_props.num_tests; + let passes = total_tests.saturating_sub(failure_count + quarantined); + + Ok(SummaryReport { + schema_version: SCHEMA_VERSION, + summary: SummaryCounts { + total: total_tests, + pass: passes, + fail: failure_count, + quarantined, + pass_ratio: pass_ratio(total_tests, passes), + }, + failures, + }) +} + +fn failures_from_test_report( + test_report: &TestReport, + org_url_slug: &String, + repo: &RepoUrlParts, + variant: &str, +) -> anyhow::Result> { + test_report + .test_results + .iter() + .flat_map(|test_result| test_result.test_case_runs.iter()) + .filter(|case_run| is_failure_run(case_run)) + .map(|case_run| summary_failure_from_case_run(case_run, org_url_slug, repo, variant)) + .collect() +} + +fn pass_ratio(total_tests: usize, passes: usize) -> Option { + if total_tests == 0 { + None + } else { + Some(((passes as f64 / total_tests as f64) * 100.0).round() / 100.0) + } +} + +fn is_failure_run(case_run: &TestCaseRun) -> bool { + TestCaseRunStatus::try_from(case_run.status).ok() == Some(TestCaseRunStatus::Failure) +} + +fn summary_failure_from_case_run( + case_run: &TestCaseRun, + org_url_slug: &String, + repo: &RepoUrlParts, + variant: &str, +) -> anyhow::Result { + let test = test_from_case_run(case_run, org_url_slug, repo, variant); + + Ok(SummaryFailure { + file: non_empty_string(&case_run.file), + line_number: line_number_from_run(case_run), + name: case_run.name.clone(), + class_name: non_empty_string(&case_run.classname), + suite_name: case_run.parent_name.clone(), + trunk_url: url_for_test_case(&get_api_host(), org_url_slug, repo, &test)?, + duration: duration_from_run(case_run), + failure_message: failure_message_from_run(case_run), + bazel_target: bazel_target_from_run(case_run), + is_quarantined: case_run.is_quarantined, + }) +} + +fn test_from_case_run( + case_run: &TestCaseRun, + org_slug: &String, + repo: &RepoUrlParts, + variant: &str, +) -> Test { + Test { + name: case_run.name.clone(), + parent_name: case_run.parent_name.clone(), + class_name: non_empty_string(&case_run.classname), + file: non_empty_string(&case_run.file), + id: gen_info_id( + org_slug.as_str(), + repo.repo_full_name().as_str(), + Some(case_run.file.as_str()), + (!case_run.classname.is_empty()).then_some(case_run.classname.as_str()), + Some(case_run.parent_name.as_str()), + Some(case_run.name.as_str()), + (!case_run.id.is_empty()).then_some(case_run.id.as_str()), + variant, + ), + timestamp_millis: None, + is_quarantined: case_run.is_quarantined, + failure_message: failure_message_from_run(case_run), + variant: Some(variant.to_string()), + } +} + +fn non_empty_string(value: &str) -> Option { + if value.is_empty() { + None + } else { + Some(value.to_string()) + } +} + +fn line_number_from_run(case_run: &TestCaseRun) -> Option { + case_run + .line_number + .as_ref() + .map(|line| line.number) + .or_else(|| (case_run.line != 0).then_some(case_run.line)) +} + +fn duration_from_run(case_run: &TestCaseRun) -> Option { + let started_at = case_run.started_at.as_ref()?; + let finished_at = case_run.finished_at.as_ref()?; + let started_at = chrono::DateTime::from(started_at.clone()); + let finished_at = chrono::DateTime::from(finished_at.clone()); + Some((finished_at - started_at).num_milliseconds() as f64 / 1000.0) +} + +fn failure_message_from_run(case_run: &TestCaseRun) -> Option { + case_run + .test_output + .as_ref() + .and_then(|output| { + non_empty_string(&output.text).or_else(|| non_empty_string(&output.message)) + }) + // trunk-ignore(clippy/deprecated) + .or_else(|| non_empty_string(&case_run.status_output_message)) +} + +fn bazel_target_from_run(case_run: &TestCaseRun) -> Option { + match &case_run.test_runner_information { + Some(TestRunnerInformation::BazelRunInformation(info)) => { + non_empty_string(&info.label) + } + _ => None, + } +} + +pub fn write_summary_output_file( + path: &Path, + result: &UploadRunResult, + test_report_override: Option<&TestReport>, +) -> anyhow::Result<()> { + let report = build_summary_report(result, test_report_override)?; + let json = serde_json::to_string_pretty(&report)?; + let parent = path.parent().filter(|p| !p.as_os_str().is_empty()); + if let Some(parent) = parent { + std::fs::create_dir_all(parent)?; + } + let temp_path = path.with_extension("tmp"); + std::fs::write(&temp_path, json)?; + std::fs::rename(&temp_path, path)?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use bundle::{ + BundleMetaBaseProps, BundleMetaDebugProps, BundleMetaJunitProps, BundleMetaV0_7_8, + }; + use context::repo::{BundleRepo, RepoUrlParts}; + use proto::test_context::test_run::{TestOutput, TestResult}; + + use super::*; + use crate::context_quarantine::QuarantineContext; + + fn sample_case_run(name: &str, parent: &str, is_quarantined: bool) -> TestCaseRun { + TestCaseRun { + name: name.to_string(), + parent_name: parent.to_string(), + classname: "MyClass".to_string(), + file: "src/foo_test.rs".to_string(), + status: TestCaseRunStatus::Failure.into(), + is_quarantined, + test_output: Some(TestOutput { + message: "assertion failed".to_string(), + ..Default::default() + }), + ..Default::default() + } + } + + fn expected_summary_failure_from_case_run( + case_run: &TestCaseRun, + org_url_slug: &str, + repo: &RepoUrlParts, + variant: &str, + ) -> SummaryFailure { + let org_url_slug_string = org_url_slug.to_string(); + let test = test_from_case_run(case_run, &org_url_slug_string, repo, variant); + SummaryFailure { + file: non_empty_string(&case_run.file), + line_number: line_number_from_run(case_run), + name: case_run.name.clone(), + class_name: non_empty_string(&case_run.classname), + suite_name: case_run.parent_name.clone(), + trunk_url: url_for_test_case(&get_api_host(), org_url_slug, repo, &test).unwrap(), + duration: duration_from_run(case_run), + failure_message: failure_message_from_run(case_run), + bazel_target: bazel_target_from_run(case_run), + is_quarantined: case_run.is_quarantined, + } + } + + fn sample_upload_result(test_report: TestReport, repo: RepoUrlParts) -> UploadRunResult { + let meta = BundleMetaV0_7_8 { + base_props: BundleMetaBaseProps { + version: String::new(), + cli_version: String::new(), + org: "test-org".to_string(), + test_collection: None, + repo: BundleRepo::default(), + bundle_upload_id: "upload-id".to_string(), + tags: vec![], + file_sets: vec![], + envs: std::collections::HashMap::new(), + upload_time_epoch: 0, + test_command: None, + os_info: None, + quarantined_tests: vec![], + codeowners: None, + use_uncloned_repo: None, + }, + junit_props: BundleMetaJunitProps { + num_tests: 10, + ..Default::default() + }, + debug_props: BundleMetaDebugProps { + command_line: String::new(), + trunk_envs: std::collections::HashMap::new(), + }, + bundle_upload_id_v2: "upload-id-v2".to_string(), + variant: None, + internal_bundled_file: None, + failed_tests: vec![], + }; + + let mut qc = QuarantineContext::skip_fetch(vec![]); + qc.org_url_slug = "test-org".to_string(); + qc.repo = repo; + + UploadRunResult { + error_report: None, + quarantine_context: qc, + meta, + test_report: Some(test_report), + validations: crate::validate_command::JunitReportValidations::new( + std::collections::BTreeMap::new(), + ), + validation_report: crate::report_limiting::ValidationReport::Limited, + show_failure_messages: false, + summary_output_file: None, + summary_output_written: false, + } + } + + #[test] + fn summary_counts_match_terminal_math() { + let repo = RepoUrlParts { + host: "github.com".to_string(), + owner: "trunk-io".to_string(), + name: "analytics-cli".to_string(), + }; + let fails = sample_case_run("fails", "suite", false); + let quarantined = sample_case_run("quarantined", "suite", true); + let test_report = TestReport { + test_results: vec![TestResult { + test_case_runs: vec![fails.clone(), quarantined.clone()], + ..Default::default() + }], + uploader_metadata: None, + }; + + let result = sample_upload_result(test_report, repo.clone()); + let report = build_summary_report(&result, None).unwrap(); + + assert!(fails.id.is_empty()); + assert!(quarantined.id.is_empty()); + assert_eq!( + report, + SummaryReport { + schema_version: 1, + summary: SummaryCounts { + total: 10, + pass: 8, + fail: 1, + quarantined: 1, + pass_ratio: Some(0.8), + }, + failures: vec![ + expected_summary_failure_from_case_run(&fails, "test-org", &repo, ""), + expected_summary_failure_from_case_run(&quarantined, "test-org", &repo, ""), + ], + } + ); + } + + #[test] + fn summary_url_uses_uuid_for_trunk_prefixed_junit_id() { + use context::meta::id::gen_info_id; + + let repo = RepoUrlParts { + host: "github.com".to_string(), + owner: "trunk-io".to_string(), + name: "analytics-cli".to_string(), + }; + let mut case_run = sample_case_run("fails", "suite", false); + case_run.id = "trunk:custom-id".to_string(); + + let test_report = TestReport { + test_results: vec![TestResult { + test_case_runs: vec![case_run.clone()], + ..Default::default() + }], + uploader_metadata: None, + }; + + let result = sample_upload_result(test_report, repo.clone()); + let report = build_summary_report(&result, None).unwrap(); + + let expected_uuid = gen_info_id( + "test-org", + repo.repo_full_name().as_str(), + Some("src/foo_test.rs"), + Some("MyClass"), + Some("suite"), + Some("fails"), + Some("trunk:custom-id"), + "", + ); + + assert_eq!(report.failures.len(), 1); + assert!( + report.failures[0].trunk_url.contains(&expected_uuid), + "expected uuid {} in url, got {}", + expected_uuid, + report.failures[0].trunk_url + ); + assert!( + !report.failures[0].trunk_url.contains("trunk:custom-id"), + "url should not contain raw junit id: {}", + report.failures[0].trunk_url + ); + assert_eq!( + report.failures[0], + expected_summary_failure_from_case_run(&case_run, "test-org", &repo, "") + ); + } +} diff --git a/cli/src/upload_command.rs b/cli/src/upload_command.rs index 4b223c69..689437ff 100644 --- a/cli/src/upload_command.rs +++ b/cli/src/upload_command.rs @@ -19,6 +19,7 @@ use superconsole::{ use tempfile::TempDir; use crate::context_quarantine::{QuarantineContext, quarantine_query_result}; +use crate::summary_output::write_summary_output_file; use crate::validate_command::JunitReportValidations; use crate::{ context::{ @@ -292,6 +293,14 @@ pub struct UploadArgs { default_missing_value = "true" )] pub show_failure_messages: bool, + #[arg( + long, + env = constants::TRUNK_SUMMARY_OUTPUT_FILE_ENV, + help = "Write a JSON summary of test results to the given file path. Useful for CI annotation workflows.", + required = false, + num_args = 1 + )] + pub summary_output_file: Option, } #[derive(clap::ValueEnum, Clone, Copy, Debug, Default, PartialEq, Eq)] @@ -369,24 +378,29 @@ pub struct UploadRunResult { pub error_report: Option, pub quarantine_context: QuarantineContext, pub meta: BundleMeta, + pub test_report: Option, pub validations: JunitReportValidations, pub validation_report: ValidationReport, pub show_failure_messages: bool, + pub summary_output_file: Option, + pub summary_output_written: bool, } -pub struct RunUploadOptions { +pub struct RunUploadOptions<'a> { pub pre_test_context: Option, pub test_run_result: Option, + pub test_report_override: Option<&'a proto::test_context::test_run::TestReport>, pub render_sender: Option>, pub quarantine_query_result_override: Option, } -impl Default for RunUploadOptions { +impl Default for RunUploadOptions<'_> { fn default() -> Self { Self { pre_test_context: None, test_run_result: None, + test_report_override: None, render_sender: None, quarantine_query_result_override: None, } @@ -395,11 +409,12 @@ impl Default for RunUploadOptions { pub async fn run_upload( upload_args: UploadArgs, - options: RunUploadOptions, + options: RunUploadOptions<'_>, ) -> anyhow::Result { let RunUploadOptions { pre_test_context, test_run_result, + test_report_override, render_sender, quarantine_query_result_override, } = options; @@ -518,6 +533,9 @@ pub async fn run_upload( &quarantined_test_ids, upload_args.use_bazel_target_for_codeowners, ) + .map(|(bundled_file, test_report, junit_validations)| { + (bundled_file, Some(test_report), junit_validations) + }) } else { generate_internal_file( &meta.base_props.file_sets, @@ -535,12 +553,16 @@ pub async fn run_upload( upload_args.use_bazel_target_for_codeowners, ) }; - let validations = if let Ok((internal_bundled_file, junit_validations)) = internal_bundled_file + let (validations, test_report) = if let Ok((internal_bundled_file, test_report, junit_validations)) = + internal_bundled_file { meta.internal_bundled_file = Some(internal_bundled_file); - JunitReportValidations::new(junit_validations) + ( + JunitReportValidations::new(junit_validations), + test_report, + ) } else { - JunitReportValidations::new(BTreeMap::new()) + (JunitReportValidations::new(BTreeMap::new()), None) }; meta.base_props.quarantined_tests = quarantine_context @@ -620,14 +642,37 @@ pub async fn run_upload( Some("There was an unexpected error that occurred while uploading test results".into()), )), }; - Ok(UploadRunResult { + + let mut run_result = UploadRunResult { quarantine_context, error_report, meta, + test_report, validations, validation_report: upload_args.validation_report, show_failure_messages: upload_args.show_failure_messages, - }) + summary_output_file: upload_args.summary_output_file.clone(), + summary_output_written: false, + }; + + if let Some(path) = &upload_args.summary_output_file { + match write_summary_output_file(path, &run_result, test_report_override) { + Ok(()) => { + run_result.summary_output_written = true; + tracing::info!("Summary written to {}", path.display()); + } + Err(err) => { + tracing::error!( + hidden_in_console = true, + "Failed to write summary output file to {}: {:?}", + path.display(), + err + ); + } + } + } + + Ok(run_result) } async fn upload_bundle( @@ -980,6 +1025,22 @@ impl EndOutput for UploadRunResult { )?, ])); } + + if self.summary_output_written { + if let Some(path) = &self.summary_output_file { + output.push(Line::default()); + output.push(Line::from_iter([Span::new_unstyled(format!( + "Summary written to {}", + path.display() + ))?])); + } + } else if self.summary_output_file.is_some() { + output.push(Line::default()); + output.push(Line::from_iter([Span::new_styled( + style("Failed to write summary output file".to_string()).with(Color::Yellow), + )?])); + } + Ok(output) } } diff --git a/cli/tests/common/command_builder.rs b/cli/tests/common/command_builder.rs index f8cbb955..4ebe6b3a 100644 --- a/cli/tests/common/command_builder.rs +++ b/cli/tests/common/command_builder.rs @@ -27,6 +27,7 @@ pub struct UploadArgs { validation_report: Option, dry_run: bool, public_repo_id: Option, + summary_output_file: Option, } impl UploadArgs { @@ -53,6 +54,7 @@ impl UploadArgs { validation_report: None, dry_run: false, public_repo_id: None, + summary_output_file: None, } } @@ -219,6 +221,17 @@ impl UploadArgs { vec![String::from("--validation-report"), validation_report] }, )) + .chain( + self.summary_output_file + .clone() + .into_iter() + .flat_map(|summary_output_file: String| { + vec![ + String::from("--summary-output-file"), + summary_output_file, + ] + }), + ) .chain(if self.dry_run { vec![String::from("--dry-run")] } else { @@ -563,6 +576,19 @@ impl<'b> CommandBuilder<'b> { self } + pub fn summary_output_file(&mut self, new_value: &str) -> &mut Self { + match &mut self.command_type { + CommandType::Upload { upload_args, .. } => { + upload_args.summary_output_file = Some(new_value.to_string()); + } + CommandType::Test { upload_args, .. } => { + upload_args.summary_output_file = Some(new_value.to_string()); + } + CommandType::Validate { .. } => {} + } + self + } + pub fn repo_url(&mut self, new_value: &str) -> &mut Self { self.command_type.repo_url(new_value); self diff --git a/cli/tests/upload.rs b/cli/tests/upload.rs index 7aac5291..58741722 100644 --- a/cli/tests/upload.rs +++ b/cli/tests/upload.rs @@ -42,6 +42,7 @@ use test_utils::{ inputs::get_test_file_path, mock_server::{MockServerBuilder, RequestPayload, SharedMockServerState}, }; +use trunk_analytics_cli::summary_output::{SummaryCounts, SummaryReport}; use trunk_analytics_cli::upload_command::{DRY_RUN_OUTPUT_DIR, get_bundle_upload_id_message}; // NOTE: must be multi threaded to start a mock server @@ -260,6 +261,55 @@ async fn upload_bundle() { )); } +// NOTE: must be multi threaded to start a mock server +#[tokio::test(flavor = "multi_thread")] +async fn upload_writes_summary_output_file() { + 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; + let summary_path = temp_dir.path().join("summary.json"); + + CommandBuilder::upload(temp_dir.path(), state.host.clone()) + .summary_output_file(summary_path.to_str().unwrap()) + .command() + .assert() + .failure(); + + let report: SummaryReport = + serde_json::from_reader(fs::File::open(&summary_path).unwrap()).unwrap(); + + assert_eq!(report.schema_version, 1); + assert!(report.summary.total > 0); + assert_eq!( + report.summary, + SummaryCounts { + total: report.summary.total, + pass: report.summary.pass, + fail: report.summary.fail, + quarantined: report.summary.quarantined, + pass_ratio: Some( + ((report.summary.pass as f64 / report.summary.total as f64) * 100.0).round() + / 100.0, + ), + } + ); + assert_eq!( + report.failures.len(), + report.summary.fail + report.summary.quarantined + ); + for failure in &report.failures { + assert!(!failure.name.is_empty()); + assert!(failure.trunk_url.contains("flaky-tests/test/")); + assert!(!failure.is_quarantined); + } + + let round_trip: SummaryReport = + serde_json::from_str(&serde_json::to_string(&report).unwrap()).unwrap(); + assert_eq!(report, round_trip); +} + // NOTE: must be multi threaded to start a mock server #[tokio::test(flavor = "multi_thread")] async fn upload_bundle_with_public_repo_id() { diff --git a/constants/src/lib.rs b/constants/src/lib.rs index 781b0645..44f781f3 100644 --- a/constants/src/lib.rs +++ b/constants/src/lib.rs @@ -50,6 +50,7 @@ pub const TRUNK_DRY_RUN_ENV: &str = "TRUNK_DRY_RUN"; pub const TRUNK_TEST_PROCESS_EXIT_CODE_ENV: &str = "TRUNK_TEST_PROCESS_EXIT_CODE"; 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_SUMMARY_OUTPUT_FILE_ENV: &str = "TRUNK_SUMMARY_OUTPUT_FILE"; pub const TRUNK_DEBUG_ENV: &str = "TRUNK_DEBUG"; // 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. @@ -83,6 +84,7 @@ pub const TRUNK_ENVS_TO_CAPTURE: &[&str] = &[ TRUNK_TEST_PROCESS_EXIT_CODE_ENV, TRUNK_VALIDATION_REPORT_ENV, TRUNK_SHOW_FAILURE_MESSAGES_ENV, + TRUNK_SUMMARY_OUTPUT_FILE_ENV, TRUNK_DEBUG_ENV, ]; diff --git a/rspec-trunk-flaky-tests/lib/trunk_spec_helper.rb b/rspec-trunk-flaky-tests/lib/trunk_spec_helper.rb index 1b6f5a89..7dba46d7 100644 --- a/rspec-trunk-flaky-tests/lib/trunk_spec_helper.rb +++ b/rspec-trunk-flaky-tests/lib/trunk_spec_helper.rb @@ -24,6 +24,7 @@ # TRUNK_DISABLE_QUARANTINING - Set to 'true' to disable quarantining # TRUNK_ALLOW_EMPTY_TEST_RESULTS - Set to 'true' to allow empty results # TRUNK_DRY_RUN - Set to 'true' to save bundle locally instead of uploading +# TRUNK_SUMMARY_OUTPUT_FILE - Path to write a JSON summary of test results for CI annotation # TRUNK_USE_UNCLONED_REPO - Set to 'true' for uncloned repo mode # TRUNK_LOCAL_UPLOAD_DIR - Directory to save test results locally (disables upload) # TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS - Time to cache quarantined tests on disk (in seconds) diff --git a/test_report/src/report.rs b/test_report/src/report.rs index 81efae5c..e5c1e109 100644 --- a/test_report/src/report.rs +++ b/test_report/src/report.rs @@ -696,6 +696,9 @@ impl MutTestReport { upload_args.pr_number = env::var(constants::TRUNK_PR_NUMBER_ENV) .ok() .and_then(|v| v.parse::().ok()); + upload_args.summary_output_file = env::var(constants::TRUNK_SUMMARY_OUTPUT_FILE_ENV) + .ok() + .map(PathBuf::from); let debug_props = BundleMetaDebugProps { command_line: self.0.borrow().command.clone(), trunk_envs: HashMap::new(), @@ -709,6 +712,7 @@ impl MutTestReport { }; let result = match gather_initial_test_context(upload_args.clone(), debug_props) { Ok(pre_test_context) => { + let inner = self.0.borrow(); match tokio::runtime::Builder::new_multi_thread() .enable_all() .build() @@ -718,6 +722,7 @@ impl MutTestReport { RunUploadOptions { pre_test_context: Some(pre_test_context), test_run_result: Some(test_run_result), + test_report_override: Some(&inner.test_report), quarantine_query_result_override: Some( self.quarantine_query_result_for_telemetry(), ),