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

[Feature][Clickhouse-Connector-V2] Add Clickhouse e2e testcase #2954

Closed
wants to merge 0 commits into from

Conversation

FWLamb
Copy link
Contributor

@FWLamb FWLamb commented Sep 30, 2022

Purpose of this pull request

Check list

@liugddx
Copy link
Member

liugddx commented Sep 30, 2022

Please solve the problem of CI. please look at https://github.com/apache/incubator-seatunnel/blob/dev/docs/en/contribution/setup.md

</parent>
<modelVersion>4.0.0</modelVersion>

<artifactId>connector-clickhouse-spark-e2e</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

move to this

reference
#2946

Copy link
Contributor Author

Choose a reason for hiding this comment

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

move to this

reference #2946

I want to know the relationship between the two or the different modules

Copy link
Member

Choose a reason for hiding this comment

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

You can use it test spark/flink with once code. Not only spark.

Copy link
Member

Choose a reason for hiding this comment

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

@FWLamb seatunnel-{enigne}-connector-v2-e2e modules are old implementation, which requires e2e testing for each engine (spark 3.3 and seatunnel engine will be added later), there will be a lot of duplicate code, which is boring and tiring.
seatunnel-connector-v2-e2e unifies the e2e of the connector-v2 and can support the e2e test of multiple engines at the same time.
The status quo is a legacy issue, I will upgrade to a new implementation as soon as possible.

Comment on lines 90 to 97
String initializeSourceTableSql = "CREATE TABLE default.source_table (" +
" `name` Nullable(String)," +
" `age` Nullable(Int32)" +
")ENGINE = Memory";
String initializeSinkTableSql = "CREATE TABLE default.sink_table (" +
" `name` Nullable(String)," +
" `age` Nullable(Int32)" +
")ENGINE = Memory";
Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines 40 to 42
sql {
sql = "select name,age from source_table"
}
Copy link
Member

Choose a reason for hiding this comment

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

remove?

test all supported fields

Comment on lines 52 to 63
@Test
public void testFakeSourceToClickhouseSink() throws IOException, InterruptedException {
Container.ExecResult execResult = executeSeaTunnelSparkJob("/clickhouse/fake_to_clickhouse.conf");
Assertions.assertEquals(0, execResult.getExitCode());
}

@Test
public void testClickhouseSourceToClickhouseSink() throws IOException, InterruptedException {
Container.ExecResult execResult = executeSeaTunnelSparkJob("/clickhouse/clickhouse_to_clickhouse.conf");
Assertions.assertEquals(0, execResult.getExitCode());
Assertions.assertIterableEquals(generateTestDataset(), queryResult());
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@Test
public void testFakeSourceToClickhouseSink() throws IOException, InterruptedException {
Container.ExecResult execResult = executeSeaTunnelSparkJob("/clickhouse/fake_to_clickhouse.conf");
Assertions.assertEquals(0, execResult.getExitCode());
}
@Test
public void testClickhouseSourceToClickhouseSink() throws IOException, InterruptedException {
Container.ExecResult execResult = executeSeaTunnelSparkJob("/clickhouse/clickhouse_to_clickhouse.conf");
Assertions.assertEquals(0, execResult.getExitCode());
Assertions.assertIterableEquals(generateTestDataset(), queryResult());
}
@Test
public void testClickhouseSourceToClickhouseSink() throws IOException, InterruptedException {
Container.ExecResult execResult = executeSeaTunnelSparkJob("/clickhouse/fake_to_clickhouse.conf");
Assertions.assertEquals(0, execResult.getExitCode());
Container.ExecResult execResult = executeSeaTunnelSparkJob("/clickhouse/clickhouse_to_clickhouse.conf");
Assertions.assertEquals(0, execResult.getExitCode());
Assertions.assertIterableEquals(generateTestDataset(), queryResult());
}

merge together

@FWLamb FWLamb requested a review from hailin0 September 30, 2022 06:28
Comment on lines 31 to 34
fields {
name = "string"
age = "int"
}
Copy link
Member

Choose a reason for hiding this comment

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

test all datatypes?

Copy link
Member

@ashulin ashulin left a comment

Choose a reason for hiding this comment

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

Please implement uniform E2E IT;
Note: Spark don‘t support the TIME type currently, so avoid testing it;
You can see #2924 or #2946

</parent>
<modelVersion>4.0.0</modelVersion>

<artifactId>connector-clickhouse-spark-e2e</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

@FWLamb seatunnel-{enigne}-connector-v2-e2e modules are old implementation, which requires e2e testing for each engine (spark 3.3 and seatunnel engine will be added later), there will be a lot of duplicate code, which is boring and tiring.
seatunnel-connector-v2-e2e unifies the e2e of the connector-v2 and can support the e2e test of multiple engines at the same time.
The status quo is a legacy issue, I will upgrade to a new implementation as soon as possible.



@BeforeEach
@SuppressWarnings("checkstyle:MagicNumber")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@SuppressWarnings("checkstyle:MagicNumber")

For classes suffixed with IT or Test, it is automatically suppressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature][Clickhouse-Connector-V2] Add Clickhouse e2e testcase
5 participants