test(spark): tail-sweep coverage for low-coverage datasource classes - #19405
test(spark): tail-sweep coverage for low-coverage datasource classes#19405yihua wants to merge 1 commit into
Conversation
Add unit coverage for BaseHoodieCatalystExpressionUtils order-preserving transformation matching, org.apache.spark.sql.avro.AvroUtils schema matching and validation, and FileFormatUtilsForFileGroupReader filter lowering. Each assertion pins the exact translated output.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19405 +/- ##
============================================
+ Coverage 72.49% 74.85% +2.35%
+ Complexity 32888 32341 -547
============================================
Files 2574 2574
Lines 149010 142978 -6032
Branches 18749 17571 -1178
============================================
- Hits 108032 107032 -1000
+ Misses 32513 27885 -4628
+ Partials 8465 8061 -404
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for working on this! This PR adds behavior-pinning unit tests for three low-coverage Spark-datasource classes: the order-preserving transformation matcher in BaseHoodieCatalystExpressionUtils, AvroUtils/AvroSchemaHelper, and FileFormatUtilsForFileGroupReader.applyFiltersToPlan. I traced each assertion against the actual source implementations (including isCastPreservingOrdering, the OrderPreservingTransformation extractor cases, the translate/reduceLeft(And) filter lowering, and the Avro schema-matching/validation error messages) and every expected value lines up with real behavior. No correctness issues found. A few style/readability suggestions in the inline comments. Please take a look, and this should be ready for a Hudi committer or PMC member to take it from here. One minor naming nit in TestAvroUtils; the other two files are clean.
cc @yihua
| | {"name":"id","type":"int"}, | ||
| | {"name":"ghost","type":["null","int"]} | ||
| |]}""".stripMargin) | ||
| val helper2 = new AvroUtils.AvroSchemaHelper( |
There was a problem hiding this comment.
🤖 nit: helper2 loses the context that this helper covers the optional-ghost case — could you rename it to helperWithOptionalGhost to mirror avroWithOptionalGhost and make the two scenarios easier to scan at a glance?
Describe the issue this Pull Request addresses
Part 2 of the Spark-datasource small-class coverage tail-sweep (sibling to #19164). Several small classes in the Spark datasource had low unit-test coverage, with branches reached only indirectly, if at all. This adds focused, behavior-pinning unit tests for the genuinely uncovered ones.
Summary and Changelog
Adds unit coverage for three low-coverage classes. Every assertion pins exact output, so a wrong result would fail the test.
org.apache.spark.sql.BaseHoodieCatalystExpressionUtils(0 missed lines, but many uncovered branches): newTestCatalystExpressionOrderPreservingdrivestryMatchAttributeOrderingPreservingTransformationacross the wholeOrderPreservingTransformationmatch. It asserts the exact sourceAttributeReferencerecovered for identity, arithmetic on either operand, unary math, string case, date add/sub, and order-preserving up-cast, and that non-order-preserving shapes (numeric-to-string cast, attribute-free arithmetic, a non-whitelistedSqrt) do not match.org.apache.spark.sql.avro.AvroUtils: newTestAvroUtilscoverssupportsDataType(atomic, struct, array, map, null supported;CalendarIntervaland its wrappers unsupported) and theAvroSchemaHelpermatching and validation paths previously exercised only through the Avro serializers: non-RECORD rejection, by-name vs positional field lookup, extra-Catalyst-field and extra-required-Avro-field validation (including theignoreNullableand nullable-Avro-field skips), and the ambiguous case-insensitive by-name match. Each error case pins the raisedIncompatibleSchemaExceptionmessage.org.apache.spark.sql.FileFormatUtilsForFileGroupReader: newTestFileFormatUtilsForFileGroupReadercoversapplyFiltersToPlan, pinning the Catalyst expression produced for each pushed-down data-sourceFilter(comparisons, null checks,In, string predicates,AlwaysTrue/AlwaysFalse, a nested and/or/not tree, and multi-filterAnd), and that an empty filter list returns the input plan unchanged.Candidates verified and left out:
VectorDistanceUtilsis already exhaustively covered with exact-value assertions byTestHoodieVectorSearchFunction, so no new test was added.HiveSyncProcedureandHoodieNestedSchemaPruningneed heavier end-to-end scaffolding (hive metastore, optimizer plan fixtures) and are deferred to a separate pass. No code was copied.Impact
Test-only. No production code changes, no public API change, and no behavior change.
Risk Level
none
Documentation Update
none
Contributor's checklist