feat(shared-chunk-cache): adding basic e2e test for shared-chunk-cache#4503
feat(shared-chunk-cache): adding basic e2e test for shared-chunk-cache#4503raj-prince merged 4 commits intomasterfrom
Conversation
raj-prince
force-pushed
the
shared_cache_e2e
branch
from
a8de675 to
1116add
Compare
github-actions
Bot
added
the
remind-reviewers
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4503 +/- ##
==========================================
- Coverage 83.64% 0 -83.65%
==========================================
Files 164 0 -164
Lines 20272 0 -20272
==========================================
- Hits 16956 0 -16956
+ Misses 2676 0 -2676
+ Partials 640 0 -640
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a foundational end-to-end test for the shared-chunk-cache functionality. The test simulates a common use case with multiple mounts to validate that the cache is correctly hit and missed, and that data is served from the cache as expected. This will help ensure the stability and performance of the shared chunk cache feature. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
raj-prince
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces end-to-end tests for the shared chunk cache feature, covering cache miss and hit scenarios for both single and multiple mount configurations. The tests are well-structured and provide a good baseline for verifying the feature's functionality. My feedback includes suggestions to enhance test robustness by improving error handling when scanning the cache directory and preventing potential division-by-zero panics during performance metric calculations.
|
Hi @meet2mky, @Tulsishah, @charith87, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @meet2mky, @Tulsishah, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
meet2mky
left a comment
meet2mky
left a comment
There was a problem hiding this comment.
Some comments, Does these tests need to be in a separate package and not read_cache. What's the reason for it?
|
Hi @Tulsishah, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
7 similar comments
|
Hi @Tulsishah, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @Tulsishah, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @Tulsishah, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @Tulsishah, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @Tulsishah, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @Tulsishah, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @Tulsishah, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
Read_cache test are single mount test, while this is multi-mount test, hence created a separate package for this. |
raj-prince
force-pushed
the
shared_cache_e2e
branch
from
b85e59c to
dd98daa
Compare
raj-prince
force-pushed
the
shared_cache_e2e
branch
from
dd98daa to
2c4e7a2
Compare
|
Looks good. Let's add the information of this package in improved_e2e_test script and in test_config.yaml for GKE in separate PR for continuous testing. |
#4503) * fixing lint and better formatting * swapping the file content - incorrectly swapped during rebase * review comments * fixing formatting
2 participants
Description
Adding cache-hit/miss test for shared-chunk-cache using two mounts scenario. To keep the test-execution robust we used modification time to test the cache-hit/cache-miss logic, not via log-parsing.
Link to the issue in case of a bug fix.
b/495298967
Testing details
Any backward incompatible change? If so, please explain.