kou commented on code in PR #45328:
URL: https://github.com/apache/arrow/pull/45328#discussion_r1924684612


##########
c_glib/test/test-array.rb:
##########
@@ -116,6 +116,18 @@ def test_to_s
     CONTENT
   end
 
+  def test_validation_success
+    array = build_int32_array([1, 2, 3, 4, 5])
+    assert_equal(true, array.validate)
+  end
+
+  def test_validation_fail
+    array = Arrow::Int8Array.new(-1, Arrow::Buffer.new(""), 
Arrow::Buffer.new(""), -1)
+    assert_raise(Arrow::Error::Invalid) do
+      array.validate
+    end

Review Comment:
   Could you also check error message like 
https://github.com/apache/arrow/blob/f8f17a58adb6b5fe4fed6c13d7d3b2e64b4a9acd/c_glib/test/test-array.rb#L179-L185
 ?



##########
c_glib/arrow-glib/basic-array.h:
##########
@@ -890,6 +890,10 @@ GARROW_AVAILABLE_IN_ALL
 GArrowDecimal256 *
 garrow_decimal256_array_get_value(GArrowDecimal256Array *array, gint64 i);
 
+GARROW_AVAILABLE_IN_20_0
+gboolean
+garrow_array_validate(GArrowArray *array, GError **error);

Review Comment:
   Could you move this to the `garrow_array_*()` block?
   
https://github.com/apache/arrow/blob/f8f17a58adb6b5fe4fed6c13d7d3b2e64b4a9acd/c_glib/arrow-glib/basic-array.h#L128



##########
c_glib/test/test-array.rb:
##########
@@ -116,6 +116,18 @@ def test_to_s
     CONTENT
   end
 
+  def test_validation_success
+    array = build_int32_array([1, 2, 3, 4, 5])
+    assert_equal(true, array.validate)

Review Comment:
   Could you use power-assert style for predicate?
   
   ```suggestion
       assert do
         array.validate
       end
   ```



##########
c_glib/test/test-array.rb:
##########
@@ -116,6 +116,18 @@ def test_to_s
     CONTENT
   end
 
+  def test_validation_success
+    array = build_int32_array([1, 2, 3, 4, 5])
+    assert_equal(true, array.validate)
+  end
+
+  def test_validation_fail
+    array = Arrow::Int8Array.new(-1, Arrow::Buffer.new(""), 
Arrow::Buffer.new(""), -1)
+    assert_raise(Arrow::Error::Invalid) do
+      array.validate
+    end
+  end
+

Review Comment:
   Could you move this to the end of this file to align with `basic-array.h` 
order?
   
   Could you use `sub_test_case` when we have multiple patterns for a method?
   
   ```ruby
   sub_test_case("#validate") do
     def test_valid
     end
   
     def test_invalid
     end
   end
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to