Make openam-core-rest tests independent of class order for InjectorHolder

openam-core-rest/src/test/java/org/forgerock/openam/core/rest/IdentityResourceNotificationTest.java

@@ -58,8 +58,8 @@
 import org.forgerock.services.context.Context;
 import org.mockito.ArgumentCaptor;
 import org.testng.annotations.AfterMethod;
-import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.BeforeSuite;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
@@ -113,15 +113,30 @@ public class IdentityResourceNotificationTest extends GuiceTestCase {
      * notification broker that is not there - so the first injector of this JVM has to be built from no modules at
      * all, and the bindings a test needs come from {@link #configure(Binder)}.
      *
+     * The loader is read once, while the first injector of the JVM is built, so it is installed before any test of
+     * the suite runs rather than before this class. Any earlier test that touches InjectorHolder with the default
+     * loader - as the session resource tests did while they built a real {@code AMIdentity}, which asks for a
+     * {@code RepoAuditorFactory} and swallows the failure - leaves InjectorHolder unusable for the rest of the JVM,
+     * and a {@code @BeforeClass} here could not undo that. Even this hook does not always come first: TestNG creates
+     * an instance of every test class before it runs any {@code @BeforeSuite} method, so a constructor, field
+     * initializer or static initializer of a test class that reaches InjectorHolder still builds the injector from
+     * the default loader. A {@code mock(IdentityServicesImpl.class)} field is one such initializer, since a static
+     * field of that class asks InjectorHolder for an instance.
+     *
      * The loader is not put back afterwards, and could not be: InjectorConfiguration exposes no way to read the
      * one in place, and the default implementation of it is not public either. It does not matter much, because
-     * the loader is read once, while the first injector of the JVM is built, and after this class has run that
-     * injector exists whatever the loader says. What a later test class in the same fork inherits is therefore an
-     * empty injector - which is what it would have got in this module anyway, since the real modules cannot be
-     * built here at all. A test that needs bindings has to bring them, the way this one does through
-     * {@link GuiceTestCase}, which is also what puts the injector back after every test method.
+     * once the first injector of the JVM exists the loader is never read again. What every other test class of the
+     * suite gets is therefore an empty injector - which is what it would have got in this module anyway, since the
+     * real modules cannot be built here at all. A test that needs bindings has to bring them, the way this one does
+     * through {@link GuiceTestCase}, which is also what puts the injector back after every test method.
+     *
+     * The suite is the whole module only because the module inherits surefire's {@code forkCount=1} and
+     * {@code reuseForks=true}, which run all its test classes in one TestNG suite in one JVM. With
+     * {@code reuseForks=false} or a {@code forkCount} above 1 surefire runs each class in a suite of its own, and a
+     * class run alone - with {@code -Dtest} or from an IDE - runs without this hook. In those runs the hook guards
+     * this class at most, not the others.
      */
-    @BeforeClass
+    @BeforeSuite(alwaysRun = true)
     public void setupGuiceModuleLoader() {
         InjectorConfiguration.setGuiceModuleLoader(new GuiceModuleLoader() {
             @Override

openam-core-rest/src/test/java/org/forgerock/openam/core/rest/session/SessionResourceTest.java

@@ -32,7 +32,6 @@
 import static org.mockito.Mockito.verify;
 import static org.testng.AssertJUnit.*;
 
-import com.google.inject.Module;
 import com.iplanet.services.naming.WebtopNamingQuery;
 import com.iplanet.sso.SSOException;
 import com.iplanet.sso.SSOToken;
@@ -42,7 +41,6 @@
 import com.sun.identity.idm.AMIdentity;
 import com.sun.identity.idm.IdRepoException;
 import com.sun.identity.shared.debug.Debug;
-import java.lang.annotation.Annotation;
 import java.security.Principal;
 import java.util.Arrays;
 import java.util.Collection;
@@ -54,8 +52,6 @@
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 import org.assertj.core.api.Assertions;
-import org.forgerock.guice.core.GuiceModuleLoader;
-import org.forgerock.guice.core.InjectorConfiguration;
 import org.forgerock.http.session.Session;
 import org.forgerock.http.session.SessionContext;
 import org.forgerock.json.JsonValue;
@@ -79,7 +75,6 @@
 import org.forgerock.openam.rest.resource.SSOTokenContext;
 import org.forgerock.openam.session.SessionPropertyWhitelist;
 import org.forgerock.openam.test.apidescriptor.ApiAnnotationAssert;
-import org.forgerock.opendj.ldap.DN;
 import org.forgerock.services.context.AttributesContext;
 import org.forgerock.services.context.ClientContext;
 import org.forgerock.services.context.Context;
@@ -88,7 +83,6 @@
 import org.forgerock.util.promise.Promise;
 import org.mockito.ArgumentCaptor;
 import org.testng.annotations.AfterMethod;
-import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
@@ -114,24 +108,6 @@ public class SessionResourceTest {
 
 
 
-    /**
-     * {@link #setUp()} constructs a real {@link AMIdentity}, whose constructor asks {@link org.forgerock.guice.core.InjectorHolder}
-     * for a {@code RepoAuditorFactory}. If this class is the first in the fork to touch the injector holder, the holder builds
-     * the injector from the modules of the running server, which cannot be built on this test classpath, and the holder
-     * class then stays broken for every later test in the same JVM. Ask for an injector built from no modules instead,
-     * the way {@code IdentityResourceNotificationTest} does; the loader is only consulted while the first injector of the
-     * JVM is built, so this is a no-op when another class got there first.
-     */
-    @BeforeClass
-    public void setupGuiceModuleLoader() {
-        InjectorConfiguration.setGuiceModuleLoader(new GuiceModuleLoader() {
-            @Override
-            public Set<Class<? extends Module>> getGuiceModules(Class<? extends Annotation> moduleAnnotation) {
-                return Collections.emptySet();
-            }
-        });
-    }
-
     @BeforeMethod
     public void setUp() throws Exception {
         ssoTokenManager = mock(SSOTokenManager.class);
@@ -149,7 +125,12 @@ public void setUp() throws Exception {
 
         realmContext = new RealmContext(mockContext, Realm.root());
 
-        amIdentity = new AMIdentity(DN.valueOf("id=demo,dc=example,dc=com"), null);
+        // A mock, not a real AMIdentity: its constructor asks InjectorHolder for a RepoAuditorFactory. If that is the
+        // first call to reach InjectorHolder in the JVM, before any loader is installed, it leaves InjectorHolder
+        // unusable on this classpath for every later test class.
+        amIdentity = mock(AMIdentity.class);
+        given(amIdentity.getName()).willReturn("demo");
+        given(amIdentity.getRealm()).willReturn("dc=example,dc=com");
 
         configureWhitelist();
 

openam-core-rest/src/test/java/org/forgerock/openam/core/rest/session/SessionResourceV2Test.java

@@ -71,7 +71,6 @@
 import org.forgerock.openam.session.SessionPropertyWhitelist;
 import org.forgerock.openam.test.apidescriptor.ApiAnnotationAssert;
 import org.forgerock.openam.utils.CrestQuery;
-import org.forgerock.opendj.ldap.DN;
 import org.forgerock.services.context.Context;
 import org.forgerock.util.promise.Promise;
 import org.forgerock.util.query.QueryFilter;
@@ -109,7 +108,12 @@ public void setUp() throws Exception {
         realmTestHelper = new RealmTestHelper();
         realmTestHelper.setupRealmClass();
 
-        amIdentity = new AMIdentity(DN.valueOf("id=demo,dc=example,dc=com"), null);
+        // A mock, not a real AMIdentity: its constructor asks InjectorHolder for a RepoAuditorFactory. If that is the
+        // first call to reach InjectorHolder in the JVM, before any loader is installed, it leaves InjectorHolder
+        // unusable on this classpath for every later test class.
+        amIdentity = mock(AMIdentity.class);
+        given(amIdentity.getName()).willReturn("demo");
+        given(amIdentity.getRealm()).willReturn("dc=example,dc=com");
 
         sessionService = mock(SessionService.class);
         partialSessionFactory = mock(PartialSessionFactory.class);