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