lirui-apache commented on issue #9149: [FLINK-13303][hive] Add e2e test for hive connector URL: https://github.com/apache/flink/pull/9149#issuecomment-513194062 Hi @twalthr, thanks for your suggestions. I agree a real e2e test should use what users would use in real deployments and is good to have. But implementing such tests requires extra efforts, e.g. rework the `FlinkStandaloneHiveRunner` to provide a standalone hive metastore server, and figure out how to test against different hive versions. Furthermore, development of such test cases are usually less efficient than ordinary JUnit tests, e.g. it'll be harder to debug when something goes wrong. The intention of this PR is to improve our test coverage of hive connector. Currently, we only have quite primitive tests like `HiveTableSinkTest` and `HiveTableSourceTest`, which are not sufficient to catch issues like [FLINK-13279](https://issues.apache.org/jira/browse/FLINK-13279). I added this test in `flink-end-to-end-tests` instead of `flink-connector-hive` only because `flink-sql-client` depends on `flink-connector-hive`, and therefore we can't access `LocalExecutor` in `flink-connector-hive`. So I guess another option is to remove the `flink-connector-hive` dependency in `flink-sql-client`. Make `flink-connector-hive` depend on `flink-sql-client` (in test scope), and move the test here to `flink-connector-hive` as an ordinary JUnit test. This way we can maintain the simplicity while being able to catch most of potential issues. Real e2e tests as you suggested can be implemented later, and perhaps in another release. Does this make sense to you?
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
