Test queries cleanup #20903
Test queries cleanup #20903
Conversation
Replace with non-deprecated equivalents.
`returnsEmptyResult()` can be followed by e.g. `isFullyPushedDown`, so not always can it be replaced with `.result().isEmpty()`.
Replace with non-deprecated equivalents.
Replace with non-deprecated equivalent.
findepi
added
test
no-release-notes
github-actions
bot
added
tests:hive
iceberg
| /** | ||
| * @deprecated use {@code result().isEmpty()} instead. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
You can consider adding QueryAssert and() to ResultAssert so you can get back to top level assertions.
Not a great fan - but have seen stuff like this.
There was a problem hiding this comment.
i wouldn't want that. ie ResultAssert should not depend on QueryAssert. maybe there wasn't any query in the first place?
| /** | ||
| * @deprecated use {@code result().isEmpty()} instead. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
re: " Undeprecate returnsEmptyResult() "
when using isFullyPushedDown the query runs twice with and without pushdown. Why is it useful to match what the result is? e.g. we don't use matches in push down tests for example.
There was a problem hiding this comment.
good question. when i introduced isFullyPushedDown i felt there are cases where it's valuable to assert results, as away to witness that test query is reasonable.
No description provided.