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] Miscellaneous improvements on CI robustness #3911

Merged
merged 2 commits into from
Feb 11, 2025

Conversation

yuxiqian
Copy link
Contributor

@yuxiqian yuxiqian commented Feb 6, 2025

This PR mainly focuses on improving test case effectiveness by:

  • Single-stage CI failure should not cause the whole matrix got cancelled
  • Removed vulnerable & unmaintained FastJson from test dependencies
  • Optimize PolarDBX test case by using dynamic port bindings
  • Fixed unstable DataStream migration tests

@yuxiqian
Copy link
Contributor Author

yuxiqian commented Feb 6, 2025

Would @leonardBang and @whhe like to take a look?

@@ -148,7 +149,7 @@ jobs:
maven-version: 3.8.6

- name: Compile and test
timeout-minutes: 90
timeout-minutes: 60
Copy link
Contributor

Choose a reason for hiding this comment

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

[INFO] Running org.apache.flink.cdc.connectors.polardbx.PolardbxCharsetITCase
Error: The action 'Compile and test' has timed out after 60 minutes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PolardbxCharsetITCase was started at ~16:27 and timed out eventually at 16:49. Locally this case wouldn't take more than 3 minutes to finish, so it might be a bug that should be closed by #3902.

"multiline_c": {"wkb":"AQUAAAACAAAAAQIAAAADAAAAAAAAAAAA8D8AAAAAAADwPwAAAAAAAABAAAAAAAAAAEAAAAAAAAAIQAAAAAAAAAhAAQIAAAACAAAAAAAAAAAAEEAAAAAAAAAQQAAAAAAAABRAAAAAAAAAFEA="},
"multipolygon_c": {"wkb":"AQYAAAACAAAAAQMAAAABAAAABQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAkQAAAAAAAAAAAAAAAAAAAJEAAAAAAAAAkQAAAAAAAAAAAAAAAAAAAJEAAAAAAAAAAAAAAAAAAAAAAAQMAAAABAAAABQAAAAAAAAAAABRAAAAAAAAAFEAAAAAAAAAcQAAAAAAAABRAAAAAAAAAHEAAAAAAAAAcQAAAAAAAABRAAAAAAAAAHEAAAAAAAAAUQAAAAAAAABRA"},
"geometrycollection_c": {"wkb":"AQcAAAADAAAAAQEAAAAAAAAAAAAkQAAAAAAAACRAAQEAAAAAAAAAAAA+QAAAAAAAAD5AAQIAAAACAAAAAAAAAAAALkAAAAAAAAAuQAAAAAAAADRAAAAAAAAANEA="}
"point_c": {"x":1.0,"y":1.0,"wkb":"AQEAAAAAAAAAAADwPwAAAAAAAPA/","srid":null},
Copy link
Contributor

Choose a reason for hiding this comment

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

Why "srid":null appears here?

Copy link
Contributor Author

@yuxiqian yuxiqian Feb 6, 2025

Choose a reason for hiding this comment

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

Because FastJSON and Jackson have different comparison rules. FastJSON regards a key with null value as if it's absent, but Jackson is a little more strict.

tools/mig-test/datastream/datastream-3.2.0/pom.xml Outdated Show resolved Hide resolved
@@ -29,7 +29,7 @@ limitations under the License.
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<flink.version>1.19.1</flink.version>
<flink.cdc.version>3.3.0</flink.cdc.version>
<debezium.version>1.9.7.Final</debezium.version>
<debezium.version>1.9.8.Final</debezium.version>
Copy link
Contributor

Choose a reason for hiding this comment

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

We can split the bump version action to seprate commit for tracking

Copy link
Contributor Author

@yuxiqian yuxiqian Feb 6, 2025

Choose a reason for hiding this comment

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

Seems it's a mistake to use 1.9.7.Final, because #3034 has bumped project debezium version to 1.9.8.Final since CDC 3.1.0. I'll split it to another commit.

* migration-test: fix datastream migration failure and simplify migration test matrix

* fix: polardbx test case to use dynamic port mapping

* nit: get rid of fastjson

* build: do not fail fast & adjust timeout to a reasonable duration
Copy link
Contributor

@leonardBang leonardBang left a comment

Choose a reason for hiding this comment

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

Thanks @yuxiqian for the update, LGTM

Copy link
Member

@whhe whhe left a comment

Choose a reason for hiding this comment

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

+1

@leonardBang leonardBang merged commit 82bf8a0 into apache:master Feb 11, 2025
25 of 28 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