Skip to content

Conversation

@minglu7
Copy link

@minglu7 minglu7 commented Oct 16, 2025

修复:更正 bytes2size 的阈值判断,修复边界数值显示错误问题(见 common/utils.go)
测试:新增 common/utils_test.go 覆盖典型与边界场景

Summary by CodeRabbit

  • Bug Fixes

    • Adjusted file size display thresholds to show larger units (GB/MB/KB) when values equal exactly 1 unit, improving readability of file size representations.
  • Tests

    • Added comprehensive unit tests for file size conversion functionality.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

Walkthrough

The Bytes2Size function's unit conversion boundaries are adjusted from strict greater-than comparisons to greater-than-or-equal comparisons, changing when byte values transition to larger units (GB, MB, KB). A new test file is added with table-driven tests verifying the function's behavior across multiple size ranges.

Changes

Cohort / File(s) Summary
Bytes2Size boundary adjustment
common/utils.go
Changed unit conversion thresholds from > to >= for GB, MB, and KB transitions. Values exactly equal to 1 unit now render with the larger unit instead of the smaller unit.
Unit tests for Bytes2Size
common/utils_test.go
Added table-driven test suite covering bytes, kilobyte, kilobyte rounding, megabyte, and gigabyte representations. Tests use standard Go subtest pattern with fatal assertions on mismatch.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A byte stepped up with newfound pride,
When thresholds changed from > to side,
Now numbers dance at equal mark,
Through gigabytes, both light and dark,
Tests now guard each conversion true! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "fix(utils): 修正 bytes2size 阈值并补充单元测试" directly and accurately describes the two main changes in the pull request. The title explicitly references fixing the bytes2size threshold (matching the boundary check changes from > to >= in common/utils.go) and adding unit tests (matching the addition of common/utils_test.go). The title is concise, specific to the affected function, and uses clear language that conveys the exact nature of the changes without vague terms or unnecessary information. A developer reviewing the commit history would immediately understand that this PR addresses a threshold calculation issue and includes corresponding test coverage.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
common/utils_test.go (1)

5-25: LGTM! Consider additional test cases for comprehensive coverage.

The table-driven test structure is well-implemented and validates the boundary fix. The test cases confirm:

  • Bytes display correctly (500 B)
  • Exact boundaries display in larger units (1 KB, 1 MB, 1.00 GB)
  • Fractional KB values truncate to integers (1536 → "1 KB")

For more comprehensive coverage, consider adding test cases for:

  • Edge values: 0 bytes, 1 byte
  • Fractional MB: e.g., 1572864 (1.5 MB) → "1 MB"
  • Fractional GB: e.g., 1610612736 (1.5 GB) → "1.50 GB"
  • Large intermediate values: e.g., 1023 KB, 1023 MB
  • One byte below boundaries: 1023, 1048575, 1073741823

Apply this diff to add edge case tests:

 		{name: "gigabyte", input: 1024 * 1024 * 1024, want: "1.00 GB"},
+		{name: "zero", input: 0, want: "0 B"},
+		{name: "oneByte", input: 1, want: "1 B"},
+		{name: "justBelowKB", input: 1023, want: "1023 B"},
+		{name: "justBelowMB", input: 1048575, want: "1023 KB"},
+		{name: "justBelowGB", input: 1073741823, want: "1023 MB"},
+		{name: "fractionalMB", input: 1572864, want: "1 MB"}, // 1.5 MB truncated
+		{name: "fractionalGB", input: 1610612736, want: "1.50 GB"}, // 1.5 GB
 	}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bcf93a2 and 0d590ae.

📒 Files selected for processing (2)
  • common/utils.go (1 hunks)
  • common/utils_test.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
common/utils_test.go (1)
common/utils.go (1)
  • Bytes2Size (147-163)
🔇 Additional comments (1)
common/utils.go (1)

150-156: LGTM! Boundary fix is correct.

The change from > to >= properly handles exact unit boundaries. Now:

  • 1024 bytes displays as "1 KB" instead of "1024 B"
  • 1048576 bytes displays as "1 MB" instead of "1024 KB"
  • 1073741824 bytes displays as "1.00 GB" instead of "1024 MB"

This aligns with standard unit conversion behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant