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

[tests][ci] Refactor ci #3626

Merged
merged 2 commits into from
Oct 15, 2024
Merged

[tests][ci] Refactor ci #3626

merged 2 commits into from
Oct 15, 2024

Conversation

GOODBOY008
Copy link
Member

No description provided.

@yuxiqian
Copy link
Contributor

Thanks for @GOODBOY008's great work!

A little suggestion: is it possible to split jobs into sub-workflow files to organize them by categories? Current there are a little too much jobs, and most of them has only one sub-task compile_and_test.

截圖 2024-09-30 17 10 48

Copy link
Contributor

@yuxiqian yuxiqian left a comment

Choose a reason for hiding this comment

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

Seems Flink version specification isn't working as expected. Flink 1.20 test job is still running tests on Flink 1.17 (and all other versions).

截圖 2024-09-30 17 24 43

@GOODBOY008 GOODBOY008 force-pushed the master-ci-test branch 2 times, most recently from c3ae751 to 1d0e0b8 Compare September 30, 2024 12:31
@GOODBOY008
Copy link
Member Author

@yuxiqian The pr just for test ci, I will add my commit to your pr.We usually check the failed CI jobs in the CI job view on the PR page, so I don't think it's necessary.
4acccc1b7a8bcb3eddf20b9c640d77ef

Copy link
Contributor

@yuxiqian yuxiqian left a comment

Choose a reason for hiding this comment

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

Looks good.

Categorized folders aren't that important. Could be done later if it takes too much efforts to do now.

flink-cdc-connect/flink-cdc-pipeline-connectors/flink-cdc-pipeline-connector-doris,\
flink-cdc-connect/flink-cdc-pipeline-connectors/flink-cdc-pipeline-connector-starrocks,\
flink-cdc-connect/flink-cdc-pipeline-connectors/flink-cdc-pipeline-connector-kafka,\
flink-cdc-connect/flink-cdc-pipeline-connectors/flink-cdc-pipeline-connector-paimon"
Copy link
Contributor

Choose a reason for hiding this comment

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

Somehow Elasticsearch connector tests have never been added into CI...

@Jiabao-Sun
Copy link
Contributor

Thanks @GOODBOY008 for this great work.
How about using github action's matrix to control flink's version that can improve the coverage of the flink versions and simplify the Parameterized Tests.

@GOODBOY008
Copy link
Member Author

@Jiabao-Sun
Do you mean specifying the Flink version and passing it to run tests using a custom Maven parameter, while the default Flink version corresponds to the project’s dependency version?

I think we can discuss later.

Copy link
Contributor

@Jiabao-Sun Jiabao-Sun left a comment

Choose a reason for hiding this comment

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

@Jiabao-Sun Do you mean specifying the Flink version and passing it to run tests using a custom Maven parameter, while the default Flink version corresponds to the project’s dependency version?

I think we can discuss later.

Yes, but we can do it later.
cc @leonardBang

@GOODBOY008 GOODBOY008 merged commit e825131 into apache:master Oct 15, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants