Skip to content

[test] close jdbc and client after each test#2438

Merged
ruanhang1993 merged 4 commits intoapache:masterfrom
loserwang1024:close-close-after-test
Sep 26, 2023
Merged

[test] close jdbc and client after each test#2438
ruanhang1993 merged 4 commits intoapache:masterfrom
loserwang1024:close-close-after-test

Conversation

@loserwang1024
Copy link
Copy Markdown
Contributor

In mysql and postgres test unit, the connections to database and Flink cluster are not closed after test.
This connections may influence other test unit.

Copy link
Copy Markdown
Member

@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.

Thanks @loserwang1024, I left some comments.

assertEqualsInAnyOrder(
Arrays.asList(binlogExpected), fetchRows(iterator, binlogExpected.length));
result.getJobClient().ifPresent(client -> client.cancel());
result.getJobClient().get().cancel().get();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Occasionally Db2 ITCase will fail because of trying to cancel a job which isn't running (#2345). Will same thing happen here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your advice, @yuxiqian . As an unbounded streaming job, this job should not stop before can it.

To be honest, I am a little confused that in db2 ItCase, why you skip job when a job isn't running. Does it a bounded job? If not, isn't it a wrong bahavior?

@loserwang1024
Copy link
Copy Markdown
Contributor Author

@yuxiqian , CC. I have modified it

Copy link
Copy Markdown
Member

@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.

+1

assertFalse(state.contains("server_id"));
assertFalse(state.contains("event"));

source.cancel();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could add a new method to help to close DebeziumSourceFunction right.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have noticed that source.close contains source.cancel, so I remove all the source.cancel before source.close

Copy link
Copy Markdown
Contributor

@ruanhang1993 ruanhang1993 left a comment

Choose a reason for hiding this comment

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

LGTM.

@ruanhang1993 ruanhang1993 merged commit a8fb5b0 into apache:master Sep 26, 2023
e-mhui pushed a commit to e-mhui/flink-cdc-connectors that referenced this pull request Oct 18, 2023
ChaomingZhangCN pushed a commit to ChaomingZhangCN/flink-cdc that referenced this pull request Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants