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

[ZEPPELIN-5879] Migrate Zeppelin Plugins to JUnit5 #4559

Merged
merged 5 commits into from
Mar 7, 2023

Conversation

Reamer
Copy link
Contributor

@Reamer Reamer commented Feb 1, 2023

What is this PR for?

This pull request migrates the JUnit 4 tests to Junit 5 tests.

What type of PR is it?

Refactoring

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-5879

How should this be tested?

  • CI

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@Reamer Reamer force-pushed the plugins_junit5 branch 2 times, most recently from a6e09a4 to f96d9b5 Compare February 2, 2023 09:23
@huage1994
Copy link
Contributor

Hi @Reamer , this PR is a great job. It all looks good to me.
I just have a very small question about this: I noticed that you removed the public modifier from the @Test method but retain the public modifier for methods that are decorated by @BeforEeach .

Is there any special purpose in this? I've tried to remove public modifier for @BeforEeach methods , it looks ok.

@Reamer
Copy link
Contributor Author

Reamer commented Mar 1, 2023

I just have a very small question about this: I noticed that you removed the public modifier from the @Test method but retain the public modifier for methods that are decorated by @BeforEeach .

Is there any special purpose in this?

There is no deeper meaning. My IDE runs the sonarlint plugin, this plugin gives me a little hint to remove the public modifier.
https://rules.sonarsource.com/java/RSPEC-5786

Unfortunately the hint is missing for @beforeeach and other methods, so I overlooked deleting the modifier at this point.

I will go back through the test classes and remove the visibility.

JUnit5 recommends removing the public modifier.
https://junit.org/junit5/docs/current/user-guide/#writing-tests-classes-and-methods

zeppelin-plugins/notebookrepo/s3/pom.xml Outdated Show resolved Hide resolved
zeppelin-plugins/notebookrepo/s3/pom.xml Outdated Show resolved Hide resolved
zeppelin-plugins/notebookrepo/s3/pom.xml Outdated Show resolved Hide resolved
Co-authored-by: Guanhua Li <guanhua_li@foxmail.com>
@Reamer Reamer merged commit aeb40a5 into apache:master Mar 7, 2023
@Reamer Reamer deleted the plugins_junit5 branch March 7, 2023 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants