Honour regex in dependsOnMethods

Closes: #141

CHANGES.txt

@@ -1,7 +1,8 @@
 Current
 New: Added .yml file extension for yaml suite files, previously only .yaml was allowed for yaml (Steven Jubb)
+Fixed: GITHUB-141: regular expression in "dependsOnMethods" does not work (Krishnan Mahadevan)
 Fixed: GITHUB-2770: FileAlreadyExistsException when report is generated (melloware)
-Fixed: GITHUB-2825: Programically Loading TestNG Suite from JAR File Fails to Delete Temporary Copy of Suite File (Steven Jubb)
+Fixed: GITHUB-2825: Programmatically Loading TestNG Suite from JAR File Fails to Delete Temporary Copy of Suite File (Steven Jubb)
 Fixed: GITHUB-2818: Add configuration key for callback discrepancy behavior (Krishnan Mahadevan)
 Fixed: GITHUB-2819: Ability to retry a data provider in case of failures (Krishnan Mahadevan)
 Fixed: GITHUB-2308: StringIndexOutOfBoundsException in findClassesInPackage - Surefire/Maven - JDK 11 fails (Krishnan Mahadevan)

testng-core/src/main/java/org/testng/DependencyMap.java

@@ -1,8 +1,6 @@
 package org.testng;
 
 import java.util.List;
-import java.util.Optional;
-import java.util.Set;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import org.testng.collections.ListMultiMap;
@@ -24,36 +22,39 @@ public DependencyMap(ITestNGMethod[] methods) {
   }
 
   public List<ITestNGMethod> getMethodsThatBelongTo(String group, ITestNGMethod fromMethod) {
-    Set<String> uniqueKeys = m_groups.keySet();
     Pattern pattern = Pattern.compile(group);
 
     List<ITestNGMethod> result =
-        m_groups.keySet().stream()
-            .parallel()
-            .filter(k -> pattern.matcher(k).matches())
-            .flatMap(k -> m_groups.get(k).stream())
+        m_groups
+            .entrySet()
+            .parallelStream()
+            .filter(entry -> pattern.matcher(entry.getKey()).matches())
+            .flatMap(entry -> entry.getValue().stream())
             .collect(Collectors.toList());
 
-    for (String k : uniqueKeys) {
-      if (Pattern.matches(group, k)) {
-        result.addAll(m_groups.get(k));
-      }
-    }
-
     if (result.isEmpty() && !fromMethod.ignoreMissingDependencies()) {
       throw new TestNGException(
           "DependencyMap::Method \""
               + fromMethod
               + "\" depends on nonexistent group \""
               + group
               + "\"");
-    } else {
-      return result;
     }
+    return result;
   }
 
   public ITestNGMethod getMethodDependingOn(String methodName, ITestNGMethod fromMethod) {
     List<ITestNGMethod> l = m_dependencies.get(methodName);
+    if (l.isEmpty()) {
+      Pattern pattern = Pattern.compile(methodName);
+      l =
+          m_dependencies
+              .entrySet()
+              .parallelStream()
+              .filter(entry -> methodNameMatch(pattern, entry.getKey()))
+              .flatMap(entry -> entry.getValue().stream())
+              .collect(Collectors.toList());
+    }
     if (l.isEmpty()) {
       // Try to fetch dependencies by using the test class in the method name.
       // This is usually needed in scenarios wherein a child class overrides a base class method.
@@ -64,26 +65,32 @@ public ITestNGMethod getMethodDependingOn(String methodName, ITestNGMethod fromM
     if (l.isEmpty() && fromMethod.ignoreMissingDependencies()) {
       return fromMethod;
     }
-    Optional<ITestNGMethod> found =
-        l.stream()
-            .parallel()
-            .filter(
-                m ->
-                    isSameInstance(fromMethod, m)
-                        || belongToDifferentClassHierarchy(fromMethod, m)
-                        || hasInstance(fromMethod, m))
-            .findFirst();
-    if (found.isPresent()) {
-      return found.get();
-    }
+    return l.parallelStream()
+        .filter(
+            m ->
+                isSameInstance(fromMethod, m)
+                    || belongToDifferentClassHierarchy(fromMethod, m)
+                    || hasInstance(fromMethod, m))
+        .findFirst()
+        .orElseThrow(
+            () ->
+                new TestNGException(
+                    "Method \""
+                        + fromMethod
+                        + "\" depends on nonexistent method \""
+                        + methodName
+                        + "\""));
+  }
 
-    throw new TestNGException(
-        "Method \"" + fromMethod + "\" depends on nonexistent method \"" + methodName + "\"");
+  private static boolean methodNameMatch(Pattern pattern, String method) {
+    String justMethodName = method.substring(method.lastIndexOf(".") + 1);
+    return pattern.matcher(justMethodName).matches();
   }
 
   private static boolean belongToDifferentClassHierarchy(
       ITestNGMethod baseClassMethod, ITestNGMethod derivedClassMethod) {
-    return !baseClassMethod.getRealClass().isAssignableFrom(derivedClassMethod.getRealClass());
+    Class<?> left = baseClassMethod.getRealClass();
+    return !left.isAssignableFrom(derivedClassMethod.getRealClass());
   }
 
   private static boolean hasInstance(

testng-core/src/test/java/test/dependent/DependentTest.java

@@ -21,11 +21,60 @@
 import test.dependent.github1380.GitHub1380Sample2;
 import test.dependent.github1380.GitHub1380Sample3;
 import test.dependent.github1380.GitHub1380Sample4;
+import test.dependent.issue141.MultipleMatchesTestClassSample;
+import test.dependent.issue141.NestedTestClassSample;
+import test.dependent.issue141.SkipReasoner;
+import test.dependent.issue141.TestClassSample;
 import test.dependent.issue2658.FailingClassSample;
 import test.dependent.issue2658.PassingClassSample;
 
 public class DependentTest extends SimpleBaseTest {
 
+  @Test(description = "GITHUB-141")
+  public void ensureDependsOnMethodsHonoursRegexPatternsUniqueMatch() {
+    TestNG testng = create(TestClassSample.class);
+    MethodNameCollector listener = new MethodNameCollector();
+    testng.addListener(listener);
+    testng.run();
+    assertThat(listener.getPassedNames()).containsExactly("test_C6390323", "randomTest");
+  }
+
+  @Test(description = "GITHUB-141")
+  public void ensureDependsOnMethodsHonoursRegexPatternsDuplicateMatches() {
+    TestNG testng = create(MultipleMatchesTestClassSample.class);
+    MethodNameCollector listener = new MethodNameCollector();
+    SkipReasoner reasoner = new SkipReasoner();
+    testng.addListener(listener);
+    testng.addListener(reasoner);
+    testng.run();
+    assertThat(listener.getPassedNames()).containsExactly("test_C6390324");
+    assertThat(listener.getFailedNames()).containsExactly("test_C6390323");
+    assertThat(listener.getSkippedNames()).containsExactly("randomTest");
+    assertThat(reasoner.getUpstreamFailures()).containsExactly("test_C6390323");
+  }
+
+  @Test(description = "GITHUB-141")
+  public void ensureDependsOnMethodsHonoursRegexPatternsDuplicateMatchesNestedClasses() {
+    TestNG testng = create(NestedTestClassSample.class);
+    MethodNameCollector listener = new MethodNameCollector();
+    SkipReasoner reasoner = new SkipReasoner();
+    testng.addListener(listener);
+    testng.addListener(reasoner);
+    testng.run();
+    String clazzName =
+        NestedTestClassSample.class.getCanonicalName()
+            + "$"
+            + NestedTestClassSample.SecondSample.class.getSimpleName();
+    assertThat(listener.getFailedInstances()).hasSize(1);
+    assertThat(listener.getFailedInstances().get(0)).contains(clazzName);
+
+    assertThat(listener.getSkippedInstances()).hasSize(1);
+    assertThat(listener.getSkippedInstances().get(0)).contains(clazzName);
+
+    assertThat(listener.getFailedNames()).containsExactly("test_C6390323");
+    assertThat(listener.getSkippedNames()).containsExactly("randomTest");
+  }
+
   @Test
   public void simpleSkip() {
     TestNG testng = create(SampleDependent1.class);

testng-core/src/test/java/test/dependent/issue141/MultipleMatchesTestClassSample.java

@@ -0,0 +1,18 @@
+package test.dependent.issue141;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+public class MultipleMatchesTestClassSample {
+
+  @Test(dependsOnMethods = "test_C[0-9]{7}")
+  public void randomTest() {}
+
+  @Test
+  public void test_C6390323() {
+    Assert.fail();
+  }
+
+  @Test
+  public void test_C6390324() {}
+}

testng-core/src/test/java/test/dependent/issue141/NestedTestClassSample.java

@@ -0,0 +1,25 @@
+package test.dependent.issue141;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+public class NestedTestClassSample {
+
+  public static class FirstSample {
+    @Test(dependsOnMethods = "test_C[0-9]{7}")
+    public void randomTest() {}
+
+    @Test
+    public void test_C6390323() {}
+  }
+
+  public static class SecondSample {
+    @Test(dependsOnMethods = "test_C[0-9]{7}")
+    public void randomTest() {}
+
+    @Test
+    public void test_C6390323() {
+      Assert.fail();
+    }
+  }
+}

testng-core/src/test/java/test/dependent/issue141/SkipReasoner.java

@@ -0,0 +1,24 @@
+package test.dependent.issue141;
+
+import java.util.List;
+import java.util.stream.Collectors;
+import org.testng.ITestListener;
+import org.testng.ITestNGMethod;
+import org.testng.ITestResult;
+
+public class SkipReasoner implements ITestListener {
+
+  private List<String> upstreamFailures;
+
+  @Override
+  public void onTestSkipped(ITestResult result) {
+    upstreamFailures =
+        result.getSkipCausedBy().stream()
+            .map(ITestNGMethod::getQualifiedName)
+            .collect(Collectors.toList());
+  }
+
+  public List<String> getUpstreamFailures() {
+    return upstreamFailures;
+  }
+}

testng-core/src/test/java/test/dependent/issue141/TestClassSample.java

@@ -0,0 +1,12 @@
+package test.dependent.issue141;
+
+import org.testng.annotations.Test;
+
+public class TestClassSample {
+
+  @Test(dependsOnMethods = "test_C[0-9]{7}")
+  public void randomTest() {}
+
+  @Test
+  public void test_C6390323() {}
+}