diff --git a/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/IdentityResourceNotificationTest.java b/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/IdentityResourceNotificationTest.java index c8b0db85ee..445b26cb1e 100644 --- a/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/IdentityResourceNotificationTest.java +++ b/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 diff --git a/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/session/SessionResourceTest.java b/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/session/SessionResourceTest.java index 8a03d8c2dc..825f78d05f 100644 --- a/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/session/SessionResourceTest.java +++ b/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> getGuiceModules(Class 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(); diff --git a/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/session/SessionResourceV2Test.java b/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/session/SessionResourceV2Test.java index e9b3235f17..bcf17294b8 100644 --- a/openam-core-rest/src/test/java/org/forgerock/openam/core/rest/session/SessionResourceV2Test.java +++ b/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);