diff --git a/CHANGES.txt b/CHANGES.txt index d0f2000d2..11a9b7aff 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,7 @@ Current 7.10.0 +Fixed: GITHUB-3048: ConcurrentModificationException when injecting values (Krishnan Mahadevan) +Fixed: GITHUB-3050: Race condition when creating Guice Modules (Krishnan Mahadevan) Fixed: GITHUB-3059: Support the ability to inject custom listener factory (Krishnan Mahadevan) Fixed: GITHUB-3045: IDataProviderListener - beforeDataProviderExecution and afterDataProviderExecution are called twice in special setup (Krishnan Mahadevan) Fixed: GITHUB-3038: java.lang.IllegalStateException: Results per method should NOT have been empty (Krishnan Mahadevan) diff --git a/testng-core/src/main/java/org/testng/internal/objects/GuiceBasedObjectDispenser.java b/testng-core/src/main/java/org/testng/internal/objects/GuiceBasedObjectDispenser.java index 1f036222a..2e6b2585a 100644 --- a/testng-core/src/main/java/org/testng/internal/objects/GuiceBasedObjectDispenser.java +++ b/testng-core/src/main/java/org/testng/internal/objects/GuiceBasedObjectDispenser.java @@ -1,6 +1,7 @@ package org.testng.internal.objects; import com.google.inject.Injector; +import java.util.concurrent.locks.ReentrantLock; import org.testng.ITestContext; import org.testng.annotations.Guice; import org.testng.internal.annotations.AnnotationHelper; @@ -12,6 +13,7 @@ class GuiceBasedObjectDispenser implements IObjectDispenser { private IObjectDispenser dispenser; + private static final ReentrantLock lock = new ReentrantLock(); @Override public void setNextDispenser(IObjectDispenser dispenser) { @@ -20,43 +22,50 @@ public void setNextDispenser(IObjectDispenser dispenser) { @Override public Object dispense(CreationAttributes attributes) { - if (attributes.getBasicAttributes() != null) { - BasicAttributes sa = attributes.getBasicAttributes(); - Class cls = - sa.getTestClass() == null ? sa.getRawClass() : sa.getTestClass().getRealClass(); - if (cannotDispense(cls)) { - return this.dispenser.dispense(attributes); - } - ITestContext ctx = attributes.getContext(); - GuiceContext suiteCtx = attributes.getSuiteContext(); - GuiceHelper helper; - // TODO: remove unused entries from helpers - if (ctx == null) { - helper = new GuiceHelper(suiteCtx); - } else { - helper = (GuiceHelper) ctx.getAttribute(GUICE_HELPER); - if (helper == null) { - helper = new GuiceHelper(ctx); - ctx.setAttribute(GUICE_HELPER, helper); - } - } - Injector injector; - if (ctx == null) { - injector = helper.getInjector(cls, suiteCtx.getInjectorFactory()); - } else { - injector = helper.getInjector(cls, ctx.getInjectorFactory()); - } - if (injector == null) { - return null; - } - if (sa.getRawClass() == null) { - return injector.getInstance(sa.getTestClass().getRealClass()); - } else { - return injector.getInstance(sa.getRawClass()); + if (attributes.getBasicAttributes() == null) { + // We don't have the ability to process object creation with elaborate attributes + return this.dispenser.dispense(attributes); + } + try { + lock.lock(); + return dispenseObject(attributes); + } finally { + lock.unlock(); + } + } + + private Object dispenseObject(CreationAttributes attributes) { + BasicAttributes sa = attributes.getBasicAttributes(); + Class cls = sa.getTestClass() == null ? sa.getRawClass() : sa.getTestClass().getRealClass(); + if (cannotDispense(cls)) { + return this.dispenser.dispense(attributes); + } + ITestContext ctx = attributes.getContext(); + GuiceContext suiteCtx = attributes.getSuiteContext(); + GuiceHelper helper; + // TODO: remove unused entries from helpers + if (ctx == null) { + helper = new GuiceHelper(suiteCtx); + } else { + helper = (GuiceHelper) ctx.getAttribute(GUICE_HELPER); + if (helper == null) { + helper = new GuiceHelper(ctx); + ctx.setAttribute(GUICE_HELPER, helper); } } - // We dont have the ability to process object creation with elaborate attributes - return this.dispenser.dispense(attributes); + Injector injector; + if (ctx == null) { + injector = helper.getInjector(cls, suiteCtx.getInjectorFactory()); + } else { + injector = helper.getInjector(cls, ctx.getInjectorFactory()); + } + if (injector == null) { + return null; + } + if (sa.getRawClass() == null) { + return injector.getInstance(sa.getTestClass().getRealClass()); + } + return injector.getInstance(sa.getRawClass()); } private static boolean cannotDispense(Class clazz) { diff --git a/testng-core/src/test/java/test/guice/GuiceTest.java b/testng-core/src/test/java/test/guice/GuiceTest.java index d3ca70160..2cbe47dae 100644 --- a/testng-core/src/test/java/test/guice/GuiceTest.java +++ b/testng-core/src/test/java/test/guice/GuiceTest.java @@ -6,11 +6,14 @@ import com.google.inject.Injector; import com.google.inject.Module; import com.google.inject.Stage; +import java.util.List; import org.jetbrains.annotations.Nullable; import org.testng.IInjectorFactory; import org.testng.TestNG; import org.testng.annotations.Test; +import org.testng.xml.XmlPackage; import org.testng.xml.XmlSuite; +import org.testng.xml.XmlTest; import test.SimpleBaseTest; import test.guice.issue2343.Person; import test.guice.issue2343.SampleA; @@ -26,6 +29,7 @@ import test.guice.issue2570.GuicePoweredConstructorInjectedRetryForDPTest; import test.guice.issue2570.GuicePoweredSetterInjectedRetry; import test.guice.issue2570.SampleTestClass; +import test.guice.issue3050.EvidenceRetryAnalyzer; public class GuiceTest extends SimpleBaseTest { @@ -108,4 +112,17 @@ public void ensureRetryAnalyzersAreGuiceAware() { .withFailMessage("There should have been 2 retry analyser instances created by Guice") .isEqualTo(2); } + + @Test(description = "GITHUB-3050") + public void ensureNoDuplicateGuiceModulesAreCreated() { + TestNG testng = create(); + XmlSuite xmlSuite = createXmlSuite("evidence_suite"); + xmlSuite.setParallel(XmlSuite.ParallelMode.CLASSES); + xmlSuite.setThreadCount(7); + XmlTest xmlTest = createXmlTest(xmlSuite, "evidence_test"); + xmlTest.setXmlPackages(List.of(new XmlPackage("test.guice.issue3050.*"))); + testng.setXmlSuites(List.of(xmlSuite)); + testng.run(); + assertThat(EvidenceRetryAnalyzer.hasOnlyOneUuid()).isTrue(); + } } diff --git a/testng-core/src/test/java/test/guice/issue3050/EvidenceModule.java b/testng-core/src/test/java/test/guice/issue3050/EvidenceModule.java new file mode 100644 index 000000000..8223de924 --- /dev/null +++ b/testng-core/src/test/java/test/guice/issue3050/EvidenceModule.java @@ -0,0 +1,15 @@ +package test.guice.issue3050; + +import com.google.inject.AbstractModule; +import com.google.inject.Provides; +import com.google.inject.Singleton; +import java.util.UUID; + +public class EvidenceModule extends AbstractModule { + + @Provides + @Singleton + private UUID randomUUID() { + return UUID.randomUUID(); + } +} diff --git a/testng-core/src/test/java/test/guice/issue3050/EvidenceRetryAnalyzer.java b/testng-core/src/test/java/test/guice/issue3050/EvidenceRetryAnalyzer.java new file mode 100644 index 000000000..ea3d1fc60 --- /dev/null +++ b/testng-core/src/test/java/test/guice/issue3050/EvidenceRetryAnalyzer.java @@ -0,0 +1,27 @@ +package test.guice.issue3050; + +import com.google.inject.Inject; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; +import org.testng.IRetryAnalyzer; +import org.testng.ITestResult; +import org.testng.annotations.Guice; + +@Guice(modules = EvidenceModule.class) +public class EvidenceRetryAnalyzer implements IRetryAnalyzer { + + public static Set uuids = ConcurrentHashMap.newKeySet(); + + public static boolean hasOnlyOneUuid() { + return uuids.size() == 1; + } + + @Inject private UUID injectedUUID; + + @Override + public boolean retry(ITestResult result) { + uuids.add(injectedUUID); + return false; + } +} diff --git a/testng-core/src/test/java/test/guice/issue3050/EvidenceTestOneSample.java b/testng-core/src/test/java/test/guice/issue3050/EvidenceTestOneSample.java new file mode 100644 index 000000000..15a76e41f --- /dev/null +++ b/testng-core/src/test/java/test/guice/issue3050/EvidenceTestOneSample.java @@ -0,0 +1,31 @@ +package test.guice.issue3050; + +import static org.testng.Assert.fail; + +import org.testng.ITestContext; +import org.testng.ITestNGMethod; +import org.testng.Reporter; +import org.testng.annotations.BeforeSuite; +import org.testng.annotations.Test; + +@Test +public class EvidenceTestOneSample { + + @BeforeSuite(alwaysRun = true) + public void suiteSetup() { + ITestContext context = Reporter.getCurrentTestResult().getTestContext(); + for (ITestNGMethod method : context.getAllTestMethods()) { + method.setRetryAnalyzerClass(EvidenceRetryAnalyzer.class); + } + } + + @Test + public void testOne() { + fail(); + } + + @Test + public void testTwo() { + fail(); + } +} diff --git a/testng-core/src/test/java/test/guice/issue3050/EvidenceTestThreeSample.java b/testng-core/src/test/java/test/guice/issue3050/EvidenceTestThreeSample.java new file mode 100644 index 000000000..ebcdf5e8d --- /dev/null +++ b/testng-core/src/test/java/test/guice/issue3050/EvidenceTestThreeSample.java @@ -0,0 +1,31 @@ +package test.guice.issue3050; + +import static org.testng.Assert.fail; + +import org.testng.ITestContext; +import org.testng.ITestNGMethod; +import org.testng.Reporter; +import org.testng.annotations.BeforeSuite; +import org.testng.annotations.Test; + +@Test +public class EvidenceTestThreeSample { + + @BeforeSuite(alwaysRun = true) + public void suiteSetup() { + ITestContext context = Reporter.getCurrentTestResult().getTestContext(); + for (ITestNGMethod method : context.getAllTestMethods()) { + method.setRetryAnalyzerClass(EvidenceRetryAnalyzer.class); + } + } + + @Test + public void testOne() { + fail(); + } + + @Test + public void testTwo() { + fail(); + } +} diff --git a/testng-core/src/test/java/test/guice/issue3050/EvidenceTestTwoSample.java b/testng-core/src/test/java/test/guice/issue3050/EvidenceTestTwoSample.java new file mode 100644 index 000000000..6ae9522d6 --- /dev/null +++ b/testng-core/src/test/java/test/guice/issue3050/EvidenceTestTwoSample.java @@ -0,0 +1,31 @@ +package test.guice.issue3050; + +import static org.testng.Assert.fail; + +import org.testng.ITestContext; +import org.testng.ITestNGMethod; +import org.testng.Reporter; +import org.testng.annotations.BeforeSuite; +import org.testng.annotations.Test; + +@Test +public class EvidenceTestTwoSample { + + @BeforeSuite(alwaysRun = true) + public void suiteSetup() { + ITestContext context = Reporter.getCurrentTestResult().getTestContext(); + for (ITestNGMethod method : context.getAllTestMethods()) { + method.setRetryAnalyzerClass(EvidenceRetryAnalyzer.class); + } + } + + @Test + public void testOne() { + fail(); + } + + @Test + public void testTwo() { + fail(); + } +}