Raise a descriptive error if non-positive integer passed to in_groups_of.

This is more consistent than the current behaviour of raising a
`ZeroDivisionError: divided by 0` error when 0 is given, which can be
non-obvious especially if `in_groups_of` is part of a longer chain of
methods.

The negative case was ok - "ArgumentError: invalid slice size" - but
this error is clearer still.

2 files changed, 11 insertions, 0 deletions

diff --git a/activesupport/lib/active_support/core_ext/array/grouping.rb b/activesupport/lib/active_support/core_ext/array/grouping.rb
index 3529d57174..87ae052eb0 100644
--- a/activesupport/lib/active_support/core_ext/array/grouping.rb
+++ b/activesupport/lib/active_support/core_ext/array/grouping.rb
@@ -18,6 +18,11 @@ class Array
   #   ["3", "4"]
   #   ["5"]
   def in_groups_of(number, fill_with = nil)
+    if number.to_i <= 0
+      raise ArgumentError,
+        "Group size must be a positive integer, was #{number.inspect}"
+    end
+
     if fill_with == false
       collection = self
     else
diff --git a/activesupport/test/core_ext/array/grouping_test.rb b/activesupport/test/core_ext/array/grouping_test.rb
index b8cfe9728c..2eb0f05141 100644
--- a/activesupport/test/core_ext/array/grouping_test.rb
+++ b/activesupport/test/core_ext/array/grouping_test.rb
@@ -90,6 +90,12 @@ class GroupingTest < ActiveSupport::TestCase
     assert_equal [[1, 2, 3], [4, 5], [6, 7]],
       (1..7).to_a.in_groups(3, false)
   end
+
+  def test_in_groups_invalid_argument
+    assert_raises(ArgumentError) { [].in_groups_of(0) }
+    assert_raises(ArgumentError) { [].in_groups_of(-1) }
+    assert_raises(ArgumentError) { [].in_groups_of(nil) }
+  end
 end
 
 class SplitTest < ActiveSupport::TestCase