[tests] Add end-to-end tests for cdc connector uber jars #594
Conversation
|
This pull request introduced a new module Currently, this pull request only add e2e tests for mysql-cdc. I will add tests for other cdc connectors later. |
|
The e2e can verify missing class problem of uber jar. See the artifact logs in the build: |
wuchong
force-pushed
the
e2e-tests
branch
2 times, most recently
from
0ff7ace
to
466a658
Compare
We have to fallback to v1.15.3 testcontainers, because Azure Pipeline always complaining the following error when starting the container: ``` 04:18:43,894 [main] INFO 🐳 [jark/oracle-xe-11g-r2-cdc:0.1] [] - Waiting for database connection to become available at jdbc:oracle:thin:system/oracle@localhost:49165:xe using query 'SELECT 1 FROM DUAL' 04:18:43,949 [docker-java-stream--1472620850] INFO com.ververica.cdc.connectors.e2e.OracleE2eITCase [] - STDOUT: Starting Oracle Net Listener. 04:18:44,102 [docker-java-stream--1472620850] INFO com.ververica.cdc.connectors.e2e.OracleE2eITCase [] - STDOUT: Starting Oracle Database 11g Express Edition instance. 04:18:58,842 [docker-java-stream--1472620850] INFO com.ververica.cdc.connectors.e2e.OracleE2eITCase [] - STDOUT: 04:22:44,218 [main] ERROR 🐳 [jark/oracle-xe-11g-r2-cdc:0.1] [] - Could not start container java.lang.IllegalStateException: Container is started, but cannot be accessed by (JDBC URL: jdbc:oracle:thin:system/oracle@localhost:49165:xe), please check container logs ``` According to this issue[1], this might be a problem of Oracle testcontainers v1.16 working with "wnameless/oracle-xe-11g-r2". A better solution might be upgrade our Oracle base image to "gvenzl/oracle-xe" which is the default image of Oracle testcontainers v1.16. [1]: testcontainers/testcontainers-java#4297
There was a problem hiding this comment.
Thanks @wuchong for the great work, the PR looks good totally, I only left some minor comments
...r-oracle-cdc/src/test/java/com/ververica/cdc/connectors/oracle/utils/OracleCdcContainer.java
Outdated
Show resolved
Hide resolved
flink-cdc-e2e-tests/src/test/java/com/ververica/cdc/connectors/e2e/utils/JdbcProxy.java
Outdated
Show resolved
Hide resolved
| if (!result) { | ||
| checkResult(expectedResult, table, fields); | ||
| } |
There was a problem hiding this comment.
A check with timeout exception is better than a final check
There was a problem hiding this comment.
I don't think so. A check here can print the mimatched results which is much more meaningful than timeout exception.
There was a problem hiding this comment.
I agree with you, I noticed the timeout check logic may problem, if the first attempt we didn't pull enough data(i.e the upstream program is still writing), the check will fail due to we call checkResult at first time in and the timeout is useless?
flink-cdc-e2e-tests/src/test/java/com/ververica/cdc/connectors/e2e/utils/ParameterProperty.java
Outdated
Show resolved
Hide resolved
flink-cdc-e2e-tests/src/test/java/com/ververica/cdc/connectors/e2e/utils/SQLJobSubmission.java
Outdated
Show resolved
Hide resolved
flink-cdc-e2e-tests/src/test/java/com/ververica/cdc/connectors/e2e/utils/TestUtils.java
Outdated
Show resolved
Hide resolved
| import java.util.List; | ||
|
|
||
| /** End-to-end tests for mysql-cdc connector uber jar. */ | ||
| public class MySqlE2eITCase extends FlinkContainerTestEnvironment { |
There was a problem hiding this comment.
I think end to end test is different with integration test, how about rename it to MySqlEndToEndTest ?
and we use tests package prefix in Flink, here we use e2e, do we need to keep the same style?
There was a problem hiding this comment.
There is only unit-test and integration-test phase in maven-surefire-plugin, no end-to-end notion in it. We matches *Test.java suffix as unit test, and *ITCase.java as integration-test. So we have to name it ITCase.
You can also see that e2e tests in flink also name like SQLClientSchemaRegistryITCase.
There was a problem hiding this comment.
It makes sense to me, how about the package prefix? Do you think we need to keep align with Flink project
There was a problem hiding this comment.
OK. I will use tests package
| " 'password' = '" + MYSQL_TEST_PASSWORD + "',", | ||
| " 'database-name' = '" + mysqlInventoryDatabase.getDatabaseName() + "',", | ||
| " 'table-name' = 'products_source',", | ||
| " 'server-id' = '5800-5900',", |
There was a problem hiding this comment.
the server-id range is to large here
There was a problem hiding this comment.
Is there any problem to use a large range?
There was a problem hiding this comment.
I mean this large range looks like we need 100 server ids in e2e test, but actually 4 server ids is enough
There was a problem hiding this comment.
Yes. But it should still works even we set a large range, right? I think many users will set a large range, because they may change parallelism frequently.
flink-cdc-e2e-tests/src/test/java/com/ververica/cdc/connectors/e2e/OracleE2eITCase.java
Outdated
Show resolved
Hide resolved
flink-cdc-e2e-tests/src/test/java/com/ververica/cdc/connectors/e2e/PostgresE2eITCase.java
Outdated
Show resolved
Hide resolved
|
Thanks @leonardBang , I have addressed the comments. |
We introduced flink-sql-connectors-xxx for all mysql-cdc, postgres-cdc and mongodb-cdc. This module is used to shade kafka and build an uber jar which is mainly used for SQL clusters. However, we don't have any tests for the uber jars, e.g. does the shading work? do we exclude any needed classes? This can be covered by e2e tests.