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

[FLINK-33542] Update tests to JUnit5 #32

Merged
merged 3 commits into from
Nov 17, 2023

Conversation

ferenc-csaky
Copy link
Contributor

Update the existing tests to JUnit5.

Except HBaseTablePlanTest, which is based on TableTestBase from the core Flink repo, that is still on JUnit4.

@ferenc-csaky
Copy link
Contributor Author

@Tan-JiaLiang if you have some time, can you take a look pls?

Copy link
Contributor

@Tan-JiaLiang Tan-JiaLiang left a comment

Choose a reason for hiding this comment

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

Thanks for giving me a chance. I left some comment.

public static final MiniClusterWithClientResource MINI_CLUSTER =
new MiniClusterWithClientResource(
@RegisterExtension
private static final MiniClusterExtension MINI_CLUSTER_EXTENSION =
Copy link
Contributor

Choose a reason for hiding this comment

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

There is moving to HBaseTestingClusterAutoStarter already, maybe we can remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked the commit that added this (FLINK-24077) and it was introduced to fix a flaky test, so I went with the dummy approach to keep it as close to the state it was before. But since this code lives in a separate repo now and the underlying logic with JUnit5 might do not have that defect anyways I think it can be removed. If it introduces test flakiness again, we will know the probable source.

Copy link
Contributor

@Tan-JiaLiang Tan-JiaLiang left a comment

Choose a reason for hiding this comment

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

LGTM!

cc @MartijnVisser

@MartijnVisser MartijnVisser merged commit a36b137 into apache:main Nov 17, 2023
6 checks passed
MartijnVisser pushed a commit that referenced this pull request Nov 17, 2023
* [FLINK-33542] Update tests to JUnit5

* Remove remaining "public" modifiers from test methods

* Remove unnecessary mini cluster extension

(cherry picked from commit a36b137)
@ferenc-csaky ferenc-csaky deleted the FLINK-33542 branch November 17, 2023 10:14
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