Fix TestNg test runner (com-lihaoyi#3813)

Fixes com-lihaoyi#3798, supersedes
com-lihaoyi#3799

Not sure how this ever worked, but it seems like it works now

Converted the example test in
com-lihaoyi#3799 to a unit test.

---------

Co-authored-by: Tobias Roeser <[email protected]>

build.mill

@@ -771,6 +771,7 @@ object dist0 extends MillPublishJavaModule {
     build.contrib.jmh.testDep(),
     build.contrib.playlib.testDep(),
     build.contrib.playlib.worker("2.8").testDep(),
+    build.contrib.testng.testDep(),
     build.bsp.worker.testDep(),
     build.testkit.testDep()
   )

contrib/testng/src/mill/testng/TestNGRunner.java

@@ -57,17 +57,14 @@ public TestNGRunner(String[] args, String[] remoteArgs, ClassLoader testClassLoa
     }
 
     public Task[] tasks(TaskDef[] taskDefs) {
-        CommandLineArgs cliArgs = new CommandLineArgs();
-        new JCommander(cliArgs).parse(args); // args is an output parameter of the constructor!
-        if(cliArgs.testClass == null){
-            String[] names = new String[taskDefs.length];
-            for(int i = 0; i < taskDefs.length; i += 1){
-                names[i] = taskDefs[i].fullyQualifiedName();
-            }
-            cliArgs.testClass = String.join(",", names);
+        Task[] returnTasks = new Task[taskDefs.length];
+        for(int i = 0; i < taskDefs.length; i += 1){
+            CommandLineArgs cliArgs = new CommandLineArgs();
+            new JCommander(cliArgs).parse(args); // args is an output parameter of the constructor!
+            cliArgs.testClass = taskDefs[i].fullyQualifiedName();
+            returnTasks[i] = new TestNGTask(taskDefs[i], testClassLoader, cliArgs);
         }
-        if (taskDefs.length == 0) return new Task[]{};
-        else return new Task[]{new TestNGTask(taskDefs[0], testClassLoader, cliArgs)};
+        return returnTasks;
     }
 
     public String done() { return null; }

contrib/testng/test/resources/demo/testng/src/foo/HelloTests.java

@@ -0,0 +1,14 @@
+package foo;
+
+import static org.testng.Assert.assertTrue;
+import org.testng.annotations.Test;
+
+public class HelloTests {
+
+  @Test
+  public void hello() throws Exception {
+    System.out.println("Testing Hello");
+    Thread.sleep(1000);
+    System.out.println("Testing Hello Completed");
+  }
+}

contrib/testng/test/resources/demo/testng/src/foo/WorldTests.java

@@ -0,0 +1,13 @@
+package foo;
+
+import static org.testng.Assert.assertTrue;
+import org.testng.annotations.Test;
+
+public class WorldTests {
+  @Test
+  public void world() throws Exception {
+    System.out.println("Testing World");
+    Thread.sleep(1000);
+    System.out.println("Testing World Completed");
+  }
+}

contrib/testng/test/resources/demo/testngGrouping/src/foo/HelloTests.java

@@ -0,0 +1,14 @@
+package foo;
+
+import static org.testng.Assert.assertTrue;
+import org.testng.annotations.Test;
+
+public class HelloTests {
+
+  @Test
+  public void hello() throws Exception {
+    System.out.println("Testing Hello");
+    Thread.sleep(1000);
+    System.out.println("Testing Hello Completed");
+  }
+}

contrib/testng/test/resources/demo/testngGrouping/src/foo/WorldTests.java

@@ -0,0 +1,13 @@
+package foo;
+
+import static org.testng.Assert.assertTrue;
+import org.testng.annotations.Test;
+
+public class WorldTests {
+  @Test
+  public void world() throws Exception {
+    System.out.println("Testing World");
+    Thread.sleep(1000);
+    System.out.println("Testing World Completed");
+  }
+}

contrib/testng/test/src/mill/testng/TestNGTests.scala

@@ -35,27 +35,46 @@ object TestNGTests extends TestSuite {
       }
     }
 
-  }
+    object testng extends JavaTests with TestModule.TestNg {
+      def ivyDeps = super.ivyDeps() ++ Agg(
+        ivy"org.testng:testng:7.10.2"
+      )
+    }
 
+    object testngGrouping extends JavaTests with TestModule.TestNg {
+      def ivyDeps = super.ivyDeps() ++ Agg(
+        ivy"org.testng:testng:7.10.2"
+      )
+      def testForkGrouping = discoveredTestClasses().grouped(1).toSeq
+    }
+  }
   val resourcePath: os.Path = os.Path(sys.env("MILL_TEST_RESOURCE_DIR")) / "demo"
 
   def tests: Tests = Tests {
-    test("TestNG") {
-      test("demo") - UnitTester(demo, resourcePath).scoped { eval =>
-        val Right(result) = eval.apply(demo.test.testFramework)
-        assert(
-          result.value == "mill.testng.TestNGFramework",
-          result.evalCount > 0
-        )
-      }
-      test("Test case lookup from inherited annotations") - UnitTester(demo, resourcePath).scoped {
-        eval =>
-          val Right(result) = eval.apply(demo.test.test())
-          val tres = result.value.asInstanceOf[(String, Seq[mill.testrunner.TestResult])]
-          assert(
-            tres._2.size == 8
-          )
-      }
+    test("demo") - UnitTester(demo, resourcePath).scoped { eval =>
+      val Right(result) = eval.apply(demo.test.testFramework)
+      assert(
+        result.value == "mill.testng.TestNGFramework",
+        result.evalCount > 0
+      )
+    }
+    test("Test case lookup from inherited annotations") - UnitTester(demo, resourcePath).scoped {
+      eval =>
+        val Right(result) = eval.apply(demo.test.test())
+        val tres = result.value
+        assert(tres._2.size == 8)
+    }
+    test("noGrouping") - UnitTester(demo, resourcePath).scoped {
+      eval =>
+        val Right(result) = eval.apply(demo.testng.test())
+        val tres = result.value._2
+        assert(tres.map(_.fullyQualifiedName).toSet == Set("foo.HelloTests", "foo.WorldTests"))
+    }
+    test("testForkGrouping") - UnitTester(demo, resourcePath).scoped {
+      eval =>
+        val Right(result) = eval.apply(demo.testngGrouping.test())
+        val tres = result.value._2
+        assert(tres.map(_.fullyQualifiedName).toSet == Set("foo.HelloTests", "foo.WorldTests"))
     }
   }
 }