From 831ed5bfa47b8f91e18099777a6add921c885a80 Mon Sep 17 00:00:00 2001 From: Ewout Pieter den Ouden Date: Thu, 7 Feb 2019 17:53:40 +0100 Subject: [PATCH 1/3] Add test scenario for better @depends warning messages --- tests/_files/DependencyFailureTest.php | 12 ++++++++++++ tests/end-to-end/dependencies-isolation.phpt | 9 ++++++--- tests/end-to-end/dependencies.phpt | 9 ++++++--- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/tests/_files/DependencyFailureTest.php b/tests/_files/DependencyFailureTest.php index 4695305d73d..39a9a379714 100644 --- a/tests/_files/DependencyFailureTest.php +++ b/tests/_files/DependencyFailureTest.php @@ -39,4 +39,16 @@ public function testFour(): void { $this->assertTrue(true); } + + /** + * This test has been added to check the printed warnings for the user + * when a dependency simply doesn't exist. + * + * @depends doesNotExist + * @see https://github.com/sebastianbergmann/phpunit/issues/3517 + */ + public function testHandlesDependsAnnotationForNonexistentTests(): void + { + $this->assertTrue(true); + } } diff --git a/tests/end-to-end/dependencies-isolation.phpt b/tests/end-to-end/dependencies-isolation.phpt index 88c0808b03c..acae771a678 100644 --- a/tests/end-to-end/dependencies-isolation.phpt +++ b/tests/end-to-end/dependencies-isolation.phpt @@ -15,7 +15,7 @@ PHPUnit %s by Sebastian Bergmann and contributors. Runtime: %s -...FSSS 7 / 7 (100%) +...FSSSS 8 / 8 (100%) Time: %s, Memory: %s @@ -27,7 +27,7 @@ There was 1 failure: -- -There were 3 skipped tests: +There were 4 skipped tests: 1) DependencyFailureTest::testTwo This test depends on "DependencyFailureTest::testOne" to pass. @@ -38,5 +38,8 @@ This test depends on "DependencyFailureTest::testTwo" to pass. 3) DependencyFailureTest::testFour This test depends on "DependencyFailureTest::testOne" to pass. +4) DependencyFailureTest::testHandlesDependsAnnotationForNonexistentTests +This test depends on "DependencyFailureTest::doesNotExist" which does not exist + FAILURES! -Tests: 7, Assertions: 4, Failures: 1, Skipped: 3. +Tests: 8, Assertions: 4, Failures: 1, Skipped: 4. diff --git a/tests/end-to-end/dependencies.phpt b/tests/end-to-end/dependencies.phpt index 8fab9884d24..e0b141c2bb9 100644 --- a/tests/end-to-end/dependencies.phpt +++ b/tests/end-to-end/dependencies.phpt @@ -14,7 +14,7 @@ PHPUnit %s by Sebastian Bergmann and contributors. Runtime: %s -...FSSS 7 / 7 (100%) +...FSSSS 8 / 8 (100%) Time: %s, Memory: %s @@ -26,7 +26,7 @@ There was 1 failure: -- -There were 3 skipped tests: +There were 4 skipped tests: 1) DependencyFailureTest::testTwo This test depends on "DependencyFailureTest::testOne" to pass. @@ -37,5 +37,8 @@ This test depends on "DependencyFailureTest::testTwo" to pass. 3) DependencyFailureTest::testFour This test depends on "DependencyFailureTest::testOne" to pass. +4) DependencyFailureTest::testHandlesDependsAnnotationForNonexistentTests +This test depends on "DependencyFailureTest::doesNotExist" which does not exist + FAILURES! -Tests: 7, Assertions: 4, Failures: 1, Skipped: 3. +Tests: 8, Assertions: 4, Failures: 1, Skipped: 4. From 3463568d58bfa14eae9c61ea5e18409d3194e96f Mon Sep 17 00:00:00 2001 From: Ewout Pieter den Ouden Date: Thu, 7 Feb 2019 19:19:33 +0100 Subject: [PATCH 2/3] Uncallable @depends will result in a warning instead of a skipped test --- src/Framework/TestCase.php | 55 ++++++++++++++------ tests/end-to-end/dependencies-isolation.phpt | 16 +++--- tests/end-to-end/dependencies.phpt | 16 +++--- 3 files changed, 59 insertions(+), 28 deletions(-) diff --git a/src/Framework/TestCase.php b/src/Framework/TestCase.php index 0d30afc49ae..262b4619f14 100644 --- a/src/Framework/TestCase.php +++ b/src/Framework/TestCase.php @@ -1723,22 +1723,11 @@ private function handleDependencies(): bool } if (!isset($passedKeys[$dependency])) { - $this->status = BaseTestRunner::STATUS_SKIPPED; - - $this->result->startTest($this); - - $this->result->addError( - $this, - new SkippedTestError( - \sprintf( - 'This test depends on "%s" to pass.', - $dependency - ) - ), - 0 - ); - - $this->result->endTest($this, 0); + if (!is_callable($dependency, false, $callableName) || $dependency !== $callableName) { + $this->markWarningForUncallableDependency($dependency); + } else { + $this->markSkippedForMissingDependecy($dependency); + } return false; } @@ -1777,6 +1766,40 @@ private function handleDependencies(): bool return true; } + private function markSkippedForMissingDependecy(string $dependency): void + { + $this->status = BaseTestRunner::STATUS_SKIPPED; + $this->result->startTest($this); + $this->result->addError( + $this, + new SkippedTestError( + \sprintf( + 'This test depends on "%s" to pass.', + $dependency + ) + ), + 0 + ); + $this->result->endTest($this, 0); + } + + private function markWarningForUncallableDependency(string $dependency): void + { + $this->status = BaseTestRunner::STATUS_WARNING; + $this->result->startTest($this); + $this->result->addWarning( + $this, + new Warning( + \sprintf( + 'This test depends on "%s" which does not exist.', + $dependency + ) + ), + 0 + ); + $this->result->endTest($this, 0); + } + /** * Get the mock object generator, creating it if it doesn't exist. */ diff --git a/tests/end-to-end/dependencies-isolation.phpt b/tests/end-to-end/dependencies-isolation.phpt index acae771a678..fc451a02782 100644 --- a/tests/end-to-end/dependencies-isolation.phpt +++ b/tests/end-to-end/dependencies-isolation.phpt @@ -15,10 +15,17 @@ PHPUnit %s by Sebastian Bergmann and contributors. Runtime: %s -...FSSSS 8 / 8 (100%) +...FSSSW 8 / 8 (100%) Time: %s, Memory: %s +There was 1 warning: + +1) DependencyFailureTest::testHandlesDependsAnnotationForNonexistentTests +This test depends on "DependencyFailureTest::doesNotExist" which does not exist. + +-- + There was 1 failure: 1) DependencyFailureTest::testOne @@ -27,7 +34,7 @@ There was 1 failure: -- -There were 4 skipped tests: +There were 3 skipped tests: 1) DependencyFailureTest::testTwo This test depends on "DependencyFailureTest::testOne" to pass. @@ -38,8 +45,5 @@ This test depends on "DependencyFailureTest::testTwo" to pass. 3) DependencyFailureTest::testFour This test depends on "DependencyFailureTest::testOne" to pass. -4) DependencyFailureTest::testHandlesDependsAnnotationForNonexistentTests -This test depends on "DependencyFailureTest::doesNotExist" which does not exist - FAILURES! -Tests: 8, Assertions: 4, Failures: 1, Skipped: 4. +Tests: 8, Assertions: 4, Failures: 1, Warnings: 1, Skipped: 3. diff --git a/tests/end-to-end/dependencies.phpt b/tests/end-to-end/dependencies.phpt index e0b141c2bb9..e05fdda1dcb 100644 --- a/tests/end-to-end/dependencies.phpt +++ b/tests/end-to-end/dependencies.phpt @@ -14,10 +14,17 @@ PHPUnit %s by Sebastian Bergmann and contributors. Runtime: %s -...FSSSS 8 / 8 (100%) +...FSSSW 8 / 8 (100%) Time: %s, Memory: %s +There was 1 warning: + +1) DependencyFailureTest::testHandlesDependsAnnotationForNonexistentTests +This test depends on "DependencyFailureTest::doesNotExist" which does not exist. + +-- + There was 1 failure: 1) DependencyFailureTest::testOne @@ -26,7 +33,7 @@ There was 1 failure: -- -There were 4 skipped tests: +There were 3 skipped tests: 1) DependencyFailureTest::testTwo This test depends on "DependencyFailureTest::testOne" to pass. @@ -37,8 +44,5 @@ This test depends on "DependencyFailureTest::testTwo" to pass. 3) DependencyFailureTest::testFour This test depends on "DependencyFailureTest::testOne" to pass. -4) DependencyFailureTest::testHandlesDependsAnnotationForNonexistentTests -This test depends on "DependencyFailureTest::doesNotExist" which does not exist - FAILURES! -Tests: 8, Assertions: 4, Failures: 1, Skipped: 4. +Tests: 8, Assertions: 4, Failures: 1, Warnings: 1, Skipped: 3. From bfe08bbb211b8518eca7fe80b33bdcc1ae0afb98 Mon Sep 17 00:00:00 2001 From: Ewout Pieter den Ouden Date: Thu, 7 Feb 2019 19:42:10 +0100 Subject: [PATCH 3/3] CS/WS --- src/Framework/TestCase.php | 2 +- tests/_files/DependencyFailureTest.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Framework/TestCase.php b/src/Framework/TestCase.php index 262b4619f14..ce229c0c40e 100644 --- a/src/Framework/TestCase.php +++ b/src/Framework/TestCase.php @@ -1723,7 +1723,7 @@ private function handleDependencies(): bool } if (!isset($passedKeys[$dependency])) { - if (!is_callable($dependency, false, $callableName) || $dependency !== $callableName) { + if (!\is_callable($dependency, false, $callableName) || $dependency !== $callableName) { $this->markWarningForUncallableDependency($dependency); } else { $this->markSkippedForMissingDependecy($dependency); diff --git a/tests/_files/DependencyFailureTest.php b/tests/_files/DependencyFailureTest.php index 39a9a379714..cec0ddf118d 100644 --- a/tests/_files/DependencyFailureTest.php +++ b/tests/_files/DependencyFailureTest.php @@ -45,6 +45,7 @@ public function testFour(): void * when a dependency simply doesn't exist. * * @depends doesNotExist + * * @see https://github.com/sebastianbergmann/phpunit/issues/3517 */ public function testHandlesDependsAnnotationForNonexistentTests(): void