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
FreePort resource to create ServerSocket #733
base: main
Are you sure you want to change the base?
Conversation
…into issue/141-free-port-extension
A couple things that need to be updated still:
|
…into issue/141-free-port-resource
…into issue/141-free-port-resource
…into issue/141-free-port-resource
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from my thought that I might have an understanding issue personally, please add some Javadoc for the class and interface (and maybe answer my personal questions :) )
docs/free-port.adoc
Outdated
to test a failure scenario when there is no service at given port. | ||
|
||
It's a JUnit Jupiter extension which provides a free port for above-mentioned use cases. | ||
Also, there is no guarantee that the port will still be available by the time you use it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe highlight this (port might be not free when used) as a note / warning?
import java.lang.annotation.RetentionPolicy; | ||
import java.lang.annotation.Target; | ||
|
||
@New(FreePort.class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it means that a new instance of the FreePort
class is created, but I would like to double check as I know @New
only as a deprecated CDI annotation (and I'm not getting any other proper search result from google 🆘). This would mean that the user always get a Socket on port zero - which is hardly what they want I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again - NewPort was removed. The @New
annotation is part of our resources abstract extension.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating.
…into issue/141-free-port-resource
I intend to tackle #808 in this one when I have the time, actually. |
Looks good to me, @beatngu13 would you also take a look? @Michael1993 It's still a draft? :D |
Another approach to #141. Based on #541