Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test code cleanup around testJoinPushdown #19998

Conversation

findepi
Copy link
Member

@findepi findepi commented Dec 1, 2023

Prep for #19996

For a test code like this

```
for (String operator : nonEqualities) {
    assertJoinConditionallyPushedDown(
    ...
```

the raised exception would contain no information which case was the
failing one.
Apparently a syntactic transformation was applied: from "with parameters
invoke test method" to "test method with a for loop over parameters".
This didn't account for return statement that could potentially lead to
cutting off portion of test coverage.
No need for separate method, if all testing is already covered by
`testJoinPushdown`.
There is no need to recreate the test table for overy of the `for
(JoinOperator` loop runs.
@findepi findepi added test no-release-notes This pull request does not require release notes entry labels Dec 1, 2023
@cla-bot cla-bot bot added the cla-signed label Dec 1, 2023
if (joinOperator == FULL_JOIN && !hasBehavior(SUPPORTS_JOIN_PUSHDOWN_WITH_FULL_JOIN)) {
// Covered by verifySupportsJoinPushdownWithFullJoinDeclaration
return;
continue;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verification that "!SUPPORTS_JOIN_PUSHDOWN => join pushdown does not
occur" is covered by `testJoinPushdown`, so
`verifySupportsJoinPushdownDeclaration` was redundant.
@findepi findepi force-pushed the findepi/create-test-table-once-in-testjoinpushdown-f28b9f branch from bb17778 to e94369a Compare December 8, 2023 12:49
@findepi findepi merged commit 5fe7e12 into trinodb:master Dec 8, 2023
65 checks passed
@findepi findepi deleted the findepi/create-test-table-once-in-testjoinpushdown-f28b9f branch December 8, 2023 14:30
@github-actions github-actions bot added this to the 435 milestone Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed no-release-notes This pull request does not require release notes entry test
Development

Successfully merging this pull request may close these issues.

2 participants