Cache /getQuarantineConfig on disk with short TTL for MutTestReport
#941
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
😎 Merged successfully - details. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #941 +/- ##
==========================================
+ Coverage 73.58% 73.86% +0.28%
==========================================
Files 72 72
Lines 16460 16539 +79
==========================================
+ Hits 12112 12217 +105
+ Misses 4348 4322 -26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
max-trunk
requested review from
TylerJang27,
acatxnamedvirtue,
cmillar-trunk,
gnalh and
mb1206
as code owners
gnalh
reviewed
Dec 22, 2025
test_report/src/report.rs
Outdated
Comment on lines
337
to
351
| let cache_data = match fs::read_to_string(&cache_path) { | ||
| Ok(data) => data, | ||
| Err(err) => { | ||
| tracing::warn!("Failed to read quarantined tests cache file: {:?}", err); | ||
| return None; | ||
| } | ||
| }; | ||
|
|
||
| let cache_entry: QuarantinedTestsDiskCacheEntry = match serde_json::from_str(&cache_data) { | ||
| Ok(entry) => entry, | ||
| Err(err) => { | ||
| tracing::warn!("Failed to parse quarantined tests cache file: {:?}", err); | ||
| return None; | ||
| } | ||
| }; |
Collaborator
There was a problem hiding this comment.
Nit: I think this can be simplified by having it streamed directly from the reader
serde_json::from_reader(reader)
gnalh
reviewed
Dec 22, 2025
test_report/src/report.rs
Outdated
Comment on lines
418
to
432
| // first check in-memory cache | ||
| if self.0.borrow().quarantined_tests.as_ref().is_some() { | ||
| // already fetched | ||
| return; | ||
| } | ||
|
|
||
| // then check disk cache | ||
| if let Some(quarantined_tests) = | ||
| self.load_quarantined_tests_from_disk_cache(&org_url_slug, &repo_url) | ||
| { | ||
| // update in-memory cache | ||
| self.0.borrow_mut().quarantined_tests = Some(quarantined_tests); | ||
| return; | ||
| } | ||
|
|
||
| // cache miss - make API call |
Collaborator
There was a problem hiding this comment.
Nit: The comments feel self-explanatory and unnecessary.
gnalh
approved these changes
Dec 22, 2025
gnalh
reviewed
Dec 22, 2025
| cached_at_secs: now, | ||
| }; | ||
|
|
||
| // create cache directory if it doesn't exist |
Collaborator
There was a problem hiding this comment.
This comment is also unnecessary
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TRUNK-17215
Caches the result of
/getQuarantineConfiglocally on disk with a TTL of 5 min, duration configurable via theTRUNK_QUARANTINED_TESTS_DISK_CACHE_TTL_SECSenv var. This caching is only configured forMutTestReport, currently only utilized by the rspec plugin. This should help mitigate the number of network requests needed for customers who:Tested with a locally-built gem with these changes, observed correct cache hits & misses. Example cached results file: