Skip to content

fix: Compare metadata of nested child fields in TestingJSONComparison - #993

Open
JonSnow1807 wants to merge 1 commit into
apache:mainfrom
JonSnow1807:compare-child-metadata
Open

JonSnow1807 wants to merge 1 commit into
apache:mainfrom
JonSnow1807:compare-child-metadata

Conversation

@JonSnow1807

Copy link
Copy Markdown

Summary

TestingJSONComparison::CompareFieldBase was comparing metadata on the parent field but not checking metadata on its children.

That meant two nested schemas could compare as equal even when their child fields had different metadata.

This change makes the comparison recurse into child fields, including dictionary children, and reuse the existing metadata comparison logic.

It still treats absent and empty metadata as equivalent, and the existing order-insensitive metadata comparison behavior is preserved.

Testing

Added a gtest covering metadata differences in list, map, and dictionary children.

All 291 CTest tests pass.

This is separate from #972, which deals with dictionary value metadata.

Fixes #973.

CompareFieldBase() serializes both fields without metadata and then compares
the metadata of the top-level field only, so a consumer that drops the
metadata of a child of a struct, list, map or union field (e.g., an extension
type on a list item) still passes the comparison.

Recurse through the children of each field (and the children of a dictionary
value type) and compare their metadata at ".children[i].metadata". Empty and
absent metadata are treated as equal there because IPC and the integration
JSON both omit empty metadata.

Fixes apache#973
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.56%. Comparing base (55df060) to head (fa231a5).

Files with missing lines Patch % Lines
src/nanoarrow/testing/testing.cc 33.33% 0 Missing and 12 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #993      +/-   ##
==========================================
- Coverage   78.59%   78.56%   -0.04%     
==========================================
  Files         106      106              
  Lines       17153    17170      +17     
  Branches     2043     2056      +13     
==========================================
+ Hits        13482    13489       +7     
+ Misses       2422     2421       -1     
- Partials     1249     1260      +11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TestingJSONComparison ignores metadata on nested child fields

2 participants