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

[hotfix][maxcompute] Fix MaxCompute Pipeline Connector unit tests by upgrading maxcompute-emulator #3862

Merged
merged 3 commits into from
Jan 16, 2025

Conversation

dingxin-tech
Copy link
Contributor

This PR fixes failing unit tests in MaxCompute Pipeline Connector by upgrading the maxcompute-emulator dependency from 0.0.4 to 0.0.7.

Changes

  • Updated maxcompute-emulator version in org.apache.flink.cdc.connectors.maxcompute.EmulatorTestBase

@github-actions github-actions bot added the build label Jan 16, 2025
@dingxin-tech
Copy link
Contributor Author

@yuxiqian Can you have a look for this PR? This is to fix the unit test failures in PR #3839.

@yuxiqian
Copy link
Contributor

yuxiqian commented Jan 16, 2025

Could @leonardBang trigger the CI workflow please?

@yuxiqian
Copy link
Contributor

Seems there's still some timezone related test failures...

org.junit.ComparisonFailure: expected:<...3-10-20,1970-01-01T0[8]:00,1970-01-01T00:00...> but was:<...3-10-20,1970-01-01T0[0]:00,1970-01-01T00:00...>

Could @dingxin-tech please redesign the test case to make it time-zone unaware, and could run with any time-zone configuration? (We may run tests in randomized timezones in the future like #3650.)

@dingxin-tech
Copy link
Contributor Author

Seems there's still some timezone related test failures...

org.junit.ComparisonFailure: expected:<...3-10-20,1970-01-01T0[8]:00,1970-01-01T00:00...> but was:<...3-10-20,1970-01-01T0[0]:00,1970-01-01T00:00...>

Could @dingxin-tech please redesign the test case to make it time-zone unaware, and could run with any time-zone configuration? (We may run tests in randomized timezones in the future like #3650.)

done.

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.

LGTM

@@ -167,7 +167,7 @@ public void testRecordConvert() {
123456.789d,
12345,
12345,
TimestampData.fromTimestamp(Timestamp.from(Instant.ofEpochSecond(0))),
TimestampData.fromTimestamp(Timestamp.valueOf("1970-01-01 00:00:00")),
Copy link
Contributor

@leonardBang leonardBang Jan 16, 2025

Choose a reason for hiding this comment

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

minor: TimestampData.fromLocalDateTime is recommended

@leonardBang leonardBang merged commit 47e5cd6 into apache:master Jan 16, 2025
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