klion26 commented on code in PR #7935:
URL: https://github.com/apache/arrow-rs/pull/7935#discussion_r2214726707
##########
parquet-variant/src/builder.rs:
##########
@@ -1860,10 +2046,22 @@ mod tests {
let inner_object_variant = outer_object.field(2).unwrap();
let inner_object = inner_object_variant.as_object().unwrap();
- assert_eq!(inner_object.len(), 1);
+ assert_eq!(inner_object.len(), 3);
assert_eq!(inner_object.field_name(0).unwrap(), "b");
assert_eq!(inner_object.field(0).unwrap(), Variant::from("a"));
+ let inner_iner_object_variant_c = inner_object.field(1).unwrap();
+ let inner_inner_object_c =
inner_iner_object_variant_c.as_object().unwrap();
+ assert_eq!(inner_inner_object_c.len(), 1);
+ assert_eq!(inner_inner_object_c.field_name(0).unwrap(), "aa");
+ assert_eq!(inner_inner_object_c.field(0).unwrap(),
Variant::from("bb"));
+
+ let inner_iner_object_variant_d = inner_object.field(2).unwrap();
+ let inner_inner_object_d =
inner_iner_object_variant_d.as_object().unwrap();
+ assert_eq!(inner_inner_object_d.len(), 1);
+ assert_eq!(inner_inner_object_d.field_name(0).unwrap(), "cc");
+ assert_eq!(inner_inner_object_d.field(0).unwrap(),
Variant::from("dd"));
+
assert_eq!(outer_object.field_name(1).unwrap(), "b");
assert_eq!(outer_object.field(1).unwrap(), Variant::from(true));
}
Review Comment:
Okay, I will add more tests for this. Currently, the list inside the object
depends on the `from_json` test, add a unit test for it in `builder.rs` is
better, I'll add this.
Not sure if the tests like `test_xx_no_finishi()`(such as
`test_object_builder_to_list_builder_inner_no_finish()`) are enough to cover
the rollback logic?
We've called `drop` in the `test_xx_no_finish()` test, do we need to call
`drop` if we add tests to cover the rollaback logic?
--
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
