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

Improve EmbeddedLdapServer Lifecycle Support #1027

Merged

Conversation

etrandafir93
Copy link
Contributor

Closes #1025

@etrandafir93 etrandafir93 force-pushed the dev/1025-unboundid_server_start_stop branch from d1b571a to 41b6afc Compare February 20, 2025 06:50
@jzheaux jzheaux changed the title Implement AutoClosable and add start() / stop() methods for LDAP server Improve LdapEmbeddedServer Lifecycle Support Feb 24, 2025
@jzheaux jzheaux changed the title Improve LdapEmbeddedServer Lifecycle Support Improve EmbeddedLdapServer Lifecycle Support Feb 24, 2025
@jzheaux jzheaux self-assigned this Feb 24, 2025
@jzheaux
Copy link
Collaborator

jzheaux commented Feb 24, 2025

Thanks for the updates, @etrandafir93. Are you able to add tests? Specifically, it would be nice to test calling start() after using the static method and to test calling close().

@etrandafir93 etrandafir93 force-pushed the dev/1025-unboundid_server_start_stop branch from 41b6afc to 8ee70ea Compare February 26, 2025 08:54
Closes spring-projects#1025

Signed-off-by: emanueltrandafir1993 <emanueltrandafir1993@gmail.com>
@etrandafir93 etrandafir93 force-pushed the dev/1025-unboundid_server_start_stop branch from 8ee70ea to d327843 Compare February 26, 2025 09:02
@etrandafir93
Copy link
Contributor Author

Thanks for the updates, @etrandafir93. Are you able to add tests? Specifically, it would be nice to test calling start() after using the static method and to test calling close().

That makes sense @jzheaux - I have added three tests now and I've tried to keep them relatively simple. In terms of asserting, I have only checked whether the port is opened or not. Do you agree with this, or should I try to come up with more elaborate tests?

@jzheaux jzheaux added this to the 3.3.0-M3 milestone Mar 11, 2025
@jzheaux jzheaux merged commit 04facc5 into spring-projects:main Mar 11, 2025
2 checks passed
@jzheaux
Copy link
Collaborator

jzheaux commented Mar 11, 2025

Thanks, @etrandafir93, this is now merged into main.

@jzheaux
Copy link
Collaborator

jzheaux commented Mar 11, 2025

Note that I added two commits. The first was to address checkstyle errors; in the future, you can run ./gradlew checkstyleMain checkstyleTest to find these before opening a PR.

The second was to restrict start from being called twice as I'd prefer to encourage a minimal amount of code in tests. If it becomes necessary to allow this later on, then we can remove the validation.

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.

Add start() and stop() methods for unboundid.EmbeddedLdapServer
2 participants