Clean the code in field.rs and add more tests
#2345
Conversation
Signed-off-by: remzi <13716567376yh@gmail.com>
| @@ -698,41 +684,25 @@ impl Field { | |||
| /// * self.metadata is a superset of other.metadata | |||
| /// * all other fields are equal | |||
There was a problem hiding this comment.
I am not sure whether this is reasonable. For example,
let child_field1 = ...;
let child_field2 = ...;
let field1 = Struct(&child_field1);
let mut field2 = Struct(&child_field2);
field2.try_merge(field1).unwrap();
assert_eq!(false, field2.contains(field1))As field2 has merged field1 into itself, I expect the field2 to contain field1.
However, you will find that the answer is false because field1 contains child_field1
but field2 contains child_field1 and child_field2.
There was a problem hiding this comment.
I am not sure either -- can you possibly add this as a test (so that at least the behavior is documented and if we change it in the future we will also have to change the test)?
|
@alamb Please help to review if you have time. 😁 |
alamb
changed the title
field.rs and add more testsField::contains and add more tests
alamb
changed the title
Field::contains and add more testsfield.rs and add more tests
| } | ||
|
|
||
| #[test] | ||
| fn test_contains_nullable() { |
| } | ||
| if is_new_field { | ||
| nested_fields.push(from_field.clone()); | ||
| match nested_fields |
There was a problem hiding this comment.
I agree this is easier to read 👍
| @@ -698,41 +684,25 @@ impl Field { | |||
| /// * self.metadata is a superset of other.metadata | |||
| /// * all other fields are equal | |||
There was a problem hiding this comment.
I am not sure either -- can you possibly add this as a test (so that at least the behavior is documented and if we change it in the future we will also have to change the test)?
Good concern. I will add. Please don't merge this now. Thank you. |
|
marking as draft so we don't accidentally merge this until it is updated |
Signed-off-by: remzi <13716567376yh@gmail.com>
|
Benchmark runs are scheduled for baseline = 195d9c5 and contender = d4ad4b7. d4ad4b7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Signed-off-by: remzi 13716567376yh@gmail.com
Which issue does this PR close?
None.
Rationale for this change
What changes are included in this PR?
fn _field,fn to_json,fn try_mergeandfn containsfn containsAre there any user-facing changes?
No.