Arsnael commented on a change in pull request #483:
URL: https://github.com/apache/james-project/pull/483#discussion_r647083602



##########
File path: 
server/data/data-ldap/src/test/java/org/apache/james/user/ldap/DockerLdapSingleton.java
##########
@@ -34,7 +34,4 @@
         .password(ADMIN_PASSWORD)
         .build();
 
-    static {
-        ldapContainer.start();

Review comment:
       Instead of removing this, I would more likely go to 
`LdapGenericContainer` class and change the start method to:
   
   ```
   public void start() {
           if (!container.isRunning()) {
               container.start();
           }
       }
   ```
   All methods starting the container point to this one. If you do what you 
changed above if you have some tests where you don't start it manually, you 
gonna spend quite some time debugging with the CI to spot and fix them. 
   
   And also with this you are more sure you have only one container running. If 
you call by mistake manually the start twice, then you have the same problem as 
initially with your solution




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to