(Chore): Better telemetry for quarantine query by TylerJang27 · Pull Request #1100 · trunk-io/analytics-cli

TylerJang27 · 2026-05-28T20:45:19Z

Stacked on #1099. Adds missing observability for the result of querying quarantined tests.

Inputs to quarantine query status:

TylerJang27 · 2026-05-28T20:48:30Z

    }

+    #[test]
+    fn test_quarantine_query_result_mapper() {


unit test for this

TylerJang27 · 2026-05-28T20:50:22Z

    pre_test_context: Option<PreTestContext>,
    test_run_result: Option<TestRunResult>,
    render_sender: Option<Sender<DisplayMessage>>,
+    quarantine_query_result_override: Option<proto::upload_metrics::trunk::QuarantineQueryResult>,


RSpec calls the quarantine config at a separate time than the upload flow. There's a gate above this where we conditionally call GetQuarantineConfig depending on if there are JUnits (which there aren't in RSpec). That's a smell, but I didn't want to undertake that refactor right now, and it's not a meaningful increase in burden to wire in a quarantine query result override here

TylerJang27 · 2026-05-28T20:51:00Z

 }

+#[tokio::test(flavor = "multi_thread")]
+async fn telemetry_upload_metrics_quarantine_query_result_success() {


These CLI integration tests require a bit more custom setup because of the scenarios they test

TylerJang27 · 2026-05-28T20:52:53Z

+enum QuarantineQueryResult {
+  QUARANTINE_QUERY_RESULT_UNSPECIFIED = 0;
+  QUARANTINE_QUERY_RESULT_SUCCESS = 1;
+  QUARANTINE_QUERY_RESULT_FAILURE = 2;
+  QUARANTINE_QUERY_RESULT_DISABLED = 3;
+  QUARANTINE_QUERY_RESULT_SKIPPED = 4;
+  // rspec only
+  QUARANTINE_QUERY_RESULT_CACHED = 5;
+}


Unspecified: default, shouldn't be sent from new CLI versions in practice

Success: we successfully got quarantined results back

Failure: something went wrong. This could also be an auth error (403/404), but we don't differentiate rn here. Realistically we want to track both

Disabled: either disabled via the command line/env var, or via the repo setting (from our db)

Skipped: no tests actually failed, no need to look up quarantine config

Cached: rspec only, quarantined results served from file on disk

Worth making any of those points an inline comment alongside any of those?

agreed, I think those comments are nice :)

TylerJang27 · 2026-05-28T20:53:07Z

 }

+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum QuarantineLookupSource {


Special enum for the rspec/report case

TylerJang27 · 2026-05-28T20:53:41Z

"Integration" tests underlying the rspec flow. Meaningfully distinct from the CLI integration tests above

TylerJang27 · 2026-05-28T20:54:15Z

+    }
+}
+
+fn publish_minimal_success_test(test_report: &MutTestReport) {


This helper keeps things simple for most of the tests below. Note that test_report.publish() is what calls upload and the telemetry call

codecov-commenter · 2026-05-28T20:57:31Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.25%. Comparing base (8a07de4) to head (5e8f731).

Additional details and impacted files

@@                        Coverage Diff                        @@
##           tyler/reorganize-report-tests    #1100      +/-   ##
=================================================================
+ Coverage                          82.15%   82.25%   +0.09%     
=================================================================
  Files                                 69       69              
  Lines                              15039    15114      +75     
=================================================================
+ Hits                               12356    12432      +76     
+ Misses                              2683     2682       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

trunk-staging-io · 2026-05-28T20:58:58Z

Failed Test	Failure Summary	Logs
`quarantine_disk_cache`	The test failed because the quarantine configuration was called more than once for the same repository, which should not happen.	Logs ↗︎

Flaky Test	Failure Summary	Logs
`quarantine_disk_cache`		Logs ↗︎

_{View Full Report ↗︎ ⋅ Docs}

dfrankland · 2026-05-29T17:19:49Z

 pub async fn run_upload(
    upload_args: UploadArgs,
    pre_test_context: Option<PreTestContext>,
    test_run_result: Option<TestRunResult>,
    render_sender: Option<Sender<DisplayMessage>>,
+    quarantine_query_result_override: Option<proto::upload_metrics::trunk::QuarantineQueryResult>,


These arguments are getting kind of long and hard to decipher at call sites. I think we should consider creating an options struct.

dfrankland · 2026-05-29T17:22:14Z

+enum QuarantineQueryResult {
+  QUARANTINE_QUERY_RESULT_UNSPECIFIED = 0;
+  QUARANTINE_QUERY_RESULT_SUCCESS = 1;
+  QUARANTINE_QUERY_RESULT_FAILURE = 2;
+  QUARANTINE_QUERY_RESULT_DISABLED = 3;
+  QUARANTINE_QUERY_RESULT_SKIPPED = 4;
+  // rspec only
+  QUARANTINE_QUERY_RESULT_CACHED = 5;
+}


agreed, I think those comments are nice :)

dfrankland · 2026-05-29T17:28:17Z

+    assert!(repo_setup_res.is_ok());
+    assert!(env::set_current_dir(&temp_dir).is_ok());


nit, optional: I'd say to just unwrap these since it's functionality equivalent and more succinct. Same goes for any other tests.

dfrankland · 2026-05-29T17:29:44Z

+        env::set_var(TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS_ENV, "0");
+    }
+
+    thread::spawn(|| {


is there a specific reason to do this in a separate thread? same goes for the other tests

dfrankland · 2026-05-29T17:30:26Z

+async fn telemetry_query_result_skipped_without_lookup_on_publish() {
+    cleanup_env_vars();
+    let temp_dir = tempdir().unwrap();
+    let _ = env::set_current_dir(&temp_dir);


nit, optional: I'd just unwrap on this rather than drop it. If the env isn't set, this test will fail, no? same goes for the other tests

dfrankland · 2026-05-29T17:33:04Z

+    let repo_url = "https://github.com/test-org/test-repo-cache-telemetry.git";
+    set_uncloned_repo_publish_env(repo_url);


nit, optional: Some tests define repo_url others just put the string inline for the set_uncloned_repo_publish_env call. IMO, make it consistent and put this string inline

TylerJang27 mentioned this pull request May 28, 2026

(Chore): Reorganize report tests #1099

Open

TylerJang27 commented May 28, 2026

View reviewed changes

quarantine telemetry

5e8f731

TylerJang27 force-pushed the tyler/better-telemetry-quarantining branch from 40ac68a to 5e8f731 Compare May 29, 2026 02:06

TylerJang27 requested review from acatxnamedvirtue, dfrankland and max-trunk May 29, 2026 02:19

acatxnamedvirtue approved these changes May 29, 2026

View reviewed changes

dfrankland approved these changes May 29, 2026

View reviewed changes

		assert!(repo_setup_res.is_ok());
		assert!(env::set_current_dir(&temp_dir).is_ok());

		let repo_url = "https://github.com/test-org/test-repo-cache-telemetry.git";
		set_uncloned_repo_publish_env(repo_url);

Conversation

TylerJang27 commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

trunk-staging-io Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

TylerJang27 commented May 28, 2026 •

edited

Loading

codecov-commenter commented May 28, 2026 •

edited

Loading

trunk-staging-io Bot commented May 28, 2026 •

edited

Loading