-
Notifications
You must be signed in to change notification settings - Fork 589
test(general): stat.GetBlockAlignedSize #4679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the computation of block-aligned sizes for cache storage utilization by updating the core logic and adding comprehensive tests.
- Implements the GetBlockAlignedSize function with overflow and parameter validation
- Adds extensive tests covering both valid inputs and error cases
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/stat/blocksize.go | Adds the block-aligned size calculation with parameter checks |
| internal/stat/blocksize_test.go | Provides thorough test cases for valid and error scenarios |
internal/stat/blocksize.go
Outdated
| } | ||
|
|
||
| if size < 0 { | ||
| return -1, errors.Wrap(errInvalidParameter, "blockSizeBytes must be positive") |
Copilot
AI
Jun 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message for a negative size misrepresents the error by stating 'blockSizeBytes must be positive'. Consider updating it to 'size must be non-negative' to accurately reflect the invalid input.
| return -1, errors.Wrap(errInvalidParameter, "blockSizeBytes must be positive") | |
| return -1, errors.Wrap(errInvalidParameter, "size must be non-negative") |
internal/stat/blocksize_test.go
Outdated
| } | ||
|
|
||
| for _, tc := range cases { | ||
| t.Run("", func(t *testing.T) { |
Copilot
AI
Jun 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using an empty string for subtest names makes it harder to identify failing cases. Consider including descriptive names that reflect the test case parameters.
| t.Run("", func(t *testing.T) { | |
| t.Run(fmt.Sprintf("blockSize=%d, size=%d", tc.blockSize, tc.size), func(t *testing.T) { |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4679 +/- ##
==========================================
+ Coverage 75.86% 76.43% +0.56%
==========================================
Files 470 530 +60
Lines 37301 40171 +2870
==========================================
+ Hits 28299 30705 +2406
- Misses 7071 7451 +380
- Partials 1931 2015 +84 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
c737564 to
119074e
Compare
119074e to
9647619
Compare
This is to better compute cache storage utilization.