Use test id instead of test object for group-based filtering

Author: sebastianbergmann

src/Framework/TestSuite.php

@@ -62,7 +62,7 @@ class TestSuite implements IteratorAggregate, Reorderable, SelfDescribing, Test
     private string $name;
 
     /**
-     * @psalm-var array<string,list<Test>>
+     * @psalm-var array<non-empty-string, list<non-empty-string>>
      */
     private array $groups = [];
 
@@ -175,17 +175,25 @@ public function addTest(Test $test, array $groups = []): void
                 $groups[] = 'default';
             }
 
-            foreach ($groups as $group) {
-                if (!isset($this->groups[$group])) {
-                    $this->groups[$group] = [$test];
-                } else {
-                    $this->groups[$group][] = $test;
-                }
-            }
-
             if ($test instanceof TestCase) {
+                $id = $test->valueObjectForEvents()->id();
+
                 $test->setGroups($groups);
             }
+
+            if ($test instanceof PhptTestCase) {
+                $id = $test->valueObjectForEvents()->id();
+            }
+
+            if (isset($id)) {
+                foreach ($groups as $group) {
+                    if (!isset($this->groups[$group])) {
+                        $this->groups[$group] = [$id];
+                    } else {
+                        $this->groups[$group][] = $id;
+                    }
+                }
+            }
         }
     }
 
@@ -310,6 +318,9 @@ public function groups(): array
         );
     }
 
+    /**
+     * @psalm-return array<non-empty-string, list<non-empty-string>>
+     */
     public function groupDetails(): array
     {
         return $this->groups;
@@ -384,22 +395,6 @@ public function run(): void
                     break;
                 }
             }
-
-            if ($test instanceof TestCase || $test instanceof self) {
-                foreach ($test->groups() as $group) {
-                    if (!isset($this->groups[$group])) {
-                        continue;
-                    }
-
-                    foreach (array_keys($this->groups[$group]) as $key) {
-                        if ($test === $this->groups[$group][$key]) {
-                            unset($this->groups[$group][$key]);
-
-                            break;
-                        }
-                    }
-                }
-            }
         }
 
         $this->invokeMethodsAfterLastTest($emitter);

src/Runner/Filter/ExcludeGroupFilterIterator.php

@@ -17,9 +17,10 @@
 final class ExcludeGroupFilterIterator extends GroupFilterIterator
 {
     /**
-     * @psalm-param list<int> $groupTests
+     * @psalm-param non-empty-string $id
+     * @psalm-param list<non-empty-string> $groupTests
      */
-    protected function doAccept(int $id, array $groupTests): bool
+    protected function doAccept(string $id, array $groupTests): bool
     {
         return !in_array($id, $groupTests, true);
     }

src/Runner/Filter/GroupFilterIterator.php

@@ -9,13 +9,13 @@
  */
 namespace PHPUnit\Runner\Filter;
 
-use function array_map;
+use function array_merge;
 use function array_push;
-use function array_values;
 use function in_array;
-use function spl_object_id;
 use PHPUnit\Framework\Test;
+use PHPUnit\Framework\TestCase;
 use PHPUnit\Framework\TestSuite;
+use PHPUnit\Runner\PhptTestCase;
 use RecursiveFilterIterator;
 use RecursiveIterator;
 
@@ -25,7 +25,7 @@
 abstract class GroupFilterIterator extends RecursiveFilterIterator
 {
     /**
-     * @psalm-var list<int>
+     * @psalm-var list<non-empty-string>
      */
     private readonly array $groupTests;
 
@@ -40,17 +40,14 @@ public function __construct(RecursiveIterator $iterator, array $groups, TestSuit
         $groupTests = [];
 
         foreach ($suite->groupDetails() as $group => $tests) {
-            if (in_array((string) $group, $groups, true)) {
-                $testHashes = array_map(
-                    'spl_object_id',
-                    $tests,
-                );
+            if (in_array($group, $groups, true)) {
+                $groupTests = array_merge($groupTests, $tests);
 
-                array_push($groupTests, ...$testHashes);
+                array_push($groupTests, ...$groupTests);
             }
         }
 
-        $this->groupTests = array_values($groupTests);
+        $this->groupTests = $groupTests;
     }
 
     public function accept(): bool
@@ -61,11 +58,16 @@ public function accept(): bool
             return true;
         }
 
-        return $this->doAccept(spl_object_id($test), $this->groupTests);
+        if ($test instanceof TestCase || $test instanceof PhptTestCase) {
+            return $this->doAccept($test->valueObjectForEvents()->id(), $this->groupTests);
+        }
+
+        return true;
     }
 
     /**
-     * @psalm-param list<int> $groupTests
+     * @psalm-param non-empty-string $id
+     * @psalm-param list<non-empty-string> $groupTests
      */
-    abstract protected function doAccept(int $id, array $groupTests): bool;
+    abstract protected function doAccept(string $id, array $groupTests): bool;
 }

src/Runner/Filter/IncludeGroupFilterIterator.php

@@ -17,9 +17,10 @@
 final class IncludeGroupFilterIterator extends GroupFilterIterator
 {
     /**
-     * @psalm-param list<int> $groupTests
+     * @psalm-param non-empty-string $id
+     * @psalm-param list<non-empty-string> $groupTests
      */
-    protected function doAccept(int $id, array $groupTests): bool
+    protected function doAccept(string $id, array $groupTests): bool
     {
         return in_array($id, $groupTests, true);
     }