feat(quarantine): persist quarantine resolution mode in meta.json

api/src/message.rs

@@ -1,3 +1,4 @@
+pub use bundle::QuarantineResolutionMode;
 use bundle::Test;
 use context::repo::RepoUrlParts;
 use serde::{Deserialize, Serialize};
@@ -24,59 +25,6 @@ pub struct CreateBundleUploadResponse {
     pub test_collection_bundle_meta_created_at: Option<String>,
 }
 
-/// Which source the server resolved quarantine status from.
-#[derive(Debug, Serialize, Clone, Copy, PartialEq, Eq, Default)]
-#[serde(rename_all = "snake_case")]
-pub enum QuarantineResolutionMode {
-    Repo,
-    TestCollection,
-    #[default]
-    Unspecified,
-}
-
-impl<'de> Deserialize<'de> for QuarantineResolutionMode {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    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<QuarantineResolutionMode> for proto::upload_metrics::trunk::QuarantineResolutionMode {
-    fn from(mode: QuarantineResolutionMode) -> Self {
-        match mode {
-            QuarantineResolutionMode::Repo => Self::Repo,
-            QuarantineResolutionMode::TestCollection => Self::TestCollection,
-            QuarantineResolutionMode::Unspecified => Self::Unspecified,
-        }
-    }
-}
-
-impl QuarantineResolutionMode {
-    pub fn resolution_log_line(
-        &self,
-        test_collection_id: Option<&str>,
-        repo: &RepoUrlParts,
-    ) -> Option<String> {
-        match self {
-            Self::TestCollection => Some(format!(
-                "Resolved quarantine status for test collection {}",
-                test_collection_id.unwrap_or("unknown"),
-            )),
-            Self::Repo => Some(format!(
-                "Resolved quarantine status for repo {}",
-                repo.repo_full_name()
-            )),
-            Self::Unspecified => None,
-        }
-    }
-}
-
 #[derive(Debug, Serialize, Clone, Deserialize, Default)]
 #[serde(rename_all = "camelCase")]
 pub struct GetQuarantineConfigResponse {

bundle/src/bundle_meta.rs

@@ -20,7 +20,7 @@ use tsify_next::Tsify;
 use wasm_bindgen::prelude::*;
 
 use crate::{
-    CustomTag, Test,
+    CustomTag, QuarantineResolutionMode, Test,
     files::{BundledFile, FileSet},
 };
 
@@ -58,6 +58,8 @@ pub struct BundleMetaBaseProps {
     pub quarantined_tests: Vec<Test>,
     pub codeowners: Option<CodeOwners>,
     pub use_uncloned_repo: Option<bool>,
+    #[serde(default)]
+    pub quarantine_resolution_mode: QuarantineResolutionMode,
 }
 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)]
 #[cfg_attr(feature = "pyo3", gen_stub_pyclass, pyclass(get_all))]

bundle/src/bundler.rs

@@ -337,7 +337,7 @@ mod tests {
 
     use super::{archive_entries, parse_internal_bin_entry};
     use crate::{
-        BundledFile, Test, VersionedBundle,
+        BundledFile, QuarantineResolutionMode, Test, VersionedBundle,
         bundle_meta::{
             BundleMeta, BundleMetaBaseProps, BundleMetaDebugProps, BundleMetaJunitProps,
             META_VERSION,
@@ -428,6 +428,7 @@ mod tests {
                 codeowners: None,
                 envs,
                 use_uncloned_repo: None,
+                quarantine_resolution_mode: QuarantineResolutionMode::TestCollection,
             },
             internal_bundled_file,
             failed_tests: vec![Test::new(

bundle/src/lib.rs

@@ -1,9 +1,11 @@
 mod bundle_meta;
 mod bundler;
 mod files;
+mod quarantine_resolution_mode;
 mod types;
 
 pub use bundle_meta::*;
 pub use bundler::*;
 pub use files::*;
+pub use quarantine_resolution_mode::*;
 pub use types::*;

bundle/src/quarantine_resolution_mode.rs

@@ -0,0 +1,65 @@
+use context::repo::RepoUrlParts;
+#[cfg(feature = "pyo3")]
+use pyo3::prelude::*;
+#[cfg(feature = "pyo3")]
+use pyo3_stub_gen::derive::gen_stub_pyclass_enum;
+use serde::{Deserialize, Serialize};
+#[cfg(feature = "wasm")]
+use tsify_next::Tsify;
+
+/// Which source the server resolved quarantine status from.
+#[derive(Debug, Serialize, Clone, Copy, PartialEq, Eq, Default)]
+#[cfg_attr(feature = "pyo3", gen_stub_pyclass_enum, pyclass(eq, eq_int))]
+#[cfg_attr(feature = "wasm", derive(Tsify))]
+#[serde(rename_all = "snake_case")]
+pub enum QuarantineResolutionMode {
+    Repo,
+    TestCollection,
+    #[default]
+    Unspecified,
+}
+
+impl<'de> Deserialize<'de> for QuarantineResolutionMode {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    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<QuarantineResolutionMode> for proto::upload_metrics::trunk::QuarantineResolutionMode {
+    fn from(mode: QuarantineResolutionMode) -> Self {
+        match mode {
+            QuarantineResolutionMode::Repo => Self::Repo,
+            QuarantineResolutionMode::TestCollection => Self::TestCollection,
+            QuarantineResolutionMode::Unspecified => Self::Unspecified,
+        }
+    }
+}
+
+impl QuarantineResolutionMode {
+    pub fn resolution_log_line(
+        &self,
+        test_collection_id: Option<&str>,
+        repo: &RepoUrlParts,
+    ) -> Option<String> {
+        match self {
+            Self::TestCollection => Some(format!(
+                "Quarantine status applied from test collection {}",
+                test_collection_id.unwrap_or("unknown"),
+            )),
+            Self::Repo => Some(format!(
+                "Quarantine status applied from repo {}",
+                repo.repo_full_name()
+            )),
+            Self::Unspecified => None,
+        }
+    }
+}

bundle/tests/test_collection_props_test.rs

@@ -1,6 +1,6 @@
 use std::collections::HashMap;
 
-use bundle::{BundleMetaBaseProps, TestCollectionProps};
+use bundle::{BundleMetaBaseProps, QuarantineResolutionMode, TestCollectionProps};
 use context::repo::BundleRepo;
 use serde_json::json;
 
@@ -21,6 +21,7 @@ fn build_base_props(test_collection: Option<TestCollectionProps>) -> BundleMetaB
         quarantined_tests: vec![],
         codeowners: None,
         use_uncloned_repo: None,
+        quarantine_resolution_mode: QuarantineResolutionMode::Unspecified,
     }
 }
 

cli/src/context.rs

@@ -13,7 +13,7 @@ use api::{client::ApiClient, message::CreateBundleUploadResponse};
 use bundle::{
     BundleMeta, BundleMetaBaseProps, BundleMetaDebugProps, BundleMetaJunitProps, BundledFile,
     FileSet, FileSetBuilder, FileSetType, INTERNAL_BIN_FILENAME, META_VERSION,
-    QuarantineBulkTestStatus, TestCollectionProps, bin_parse,
+    QuarantineBulkTestStatus, QuarantineResolutionMode, TestCollectionProps, bin_parse,
 };
 use codeowners::{CodeOwners, OwnersSource};
 use constants::{ENVS_TO_GET, TRUNK_API_TOKEN_ENV, TRUNK_ENVS_TO_CAPTURE, TRUNK_PR_NUMBER_ENV};
@@ -194,6 +194,7 @@ pub fn gather_initial_test_context(
             os_info: Some(env::consts::OS.to_string()),
             codeowners: None,
             use_uncloned_repo: Some(upload_args.use_uncloned_repo),
+            quarantine_resolution_mode: QuarantineResolutionMode::Unspecified,
         },
         failed_tests: Vec::with_capacity(0),
         variant: upload_args.variant.as_ref().map(|v| {

cli/src/upload_command.rs

@@ -5,7 +5,7 @@ use std::sync::mpsc::Sender;
 
 use api::client::{ApiClient, ApiErrorEndpoint};
 use api::{client::get_api_host, urls::url_for_test_case};
-use bundle::{BundleMeta, BundlerUtil, Test, unzip_tarball};
+use bundle::{BundleMeta, BundlerUtil, QuarantineResolutionMode, Test, unzip_tarball};
 use clap::{ArgAction, Args};
 use codeowners::OwnersSource;
 use constants::EXIT_SUCCESS;
@@ -380,8 +380,7 @@ pub struct RunUploadOptions {
     pub render_sender: Option<Sender<DisplayMessage>>,
     pub quarantine_query_result_override:
         Option<proto::upload_metrics::trunk::QuarantineQueryResult>,
-    pub quarantine_resolution_mode_override:
-        Option<proto::upload_metrics::trunk::QuarantineResolutionMode>,
+    pub quarantine_resolution_mode_override: Option<QuarantineResolutionMode>,
 }
 
 impl Default for RunUploadOptions {
@@ -555,6 +554,10 @@ pub async fn run_upload(
         .quarantine_results
         .clone();
 
+    let quarantine_resolution_mode = quarantine_resolution_mode_override
+        .unwrap_or(quarantine_context.quarantine_resolution_mode);
+    meta.base_props.quarantine_resolution_mode = quarantine_resolution_mode;
+
     meta.failed_tests = quarantine_context.failures.clone();
 
     let upload_started_at = chrono::Utc::now();
@@ -570,8 +573,6 @@ 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 quarantine_resolution_mode = quarantine_resolution_mode_override
-        .unwrap_or_else(|| quarantine_context.quarantine_resolution_mode.into());
     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(),
@@ -590,7 +591,10 @@ pub async fn run_upload(
         failed: false,
         failure_reason: "".into(),
         quarantine_query_result: quarantine_query_result.into(),
-        quarantine_resolution_mode: quarantine_resolution_mode.into(),
+        quarantine_resolution_mode: proto::upload_metrics::trunk::QuarantineResolutionMode::from(
+            quarantine_resolution_mode,
+        )
+        .into(),
     };
     let mut request = api::message::TelemetryUploadMetricsRequest { upload_metrics };
     if !upload_args.dry_run {

cli/tests/upload.rs

@@ -11,7 +11,9 @@ use assert_matches::assert_matches;
 use axum::body::{Body, Bytes};
 use axum::response::IntoResponse;
 use axum::{Json, extract::State, http::StatusCode, response::Response};
-use bundle::{BundleMeta, FileSetType, INTERNAL_BIN_FILENAME, TestCollectionProps};
+use bundle::{
+    BundleMeta, FileSetType, INTERNAL_BIN_FILENAME, QuarantineResolutionMode, TestCollectionProps,
+};
 use chrono::{DateTime, TimeDelta};
 use clap::Parser;
 mod common;
@@ -211,6 +213,11 @@ async fn upload_bundle() {
     assert_eq!(base_props.test_command, None);
     assert!(base_props.os_info.is_some());
     assert!(base_props.quarantined_tests.is_empty());
+    assert_eq!(
+        base_props.quarantine_resolution_mode,
+        QuarantineResolutionMode::Unspecified,
+        "a server reporting no resolution mode must land as unspecified"
+    );
     assert_eq!(bundle_meta.failed_tests.len(), failure_count);
     assert_eq!(bundle_meta.bundle_upload_id_v2, "test-bundle-upload-id-v2");
     assert_eq!(
@@ -292,6 +299,64 @@ async fn upload_bundle() {
     ));
 }
 
+// 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);
+
+    let mut mock_server_builder = MockServerBuilder::new();
+    mock_server_builder.set_get_quarantining_config_handler(
+        |Json(_): Json<GetQuarantineConfigRequest>| async move {
+            Json(GetQuarantineConfigResponse {
+                is_disabled: false,
+                quarantined_tests: Vec::new(),
+                quarantine_resolution_mode: QuarantineResolutionMode::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()
+        // the mock JUnit reports contain unquarantined failures, so the upload itself succeeds
+        // while the command exits non-zero
+        .failure();
+
+    let requests = state.requests.lock().unwrap().clone();
+    let tar_extract_directory = requests
+        .iter()
+        .find_map(|request| match request {
+            RequestPayload::S3Upload(directory) => Some(directory),
+            _ => None,
+        })
+        .expect("the bundle must have been uploaded");
+
+    let file = fs::File::open(tar_extract_directory.join("meta.json")).unwrap();
+    let bundle_meta: BundleMeta = serde_json::from_reader(BufReader::new(file)).unwrap();
+    assert_eq!(
+        bundle_meta.base_props.quarantine_resolution_mode,
+        QuarantineResolutionMode::TestCollection,
+        "the resolution mode must be persisted in meta.json"
+    );
+
+    let upload_metrics = requests
+        .iter()
+        .find_map(|request| match request {
+            RequestPayload::TelemetryUploadMetrics(metrics) => Some(metrics),
+            _ => None,
+        })
+        .expect("telemetry must have been sent");
+    assert_eq!(
+        upload_metrics.quarantine_resolution_mode,
+        i32::from(proto::upload_metrics::trunk::QuarantineResolutionMode::TestCollection)
+    );
+}
+
 // NOTE: must be multi threaded to start a mock server
 #[tokio::test(flavor = "multi_thread")]
 async fn upload_bundle_with_public_repo_id() {

context-js/tests/parse_compressed_bundle.test.ts

@@ -109,6 +109,7 @@ const generateBundleMeta = () =>
     org: faker.company.name(),
     os_info: process.platform,
     quarantined_tests: [],
+    quarantine_resolution_mode: "unspecified",
     codeowners: {
       path: faker.system.filePath(),
     },

context-py/tests/test_parse_meta.py

@@ -305,6 +305,8 @@ def test_parse_and_dump_meta_roundtrip():
     valid_meta_str = json.dumps(valid_meta, sort_keys=True)
     expected_meta = dict(valid_meta)
     expected_meta.pop("test_collection_id")
+    # Absent from the input, so it round-trips as the default rather than being dropped.
+    expected_meta["quarantine_resolution_mode"] = "unspecified"
     expected_meta_str = json.dumps(expected_meta, sort_keys=True)
     bundle_meta = parse_meta(valid_meta_str.encode())
     assert (

test_report/src/report.rs

@@ -269,16 +269,13 @@ impl MutTestReport {
         }
     }
 
-    fn quarantine_resolution_mode_for_telemetry(
-        &self,
-    ) -> proto::upload_metrics::trunk::QuarantineResolutionMode {
+    fn quarantine_resolution_mode(&self) -> QuarantineResolutionMode {
         self.0
             .borrow()
             .quarantine_config
             .as_ref()
             .map(|config| config.quarantine_resolution_mode)
             .unwrap_or_default()
-            .into()
     }
 
     fn serialize_test_report(&self) -> Vec<u8> {
@@ -791,7 +788,7 @@ impl MutTestReport {
                                 self.quarantine_query_result_for_telemetry(),
                             ),
                             quarantine_resolution_mode_override: Some(
-                                self.quarantine_resolution_mode_for_telemetry(),
+                                self.quarantine_resolution_mode(),
                             ),
                             ..Default::default()
                         },