[GitHub] [flink] LadyForest edited a comment on pull request #18394: [FLINK-25520][Table SQL/API] Implement "ALTER TABLE ... COMPACT" SQL

[GitBox <gi...@apache.org>]

LadyForest edited a comment on pull request #18394:
URL: https://github.com/apache/flink/pull/18394#issuecomment-1016159041


   > I just feel, the testing classes are too complicate, can we simplify them? We only need to test what we need, there is no need to have very strong features.
   
   I'm afraid this version has been simplified to the most extent. After all, we need to invoke a job running (whatever job) for the ITCase right? Or to be more reasonable, let a meaningful job (which presents a "compaction") run.
   
   Because the helper test factory `TestManagedTableFactory` is located in the `table-common` module (which causes us not to depend on any existing source/sink impl, o.w. we will get a cyclic dependency), we cannot leverage the ready-made source/sink impl to let even a word count job run. So everything needs to be built from scratch.
   
   What if we move `TestManagedTableFactory` from the module `table-common` to `table-planner`?
   That is great to some extent, all the pom issues and "too complicated" test helpers are resolved, but `FactoryUtilTest#testManagedConnector` will fail.
   
   And I don't think it's a good idea to maintain two `TestManagedTableFactory` both under `table-common` and `table-planner` modules.


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org