Repository navigation
test(v3/icons): assert .ico is small, not an exact byte size - #5627
Conversation
TestGenerateIcon asserted the generated .ico was exactly 571 bytes. After the default icons were updated (#5610), the single-16px .ico is 481 bytes, so the test failed on master (macOS + Windows). The exact byte count depends on the source image, so assert the file is small (<5000 bytes) and non-empty instead — which is the test's actual intent and won't re-break on future icon changes.
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughTwo test assertions in ChangesIcon Test Assertion Relaxation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies" 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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes a brittle v3 icon-generation test by removing hard-coded .ico byte-size assertions and replacing them with size-sanity checks that better match the test’s intent (a single 16px icon) and won’t break when the source icon artwork changes.
Changes:
- Replaced exact
.icosize assertions (571 bytes) with checks that the output is non-empty and “small” (≤ 5000 bytes) forsizes=16. - Applied the same “small & non-empty” validation to the
"0,16"sizes case (ensuring0is ignored).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
TestGenerateIcon hard-asserted the generated
.icowas exactly 571 bytes. After the default icons were updated in #5610, the single-16px.icois 481 bytes, so the test has been failing onmaster(macOS + Windows Go Tests v3).The exact byte count depends on the source image, so this replaces the exact-size assertions (two test cases) with a check that the file is small (<5000 bytes) and non-empty — the test's actual intent (a single 16px icon) — which won't re-break when icons change again.
Verified
go test -run TestGenerateIcon ./internal/commands/passes.Summary by CodeRabbit