[test_report] Add support for variants #844

Missed this in my original update to support the trunk cli envs. We need to make sure variant information is passed through when checking for quarantining.

Author: gnalh

Cargo.lock

@@ -35,6 +35,7 @@ bundle = { path = "../bundle" }
 more-asserts = "0.3.1"
 test_utils = { path = "../test_utils" }
 serial_test = "3.1.1"
+axum = { version = "0.7.5", features = ["macros"] }
 
 [features]
 bindings = []

test_report/Cargo.toml

@@ -267,6 +267,8 @@ impl MutTestReport {
         );
         match (api_client, bundle_repo) {
             (Ok(api_client), Ok(bundle_repo)) => {
+                let variant =
+                    env::var(constants::TRUNK_VARIANT_ENV).unwrap_or_else(|_| "".to_string());
                 let test_identifier = Test::new(
                     id.clone(),
                     name.unwrap_or_default(),
@@ -276,9 +278,14 @@ impl MutTestReport {
                     org_url_slug.clone(),
                     &bundle_repo.repo,
                     None,
-                    "".to_string(),
+                    variant.clone(),
+                );
+                self.populate_quarantined_tests(
+                    &api_client,
+                    &bundle_repo.repo,
+                    org_url_slug,
+                    variant,
                 );
-                self.populate_quarantined_tests(&api_client, &bundle_repo.repo, org_url_slug);
                 if let Some(quarantined_tests) = self.0.borrow().quarantined_tests.as_ref() {
                     return quarantined_tests.get(&test_identifier.id).is_some();
                 }
@@ -296,6 +303,7 @@ impl MutTestReport {
         api_client: &ApiClient,
         repo: &RepoUrlParts,
         org_url_slug: String,
+        variant: String,
     ) {
         if self.0.borrow().quarantined_tests.as_ref().is_some() {
             // already fetched
@@ -328,7 +336,7 @@ impl MutTestReport {
                             org_url_slug.clone(),
                             repo,
                             None,
-                            "".to_string(),
+                            variant.clone(),
                         );
 
                         quarantined_tests.insert(test.id.clone(), test);

test_report/src/report.rs

@@ -1,7 +1,8 @@
 use std::{env, fs, io::BufReader, path::Path, thread};
 
 use assert_matches::assert_matches;
-use bundle::{BundleMeta, FileSetType};
+use axum::{extract::State, Json};
+use bundle::{BundleMeta, FileSetType, Test};
 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,
@@ -18,7 +19,7 @@ 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};
+use test_utils::mock_server::{MockServerBuilder, RequestPayload, SharedMockServerState};
 
 pub fn generate_mock_codeowners<T: AsRef<Path>>(directory: T) {
     const CODEOWNERS: &str = r#"
@@ -472,3 +473,166 @@ async fn test_variant_priority_constructor_over_env() {
     // Restore original directory
     let _ = env::set_current_dir(original_dir);
 }
+
+#[tokio::test(flavor = "multi_thread")]
+#[serial]
+async fn test_variant_impacts_quarantining() {
+    cleanup_env_vars();
+    let temp_dir = tempdir().unwrap();
+    let repo_setup_res = setup_repo_with_commit(&temp_dir);
+    assert!(repo_setup_res.is_ok());
+    let _ = env::set_current_dir(&temp_dir);
+
+    // Test parameters that will be used for quarantine checking
+    let test_name = Some("test_name".to_string());
+    let test_parent_name = Some("test_parent".to_string());
+    let test_classname = Some("TestClass".to_string());
+    let test_file = Some("test_file.rs".to_string());
+
+    let repo = RepoUrlParts {
+        host: "github.com".into(),
+        owner: "trunk-io".into(),
+        name: "analytics-cli".into(),
+    };
+
+    // Generate the expected test ID with variant1
+    let expected_test_id_variant1 = Test::new(
+        None,
+        test_name.clone().unwrap_or_default(),
+        test_parent_name.clone().unwrap_or_default(),
+        test_classname.clone(),
+        test_file.clone(),
+        "test-org".to_string(),
+        &repo,
+        None,
+        "variant1".to_string(),
+    )
+    .id;
+
+    // Generate the expected test ID with variant2
+    let expected_test_id_variant2 = Test::new(
+        None,
+        test_name.clone().unwrap_or_default(),
+        test_parent_name.clone().unwrap_or_default(),
+        test_classname.clone(),
+        test_file.clone(),
+        "test-org".to_string(),
+        &repo,
+        None,
+        "variant2".to_string(),
+    )
+    .id;
+
+    // Verify they're different
+    assert_ne!(expected_test_id_variant1, expected_test_id_variant2);
+
+    // Create a custom mock server handler that returns a quarantined test with the variant1 ID
+    use api::message::{ListQuarantinedTestsResponse, Page, QuarantineSetting, QuarantinedTest};
+    let state = {
+        let mut builder = MockServerBuilder::new();
+        let expected_id_v1 = expected_test_id_variant1.clone();
+        let test_name_clone = test_name.clone();
+        let test_parent_name_clone = test_parent_name.clone();
+        let test_classname_clone = test_classname.clone();
+        let test_file_clone = test_file.clone();
+        builder.list_quarantined_tests_handler(
+            move |_state: State<SharedMockServerState>,
+                  _req: Json<api::message::ListQuarantinedTestsRequest>| async move {
+                Json(ListQuarantinedTestsResponse {
+                    quarantined_tests: vec![QuarantinedTest {
+                        test_case_id: expected_id_v1.clone(),
+                        name: test_name_clone.clone().unwrap_or_default(),
+                        parent: Some(test_parent_name_clone.clone().unwrap_or_default()),
+                        classname: test_classname_clone.clone(),
+                        file: test_file_clone.clone(),
+                        status: "failure".to_string(),
+                        quarantine_setting: QuarantineSetting::AlwaysQuarantine,
+                    }],
+                    page: Page {
+                        total_rows: 1,
+                        total_pages: 1,
+                        next_page_token: "".to_string(),
+                        prev_page_token: "".to_string(),
+                        last_page_token: "".to_string(),
+                        page_index: 0,
+                    },
+                })
+            },
+        );
+        builder.spawn_mock_server().await
+    };
+
+    env::set_var(TRUNK_PUBLIC_API_ADDRESS_ENV, &state.host);
+    env::set_var(TRUNK_API_TOKEN_ENV, "test-token");
+    env::set_var(TRUNK_ORG_URL_SLUG_ENV, "test-org");
+    env::set_var(TRUNK_USE_UNCLONED_REPO_ENV, "false");
+
+    // Test with variant1 - should find the quarantined test
+    env::set_var(TRUNK_VARIANT_ENV, "variant1");
+    let test_name_v1 = test_name.clone();
+    let test_parent_name_v1 = test_parent_name.clone();
+    let test_classname_v1 = test_classname.clone();
+    let test_file_v1 = test_file.clone();
+    let is_quarantined_v1 = thread::spawn(move || {
+        let test_report_v1 = MutTestReport::new("test".into(), "test-command".into(), None);
+        test_report_v1.is_quarantined(
+            None,
+            test_name_v1,
+            test_parent_name_v1,
+            test_classname_v1,
+            test_file_v1,
+        )
+    })
+    .join()
+    .unwrap();
+    assert!(
+        is_quarantined_v1,
+        "Test should be quarantined when variant matches"
+    );
+
+    // Test with variant2 - should NOT find the quarantined test (different variant)
+    env::set_var(TRUNK_VARIANT_ENV, "variant2");
+    let test_name_v2 = test_name.clone();
+    let test_parent_name_v2 = test_parent_name.clone();
+    let test_classname_v2 = test_classname.clone();
+    let test_file_v2 = test_file.clone();
+    let is_quarantined_v2 = thread::spawn(move || {
+        let test_report_v2 = MutTestReport::new("test".into(), "test-command".into(), None);
+        test_report_v2.is_quarantined(
+            None,
+            test_name_v2,
+            test_parent_name_v2,
+            test_classname_v2,
+            test_file_v2,
+        )
+    })
+    .join()
+    .unwrap();
+    assert!(
+        !is_quarantined_v2,
+        "Test should NOT be quarantined when variant doesn't match"
+    );
+
+    // Test with no variant - should NOT find the quarantined test
+    env::remove_var(TRUNK_VARIANT_ENV);
+    let test_name_v3 = test_name.clone();
+    let test_parent_name_v3 = test_parent_name.clone();
+    let test_classname_v3 = test_classname.clone();
+    let test_file_v3 = test_file.clone();
+    let is_quarantined_v3 = thread::spawn(move || {
+        let test_report_v3 = MutTestReport::new("test".into(), "test-command".into(), None);
+        test_report_v3.is_quarantined(
+            None,
+            test_name_v3,
+            test_parent_name_v3,
+            test_classname_v3,
+            test_file_v3,
+        )
+    })
+    .join()
+    .unwrap();
+    assert!(
+        !is_quarantined_v3,
+        "Test should NOT be quarantined when variant is empty"
+    );
+}