Cleanup

sebastianbergmann · Oct 25, 2019 · f72543f · f72543f
1 parent e0dedce
commit f72543f
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 115 deletions.
diff --git a/...rk/Exception/UnexpectedValueException.php → ...k/Exception/NoChildTestSuiteException.php b/...rk/Exception/UnexpectedValueException.php → ...k/Exception/NoChildTestSuiteException.php
@@ -9,6 +9,6 @@
  */
 namespace PHPUnit\Framework;
 
-final class UnexpectedValueException extends Exception
+final class NoChildTestSuiteException extends Exception
 {
 }
diff --git a/src/Framework/TestSuiteIterator.php b/src/Framework/TestSuiteIterator.php
@@ -9,12 +9,10 @@
  */
 namespace PHPUnit\Framework;
 
-use RecursiveIterator;
-
 /**
  * @internal This class is not covered by the backward compatibility promise for PHPUnit
  */
-final class TestSuiteIterator implements RecursiveIterator
+final class TestSuiteIterator implements \RecursiveIterator
 {
     /**
      * @var int
@@ -31,71 +29,51 @@ public function __construct(TestSuite $testSuite)
         $this->tests = $testSuite->tests();
     }
 
-    /**
-     * Rewinds the Iterator to the first element.
-     */
     public function rewind(): void
     {
         $this->position = 0;
     }
 
-    /**
-     * Checks if there is a current element after calls to rewind() or next().
-     */
     public function valid(): bool
     {
         return $this->position < \count($this->tests);
     }
 
-    /**
-     * Returns the key of the current element.
-     */
     public function key(): int
     {
         return $this->position;
     }
 
-    /**
-     * Returns the current element.
-     */
-    public function current(): ?Test
+    public function current(): Test
     {
-        return $this->valid() ? $this->tests[$this->position] : null;
+        return $this->tests[$this->position];
     }
 
-    /**
-     * Moves forward to next element.
-     */
     public function next(): void
     {
         $this->position++;
     }
 
     /**
-     * Returns the sub iterator for the current element.
-     *
-     * @throws \UnexpectedValueException if the current element is no TestSuite
+     * @throws NoChildTestSuiteException
      */
     public function getChildren(): self
     {
         if (!$this->hasChildren()) {
-            throw new UnexpectedValueException(
-                'The current item is no TestSuite instance and hence cannot have any children.',
-                1567849414
+            throw new NoChildTestSuiteException(
+                'The current item is not a TestSuite instance and therefore does not have any children.'
             );
         }
 
-        /** @var TestSuite $current */
         $current = $this->current();
 
+        \assert($current instanceof TestSuite);
+
         return new self($current);
     }
 
-    /**
-     * Checks whether the current element has children.
-     */
     public function hasChildren(): bool
     {
-        return $this->current() instanceof TestSuite;
+        return $this->valid() && $this->current() instanceof TestSuite;
     }
 }
diff --git a/tests/unit/Framework/TestSuiteIteratorTest.php b/tests/unit/Framework/TestSuiteIteratorTest.php
@@ -14,86 +14,60 @@
  */
 final class TestSuiteIteratorTest extends TestCase
 {
-    /*
-     * tests for the initial state with empty test suite
-     */
-
     public function testKeyForEmptyTestSuiteInitiallyReturnsZero(): void
     {
-        $testSuite = new TestSuite();
+        $testSuite = new TestSuite;
         $subject   = new TestSuiteIterator($testSuite);
 
         $this->assertSame(0, $subject->key());
     }
 
     public function testValidForEmptyTestSuiteInitiallyReturnsFalse(): void
     {
-        $testSuite = new TestSuite();
+        $testSuite = new TestSuite;
         $subject   = new TestSuiteIterator($testSuite);
 
         $this->assertFalse($subject->valid());
     }
 
-    public function testCurrentForEmptyTestSuiteInitiallyReturnsNull(): void
-    {
-        $testSuite = new TestSuite();
-        $subject   = new TestSuiteIterator($testSuite);
-
-        $this->assertNull($subject->current());
-    }
-
-    /*
-     * tests for the initial state with non-empty test suite
-     */
-
     public function testKeyForNonEmptyTestSuiteInitiallyReturnsZero(): void
     {
-        $testSuite = new TestSuite();
-        $testSuite->addTest(new \EmptyTestCaseTest());
-        $subject = new TestSuiteIterator($testSuite);
+        $subject = new TestSuiteIterator($this->suiteWithEmptyTestCase());
 
         $this->assertSame(0, $subject->key());
     }
 
     public function testValidForNonEmptyTestSuiteInitiallyReturnsTrue(): void
     {
-        $testSuite = new TestSuite();
-        $testSuite->addTest(new \EmptyTestCaseTest());
-        $subject = new TestSuiteIterator($testSuite);
+        $subject = new TestSuiteIterator($this->suiteWithEmptyTestCase());
 
         $this->assertTrue($subject->valid());
     }
 
     public function testCurrentForNonEmptyTestSuiteInitiallyReturnsFirstTest(): void
     {
-        $test      = new \EmptyTestCaseTest();
-        $testSuite = new TestSuite();
+        $test      = new \EmptyTestCaseTest;
+        $testSuite = new TestSuite;
         $testSuite->addTest($test);
         $subject = new TestSuiteIterator($testSuite);
 
         $this->assertSame($test, $subject->current());
     }
 
-    /*
-     * tests for rewind
-     */
-
     public function testRewindResetsKeyToZero(): void
     {
-        $testSuite = new TestSuite();
-        $testSuite->addTest(new \EmptyTestCaseTest());
-        $subject = new TestSuiteIterator($testSuite);
-        $subject->next();
+        $subject = new TestSuiteIterator($this->suiteWithEmptyTestCase());
 
+        $subject->next();
         $subject->rewind();
 
         $this->assertSame(0, $subject->key());
     }
 
     public function testRewindResetsCurrentToFirstElement(): void
     {
-        $testSuite = new TestSuite();
-        $test      = new \EmptyTestCaseTest();
+        $testSuite = new TestSuite;
+        $test      = new \EmptyTestCaseTest;
         $testSuite->addTest($test);
         $subject = new TestSuiteIterator($testSuite);
         $subject->next();
@@ -103,75 +77,49 @@ public function testRewindResetsCurrentToFirstElement(): void
         $this->assertSame($test, $subject->current());
     }
 
-    /*
-     * tests for next
-     */
-
     public function testNextIncreasesKeyFromZeroToOne(): void
     {
-        $testSuite = new TestSuite();
-        $testSuite->addTest(new \EmptyTestCaseTest());
-        $subject = new TestSuiteIterator($testSuite);
+        $subject = new TestSuiteIterator($this->suiteWithEmptyTestCase());
 
         $subject->next();
 
         $this->assertSame(1, $subject->key());
     }
 
-    public function testCurrentAfterLastElementReturnsNull(): void
-    {
-        $testSuite = new TestSuite();
-        $testSuite->addTest(new \EmptyTestCaseTest());
-        $subject = new TestSuiteIterator($testSuite);
-
-        $subject->next();
-
-        $this->assertNull($subject->current());
-    }
-
     public function testValidAfterLastElementReturnsFalse(): void
     {
-        $testSuite = new TestSuite();
-        $testSuite->addTest(new \EmptyTestCaseTest());
-        $subject = new TestSuiteIterator($testSuite);
+        $subject = new TestSuiteIterator($this->suiteWithEmptyTestCase());
 
         $subject->next();
 
         $this->assertFalse($subject->valid());
     }
 
-    /*
-     * tests for getChildren
-     */
-
     public function testGetChildrenForEmptyTestSuiteThrowsException(): void
     {
-        $testSuite = new TestSuite();
-        $subject   = new TestSuiteIterator($testSuite);
+        $subject   = new TestSuiteIterator(new TestSuite);
 
-        $this->expectException(UnexpectedValueException::class);
+        $this->expectException(NoChildTestSuiteException::class);
 
         $subject->getChildren();
     }
 
     public function testGetChildrenForCurrentTestThrowsException(): void
     {
-        $testSuite = new TestSuite();
-        $testSuite->addTest(new \EmptyTestCaseTest());
-        $subject = new TestSuiteIterator($testSuite);
+        $subject = new TestSuiteIterator($this->suiteWithEmptyTestCase());
 
-        $this->expectException(UnexpectedValueException::class);
+        $this->expectException(NoChildTestSuiteException::class);
 
         $subject->getChildren();
     }
 
     public function testGetChildrenReturnsNewInstanceWithCurrentTestSuite(): void
     {
-        $childSuite = new TestSuite();
-        $test       = new \EmptyTestCaseTest();
+        $childSuite = new TestSuite;
+        $test       = new \EmptyTestCaseTest;
         $childSuite->addTest($test);
 
-        $testSuite  = new TestSuite();
+        $testSuite  = new TestSuite;
         $testSuite->addTest($childSuite);
 
         $subject = new TestSuiteIterator($testSuite);
@@ -182,14 +130,10 @@ public function testGetChildrenReturnsNewInstanceWithCurrentTestSuite(): void
         $this->assertSame($test, $children->current());
     }
 
-    /*
-     * tests for hasChildren
-     */
-
     public function testHasChildrenForCurrentTestSuiteReturnsTrue(): void
     {
-        $testSuite  = new TestSuite();
-        $childSuite = new TestSuite();
+        $testSuite  = new TestSuite;
+        $childSuite = new TestSuite;
         $testSuite->addTest($childSuite);
         $subject = new TestSuiteIterator($testSuite);
 
@@ -198,19 +142,24 @@ public function testHasChildrenForCurrentTestSuiteReturnsTrue(): void
 
     public function testHasChildrenForCurrentTestReturnsFalse(): void
     {
-        $testSuite = new TestSuite();
-        $test      = new \EmptyTestCaseTest();
-        $testSuite->addTest($test);
-        $subject = new TestSuiteIterator($testSuite);
+        $subject = new TestSuiteIterator($this->suiteWithEmptyTestCase());
 
         $this->assertFalse($subject->hasChildren());
     }
 
     public function testHasChildrenForNoTestsReturnsFalse(): void
     {
-        $testSuite = new TestSuite();
-        $subject   = new TestSuiteIterator($testSuite);
+        $subject = new TestSuiteIterator(new TestSuite);
 
         $this->assertFalse($subject->hasChildren());
     }
+
+    private function suiteWithEmptyTestCase(): TestSuite
+    {
+        $suite = new TestSuite;
+
+        $suite->addTest(new \EmptyTestCaseTest);
+
+        return $suite;
+    }
 }