Skip to content

Commit b08a9b4

Browse files
committed
cache getQuarantineConfig on disk with short TTL for MutTestReport
1 parent 1cbbb03 commit b08a9b4

File tree

7 files changed

+347
-8
lines changed

7 files changed

+347
-8
lines changed

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

constants/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ pub const EXIT_FAILURE: i32 = 1;
1919
pub const CODEOWNERS_LOCATIONS: &[&str] = &[".github", ".bitbucket", ".", "docs", ".gitlab"];
2020

2121
pub const DEFAULT_ORIGIN: &str = "https://api.trunk.io";
22+
pub const DEFAULT_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS: u64 = 5 * 60; // 5 minutes
23+
pub const CACHE_DIR: &str = "trunk-flaky-tests";
2224
pub const TRUNK_PUBLIC_API_ADDRESS_ENV: &str = "TRUNK_PUBLIC_API_ADDRESS";
2325
pub const TRUNK_API_CLIENT_RETRY_COUNT_ENV: &str = "TRUNK_API_CLIENT_RETRY_COUNT";
2426

@@ -31,6 +33,8 @@ pub const TRUNK_REPO_HEAD_SHA_ENV: &str = "TRUNK_REPO_HEAD_SHA";
3133
pub const TRUNK_REPO_HEAD_BRANCH_ENV: &str = "TRUNK_REPO_HEAD_BRANCH";
3234
pub const TRUNK_REPO_HEAD_COMMIT_EPOCH_ENV: &str = "TRUNK_REPO_HEAD_COMMIT_EPOCH";
3335
pub const TRUNK_REPO_HEAD_AUTHOR_NAME_ENV: &str = "TRUNK_REPO_HEAD_AUTHOR_NAME";
36+
pub const TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS_ENV: &str =
37+
"TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS";
3438
pub const TRUNK_CODEOWNERS_PATH_ENV: &str = "TRUNK_CODEOWNERS_PATH";
3539
pub const TRUNK_VARIANT_ENV: &str = "TRUNK_VARIANT";
3640
pub const TRUNK_USE_UNCLONED_REPO_ENV: &str = "TRUNK_USE_UNCLONED_REPO";

rspec-trunk-flaky-tests/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ Understanding the quarantining flow helps you know what to expect when using thi
146146

147147
- Tests run one by one as normal.
148148
- When a test **fails**, the gem intercepts the failure through RSpec's `set_exception` hook.
149-
- **On the first failure**, the gem makes an API call to fetch the list of quarantined tests from Trunk servers. This list is then **cached in memory** for the remainder of the test run to minimize API calls.
149+
- **On the first failure**, the gem makes an API call to fetch the list of quarantined tests from Trunk servers. This list is then **cached in memory** for the remainder of the test run to minimize API calls. The list is also cached on disk with a TTL, configurable via the `TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS` environment variable (default 300s = 5m).
150150
- For each subsequent failure, the gem checks the cached quarantine list (no additional API calls).
151151

152152
3. **Quarantine Check and Exception Override**

rspec-trunk-flaky-tests/lib/trunk_spec_helper.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
# TRUNK_DRY_RUN - Set to 'true' to save bundle locally instead of uploading
2626
# TRUNK_USE_UNCLONED_REPO - Set to 'true' for uncloned repo mode
2727
# TRUNK_LOCAL_UPLOAD_DIR - Directory to save test results locally (disables upload)
28+
# TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS - Time to cache quarantined tests on disk (in seconds)
2829
# DISABLE_RSPEC_TRUNK_FLAKY_TESTS - Set to 'true' to completely disable Trunk
2930
#
3031
require 'rspec/core'
@@ -59,7 +60,7 @@ def trunk_disabled
5960
ENV['DISABLE_RSPEC_TRUNK_FLAKY_TESTS'] == 'true' || ENV['TRUNK_ORG_URL_SLUG'].nil? || ENV['TRUNK_API_TOKEN'].nil?
6061
end
6162

62-
# we want to cache the test report so we can add to it as we go and reduce the number of API calls
63+
# we want to cache the test report in memory so we can add to it as we go and reduce the number of API calls
6364
$test_report = TestReport.new('rspec', "#{$PROGRAM_NAME} #{ARGV.join(' ')}", nil)
6465

6566
module RSpec

test_report/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ prost-wkt-types = { version = "0.5.1", features = ["vendored-protox"] }
1616
prost = "0.12.6"
1717
tempfile = "3.2.0"
1818
chrono = "0.4.33"
19+
serde = { version = "1.0", features = ["derive"] }
1920
serde_json = "1.0.133"
2021
js-sys = { version = "0.3.70", optional = true }
2122
tokio = "1.43.1"
@@ -28,6 +29,7 @@ api = { version = "0.1.0", path = "../api" }
2829
context = { version = "0.1.0", path = "../context" }
2930
codeowners = { version = "0.1.3", path = "../codeowners" }
3031
constants = { path = "../constants" }
32+
uuid = { version = "1.10.0", features = ["v5"] }
3133

3234
[dev-dependencies]
3335
assert_matches = "1.5.0"

test_report/src/report.rs

Lines changed: 132 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::{
33
collections::HashMap,
44
env, fs,
55
path::{Path, PathBuf},
6-
time::SystemTime,
6+
time::{Duration, SystemTime},
77
};
88

99
use api::{client::ApiClient, message};
@@ -19,18 +19,27 @@ use proto::test_context::test_run::{
1919
CodeOwner, TestCaseRun, TestCaseRunStatus, TestReport as TestReportProto, TestResult,
2020
UploaderMetadata,
2121
};
22+
use serde::{Deserialize, Serialize};
2223
use third_party::sentry;
2324
use tracing_subscriber::{filter::FilterFn, prelude::*};
2425
use trunk_analytics_cli::{context::gather_initial_test_context, upload_command::run_upload};
26+
use uuid::Uuid;
2527
#[cfg(feature = "wasm")]
2628
use wasm_bindgen::prelude::wasm_bindgen;
2729

30+
#[derive(Debug, Clone, Serialize, Deserialize)]
31+
struct QuarantinedTestsDiskCacheEntry {
32+
quarantined_tests: HashMap<String, bool>,
33+
cached_at_secs: u64,
34+
}
35+
2836
#[derive(Debug, Clone, PartialEq)]
2937
pub struct TestReport {
3038
test_report: TestReportProto,
3139
command: String,
3240
started_at: SystemTime,
3341
quarantined_tests: Option<HashMap<String, bool>>,
42+
quarantined_tests_disk_cache_ttl: Duration,
3443
codeowners: Option<CodeOwners>,
3544
variant: Option<String>,
3645
repo: Option<BundleRepo>,
@@ -143,14 +152,21 @@ impl MutTestReport {
143152
.as_deref(),
144153
)
145154
});
155+
let quarantined_tests_disk_cache_ttl = Duration::from_secs(
156+
env::var(constants::TRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS_ENV)
157+
.ok()
158+
.and_then(|v| v.parse().ok())
159+
.unwrap_or(constants::DEFAULT_QUARANTINED_TESTS_DISK_CACHE_TTL_SECS),
160+
);
146161
Self(RefCell::new(TestReport {
147162
test_report,
148163
command,
149164
started_at,
150165
quarantined_tests: None,
166+
quarantined_tests_disk_cache_ttl,
151167
codeowners,
152168
repo,
153-
variant: variant.clone(),
169+
variant,
154170
}))
155171
}
156172

@@ -298,22 +314,127 @@ impl MutTestReport {
298314
}
299315
}
300316

317+
fn get_quarantined_tests_cache_file_path(&self, org_url_slug: &str, repo_url: &str) -> PathBuf {
318+
let cache_key = Uuid::new_v5(
319+
&Uuid::NAMESPACE_URL,
320+
format!("{org_url_slug}#{repo_url}").as_bytes(),
321+
)
322+
.to_string();
323+
let quarantined_tests_cache_file_name = format!("quarantined_tests_{cache_key}.json");
324+
325+
env::temp_dir()
326+
.join(constants::CACHE_DIR)
327+
.join(quarantined_tests_cache_file_name)
328+
}
329+
330+
fn load_quarantined_tests_from_disk_cache(
331+
&self,
332+
org_url_slug: &str,
333+
repo_url: &str,
334+
) -> Option<HashMap<String, bool>> {
335+
let cache_path = self.get_quarantined_tests_cache_file_path(org_url_slug, repo_url);
336+
337+
let cache_data = match fs::read_to_string(&cache_path) {
338+
Ok(data) => data,
339+
Err(err) => {
340+
tracing::warn!("Failed to read quarantined tests cache file: {:?}", err);
341+
return None;
342+
}
343+
};
344+
345+
let cache_entry: QuarantinedTestsDiskCacheEntry = match serde_json::from_str(&cache_data) {
346+
Ok(entry) => entry,
347+
Err(err) => {
348+
tracing::warn!("Failed to parse quarantined tests cache file: {:?}", err);
349+
return None;
350+
}
351+
};
352+
353+
let now = match SystemTime::now().duration_since(SystemTime::UNIX_EPOCH) {
354+
Ok(duration) => duration.as_secs(),
355+
Err(_) => {
356+
tracing::warn!("Failed to get current time");
357+
return None;
358+
}
359+
};
360+
let cache_age = now.saturating_sub(cache_entry.cached_at_secs);
361+
362+
if cache_age <= self.0.borrow().quarantined_tests_disk_cache_ttl.as_secs() {
363+
Some(cache_entry.quarantined_tests)
364+
} else {
365+
let _ = fs::remove_file(&cache_path);
366+
None
367+
}
368+
}
369+
370+
fn save_quarantined_tests_to_disk_cache(
371+
&self,
372+
org_url_slug: &str,
373+
repo_url: &str,
374+
quarantined_tests: &HashMap<String, bool>,
375+
) {
376+
let cache_path = self.get_quarantined_tests_cache_file_path(org_url_slug, repo_url);
377+
378+
let now = match SystemTime::now().duration_since(SystemTime::UNIX_EPOCH) {
379+
Ok(duration) => duration.as_secs(),
380+
Err(_) => {
381+
tracing::warn!("Failed to get current time");
382+
return;
383+
}
384+
};
385+
386+
let cache_entry = QuarantinedTestsDiskCacheEntry {
387+
quarantined_tests: quarantined_tests.clone(),
388+
cached_at_secs: now,
389+
};
390+
391+
// create cache directory if it doesn't exist
392+
let cache_dir = match cache_path.parent() {
393+
Some(dir) => dir,
394+
None => {
395+
tracing::warn!("Failed to get cache directory");
396+
return;
397+
}
398+
};
399+
if let Err(err) = fs::create_dir_all(cache_dir) {
400+
tracing::warn!("Failed to create cache directory: {:?}", err);
401+
return;
402+
}
403+
404+
if let Ok(json) = serde_json::to_string(&cache_entry) {
405+
if let Err(err) = fs::write(&cache_path, json) {
406+
tracing::warn!("Failed to write quarantined tests cache file: {:?}", err);
407+
}
408+
}
409+
}
410+
301411
fn populate_quarantined_tests(
302412
&self,
303413
api_client: &ApiClient,
304414
repo: &RepoUrlParts,
305415
repo_url: String,
306416
org_url_slug: String,
307417
) {
418+
// first check in-memory cache
308419
if self.0.borrow().quarantined_tests.as_ref().is_some() {
309-
// already fetched
310420
return;
311421
}
422+
423+
// then check disk cache
424+
if let Some(quarantined_tests) =
425+
self.load_quarantined_tests_from_disk_cache(&org_url_slug, &repo_url)
426+
{
427+
// update in-memory cache
428+
self.0.borrow_mut().quarantined_tests = Some(quarantined_tests);
429+
return;
430+
}
431+
432+
// cache miss - make API call
312433
let mut quarantined_tests = HashMap::new();
313434
let request = message::GetQuarantineConfigRequest {
314-
org_url_slug,
435+
org_url_slug: org_url_slug.clone(),
315436
test_identifiers: vec![],
316-
remote_urls: vec![repo_url],
437+
remote_urls: vec![repo_url.clone()],
317438
repo: repo.clone(),
318439
};
319440
let response = tokio::runtime::Builder::new_multi_thread()
@@ -336,7 +457,12 @@ impl MutTestReport {
336457
);
337458
}
338459
}
339-
self.0.borrow_mut().quarantined_tests = Some(quarantined_tests);
460+
461+
println!("quarantined_tests: {quarantined_tests:?}");
462+
463+
// update both in-memory and disk cache
464+
self.0.borrow_mut().quarantined_tests = Some(quarantined_tests.clone());
465+
self.save_quarantined_tests_to_disk_cache(&org_url_slug, &repo_url, &quarantined_tests);
340466
}
341467

342468
fn get_org_url_slug(&self) -> String {

0 commit comments

Comments
 (0)