V10.5.3/replace httpbin with mockerapi#165
Conversation
Greptile SummaryThis PR replaces external httpbin.org HTTP calls in
Confidence Score: 5/5Safe to merge — all changes are confined to test code with no production logic affected. The PR touches only test files. The httpbin replacement with an in-process mock is well-structured and thread-safe. The timing-assertion and cache-assertion refactors reduce duplication while preserving the same logical coverage. No new issues were found beyond what earlier review threads already discussed. No files require special attention beyond the timing discussion already captured in earlier review comments on SlimMemoryCacheTest.cs. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Thread.Sleep - wait for expiry] --> B[AssertNamespaceIsLogicallyExpired\nSpinWait up to 15 s\n_cache.Count == 0]
B -->|true| C[AssertNamespaceIsPhysicallyPresent\nInstantaneous snapshot\nWhere.Count == NumberOfItemsToCache]
B -->|timeout| FAIL1[Test Fails\nLogical expiry not observed]
C -->|pass| D[AssertNamespaceIsPhysicallyRemoved\nSpinWait up to 15 s\n!_cache.Any]
C -->|fail| FAIL2[Test Fails\nPhysical entries unexpectedly absent\npossible race with cleanup sweep]
D -->|true| PASS[Test Passes]
D -->|timeout| FAIL3[Test Fails\nPhysical removal not observed]
style FAIL1 fill:#f66,color:#fff
style FAIL2 fill:#f66,color:#fff
style FAIL3 fill:#f66,color:#fff
style PASS fill:#6a6,color:#fff
Reviews (3): Last reviewed commit: "🔄 replace slimhttpclientfactory with st..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f613adad2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| private void AssertNamespaceIsLogicallyExpired(string ns) | ||
| { | ||
| Assert.True(SpinWait.SpinUntil(() => _cache.Count(ns) == 0, CleanupTimeout), |
There was a problem hiding this comment.
Require proof that expiration fixtures were populated
When either expiration-verification test is run by itself, or if one of the earlier priority-ordered population tests is skipped or fails before adding entries, this predicate is already true for an empty namespace. The deleted NumberOfItemsToCache checks were the only proof that the namespace still contained the 1000 expired entries before cleanup, so these tests can now pass without exercising logical expiration or the background removal path at all.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #165 +/- ##
===========================================
+ Coverage 82.80% 94.19% +11.39%
===========================================
Files 602 602
Lines 19157 19186 +29
Branches 2001 2009 +8
===========================================
+ Hits 15863 18073 +2210
+ Misses 3216 1049 -2167
+ Partials 78 64 -14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Add assertion helper method to verify cache entries remain physically present before cleanup. Reduce jitter tolerance in TimeMeasure test to make timing assertions more deterministic.
This pull request refactors the
TimeMeasureTestunit tests to improve code readability and maintainability. The main change is the introduction of a helper method,AssertElapsedAround, which standardizes the way elapsed time assertions are made across all test cases. Additionally, the jitter logic is updated to use separate lower and upper bounds, allowing for more flexible time assertions.Key changes:
Test Assertion Refactoring:
AssertElapsedAroundhelper method to replace repeatedAssert.InRangelogic, providing a single place to manage elapsed time assertions. This method usesLowerJitterandUpperJitterfor more flexible boundaries.WithActionandWithFunc(with 0 to 10 arguments) to useAssertElapsedAroundinstead of directly callingAssert.InRange. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19] [20] [21]Asynchronous Test Updates:
AssertElapsedAroundfor consistency with the synchronous tests. [1] [2] [3] [4] [5]Test Configuration Improvement:
Jittervalue withLowerJitterandUpperJitterto allow for asymmetric tolerance in timing assertions, making the tests more robust to timing fluctuations.