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

Work in progress: JUnit 5 migration #227

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DamnedElric
Copy link

This is not done yet, it's currently intended as a proof of concept for migrating tests to JUnit 5

@garydgregory
Copy link
Member

Hi @DamnedElric
What is the status?

@DamnedElric
Copy link
Author

DamnedElric commented Mar 16, 2024

Hi @garydgregory ,

I'll quote what I said on the mailing list, but the TL;DR; is that I'm awaiting feedback, and that I'll try to make some more progress.

Following Rob's suggestion, I migrated FTPSClientTest from @parameterized to @nested

The result so far can be found here: b14ee39

Basically the test methods were moved from FTPSClientTest.java [1] to AbstractFtpsTest.java [2]. The two @parameterized values are then injected using @nested.

The behaviour in maven is exactly the same as before. The behaviour in Eclipse bugs me a little, instead of running both nested tests by default, it asks which one should be executed. But that seems to be the case with @nested tests in general.

[1] https://github.com/apache/commons-net/blob/b14ee39cca486bda106758a48d31c91ad52d0d83/src/test/java/org/apache/commons/net/ftp/FTPSClientTest.java

[2] https://github.com/apache/commons-net/blob/b14ee39cca486bda106758a48d31c91ad52d0d83/src/test/java/org/apache/commons/net/ftp/AbstractFtpsTest.java

Any feedback is welcome. I personally feel like this test case is a bit too complex to be easily grokked, and perhaps it can be improved a bit more after the JUnit migration.

@garydgregory
Copy link
Member

garydgregory commented May 4, 2024

Hello @DamnedElric
Yeah, I don't use the Nested feature. Eclipse behavior is important to me, I want that to work as well as it can...

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.

2 participants