diff --git a/CHANGES.txt b/CHANGES.txt index fc9351aeca..03d85128e9 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -8,6 +8,7 @@ Fixed: GITHUB-2709: Testnames not working together with suites in suite (Martin Fixed: GITHUB-2704: IHookable and IConfigurable callback discrepancy (Krishnan Mahadevan) Fixed: GITHUB-2637: Upgrade to JDK11 as the minimum JDK requirements (Krishnan Mahadevan) Fixed: GITHUB-2734: Keep the initial order of listeners (Andrei Solntsev) +Fixed: GITHUB-2359: Testng @BeforeGroups is running in parallel with testcases in the group (Anton Velma) 7.5 Fixed: GITHUB-2701: Bump gradle version to 7.3.3 to support java17 build (ZhangJian He) diff --git a/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java b/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java index 690887a18b..e5f257d3ad 100644 --- a/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java +++ b/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java @@ -1,14 +1,19 @@ package org.testng.internal; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; +import java.util.stream.Collectors; import org.testng.ITestNGMethod; import org.testng.collections.Lists; import org.testng.collections.Maps; +import org.testng.log4testng.Logger; /** * This class wraps access to beforeGroups and afterGroups methods, since they are passed around the @@ -21,10 +26,8 @@ public class ConfigurationGroupMethods { /** The list of beforeGroups methods keyed by the name of the group */ private final Map> m_beforeGroupsMethods; - private final Set beforeGroupsThatHaveAlreadyRun = - Collections.newSetFromMap(new ConcurrentHashMap<>()); - private final Set afterGroupsThatHaveAlreadyRun = - Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final Map beforeGroupsThatHaveAlreadyRun = new ConcurrentHashMap<>(); + private final Set afterGroupsThatHaveAlreadyRun = Collections.newSetFromMap(new ConcurrentHashMap<>()); /** The list of afterGroups methods keyed by the name of the group */ private final Map> m_afterGroupsMethods; @@ -102,9 +105,17 @@ private Map> initializeAfterGroupsMap() { return result; } - public List getBeforeGroupMethodsForGroup(String group) { + public List getBeforeGroupMethodsForGroup(String[] groups) { + if (groups.length == 0) { + return Collections.emptyList(); + } + synchronized (beforeGroupsThatHaveAlreadyRun) { - return retrieve(beforeGroupsThatHaveAlreadyRun, m_beforeGroupsMethods, group); + return Arrays.stream(groups) + .map(t -> retrieve(beforeGroupsThatHaveAlreadyRun, m_beforeGroupsMethods, t)) + .filter(Objects::nonNull) + .flatMap(Collection::stream) + .collect(Collectors.toList()); } } @@ -117,6 +128,7 @@ public List getAfterGroupMethodsForGroup(String group) { public void removeBeforeGroups(String[] groups) { for (String group : groups) { m_beforeGroupsMethods.remove(group); + beforeGroupsThatHaveAlreadyRun.get(group).countDown(); } } @@ -126,6 +138,21 @@ public void removeAfterGroups(Collection groups) { } } + private static List retrieve( + Map tracker, Map> map, String group) { + if (tracker.containsKey(group)) { + try { + tracker.get(group).await(); + } catch (InterruptedException handled) { + Logger.getLogger(ConfigurationGroupMethods.class).error(handled.getMessage(), handled); + Thread.currentThread().interrupt(); + } + return Collections.emptyList(); + } + tracker.put(group, new CountDownLatch(1)); + return map.get(group); + } + private static List retrieve( Set tracker, Map> map, String group) { if (tracker.contains(group)) { diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java index a778b2111d..c9602d43be 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java @@ -111,18 +111,12 @@ public boolean hasConfigurationFailureFor( * @param arguments - A {@link GroupConfigMethodArguments} object. */ public void invokeBeforeGroupsConfigurations(GroupConfigMethodArguments arguments) { - List filteredMethods = Lists.newArrayList(); String[] groups = arguments.getTestMethod().getGroups(); - for (String group : groups) { - List methods = - arguments.getGroupMethods().getBeforeGroupMethodsForGroup(group); - if (methods != null) { - filteredMethods.addAll(methods); - } - } + ITestNGMethod[] beforeMethodsArray = arguments.getGroupMethods() + .getBeforeGroupMethodsForGroup(groups) + .toArray(new ITestNGMethod[0]); - ITestNGMethod[] beforeMethodsArray = filteredMethods.toArray(new ITestNGMethod[0]); // // Invoke the right groups methods // @@ -131,20 +125,19 @@ public void invokeBeforeGroupsConfigurations(GroupConfigMethodArguments argument Arrays.stream(beforeMethodsArray) .filter(ConfigInvoker::isGroupLevelConfigurationMethod) .toArray(ITestNGMethod[]::new); - if (filteredConfigurations.length == 0) { - return; + if (filteredConfigurations.length != 0) { + // don't pass the IClass or the instance as the method may be external + // the invocation must be similar to @BeforeTest/@BeforeSuite + ConfigMethodArguments configMethodArguments = + new Builder() + .usingConfigMethodsAs(filteredConfigurations) + .forSuite(arguments.getSuite()) + .usingParameters(arguments.getParameters()) + .usingInstance(arguments.getInstance()) + .forTestMethod(arguments.getTestMethod()) + .build(); + invokeConfigurations(configMethodArguments); } - // don't pass the IClass or the instance as the method may be external - // the invocation must be similar to @BeforeTest/@BeforeSuite - ConfigMethodArguments configMethodArguments = - new Builder() - .usingConfigMethodsAs(filteredConfigurations) - .forSuite(arguments.getSuite()) - .usingParameters(arguments.getParameters()) - .usingInstance(arguments.getInstance()) - .forTestMethod(arguments.getTestMethod()) - .build(); - invokeConfigurations(configMethodArguments); } // diff --git a/testng-core/src/test/java/test/beforegroups/BeforeGroupsTest.java b/testng-core/src/test/java/test/beforegroups/BeforeGroupsTest.java index c490e9d223..66a60c1026 100644 --- a/testng-core/src/test/java/test/beforegroups/BeforeGroupsTest.java +++ b/testng-core/src/test/java/test/beforegroups/BeforeGroupsTest.java @@ -9,6 +9,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import org.testng.ITestResult; import org.testng.TestListenerAdapter; import org.testng.TestNG; import org.testng.annotations.Test; @@ -26,6 +27,8 @@ import test.beforegroups.issue1694.BaseClassWithBeforeGroups; import test.beforegroups.issue2229.AnotherTestClassSample; import test.beforegroups.issue2229.TestClassSample; +import test.beforegroups.issue2359.ListenerAdapter; +import test.beforegroups.issue2359.SampleFor2359; import test.beforegroups.issue346.SampleTestClass; public class BeforeGroupsTest extends SimpleBaseTest { @@ -100,6 +103,26 @@ public void ensureBeforeGroupsAreInvokedByDefaultEvenWithoutGrouping() { assertThat(AnotherTestClassSample.logs).containsExactlyElementsOf(expectedLogs); } + @Test + public void ensureBeforeGroupIsRunBeforeFirstTestInParallelMethodLaunch() { + TestNG tng = new TestNG(); + + tng.setTestClasses(new Class[] {SampleFor2359.class}); + + // bug only exists when running parallel by instances with this flag set to false + tng.setParallel(XmlSuite.ParallelMode.METHODS); + tng.setThreadCount(3); + + ListenerAdapter adapter = new ListenerAdapter(); + tng.addListener(adapter); + + tng.run(); + + ITestResult beforeGroup = adapter.getPassedConfiguration().iterator().next(); + adapter.getPassedTests().forEach(t -> + assertThat(t.getStartMillis()).isGreaterThanOrEqualTo(beforeGroup.getEndMillis())); + } + private static void createXmlTest(XmlSuite xmlSuite, String name, String group) { XmlTest xmlTest = new XmlTest(xmlSuite); xmlTest.setName(name); diff --git a/testng-core/src/test/java/test/beforegroups/issue2359/ListenerAdapter.java b/testng-core/src/test/java/test/beforegroups/issue2359/ListenerAdapter.java new file mode 100644 index 0000000000..362f74fbb7 --- /dev/null +++ b/testng-core/src/test/java/test/beforegroups/issue2359/ListenerAdapter.java @@ -0,0 +1,21 @@ +package test.beforegroups.issue2359; + +import org.testng.ITestResult; +import org.testng.TestListenerAdapter; +import java.util.Collection; +import java.util.concurrent.ConcurrentLinkedQueue; + +public class ListenerAdapter extends TestListenerAdapter { + + private final Collection passedConfiguration = new ConcurrentLinkedQueue<>(); + + @Override + public void onConfigurationSuccess(ITestResult itr) { + super.onConfigurationSuccess(itr); + this.passedConfiguration.add(itr); + } + + public Collection getPassedConfiguration() { + return passedConfiguration; + } +} diff --git a/testng-core/src/test/java/test/beforegroups/issue2359/SampleFor2359.java b/testng-core/src/test/java/test/beforegroups/issue2359/SampleFor2359.java new file mode 100644 index 0000000000..28bacf8a84 --- /dev/null +++ b/testng-core/src/test/java/test/beforegroups/issue2359/SampleFor2359.java @@ -0,0 +1,32 @@ +package test.beforegroups.issue2359; + +import org.testng.annotations.BeforeGroups; +import org.testng.annotations.Test; +import java.util.concurrent.TimeUnit; + +public class SampleFor2359 { + + @BeforeGroups(groups = "testng2359") + public void beforeGroup() { + try { + TimeUnit.SECONDS.sleep(2); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + @Test(groups = "testng2359") + public void sampleTest1() { + + } + + @Test(groups = "testng2359") + public void sampleTest2() { + + } + + @Test(groups = "testng2359") + public void sampleTest3() { + + } +}