gustavodemorais commented on PR #27415:
URL: https://github.com/apache/flink/pull/27415#issuecomment-4274304598
Hey @SteveStevenpoor, thanks for working on this! Beyond the inline
comments, I wanted to suggest a broader refactoring direction.
We've been moving our tests toward the TableTestProgram /
TableTestProgramRunner framework (you already know MultiJoinSemanticTests and
MultiJoinTestPrograms - but there are other suites). The idea is to define test
programs declaratively and let different runners execute them, which avoids a
lot of hand-rolled boilerplate.
I think it'd be great if we could add these tests using it as well and use
it as an opportunity to go more into this direction in the codebase.
1. Define new test programs in MultiJoinTestPrograms.java for the scenarios
that aren't already there (boolean joins, nested types, complex conditions,
etc.). The source data can go in MultiJoinTestUtils.java alongside the existing
USERS_SOURCE, ORDERS_SOURCE, PAYMENTS_SOURCE.
2. Create a new runner (e.g. ComparisonTests or ParameterizedSemanticTests?
extends CommonSemanticTestBase) that runs each program twice - once with
TABLE_OPTIMIZER_MULTI_JOIN_ENABLED=false and once with =true - and compares the
results. This replaces the manual executeQuery/getExpectedResult logic.
3. That gets rid of the custom TableInfo class, createTableView,
createSinkTable, the .out file generation, and the Scala interop
(JavaConverters, failingDataSource, scala StreamTableEnvironment). All of this
use a lot of old helpers or older classes that in general we're trying to avoid.
We could also make some simplifications - the main goal with the suite was
to make sure we cover missing use cases that we already had covered with binary
tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
