Reject a non-positive size or count in Spool.split - #801
Conversation
A size of zero or less produced a step of zero or less, and the loop advances start by step, so the generator yielded forever. count=0 raised ZeroDivisionError. Both now raise ParameterError alongside the existing "one of size or count" check. _spool_map computes its own default size from the spool length, which is zero for an empty spool, so it asks for at least one patch per split. Mapping an empty spool over a client still returns an empty list.
|
Warning Review limit reached
Next review available in: 9 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #801 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 164 164
Lines 17829 17833 +4
=========================================
+ Hits 17829 17833 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
Spool.splitaccepted sizes and counts it could not honor:size=0and any negative size producedstep <= 0, and the loop advancesstartbystep, so the generator yielded forever.list(spool.split(size=0))never returns.count=0raisedZeroDivisionError.Both now raise
ParameterError, next to the existing "one of size or count" check._spool_mapderives its own default size from the spool length, which is zero for an empty spool, so it asks for at least one patch per split. Mapping an empty spool over a client still returns an empty list — that path had no test and now does.Found while reviewing #800, which is why the guard didn't ride along there: it changes the public API's error behavior and deserved its own tests.
Changelog
Spool.splitraisesParameterErrorfor a non-positivesizeorcount;size=0previously yielded forever andcount=0failed with a confusingTypeError.Checklist
I have (if applicable):