From a1362315039abc3c7f9f173854379266b8d2cc4f Mon Sep 17 00:00:00 2001 From: Ariful Alam Date: Tue, 20 Feb 2024 18:24:37 +0600 Subject: [PATCH 01/10] fix: modify `Result::exitCode` logic to address warning handling with `--fail-on-warning` --- src/Result.php | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/Result.php b/src/Result.php index 9b3a83e5..98e9e8b6 100644 --- a/src/Result.php +++ b/src/Result.php @@ -40,9 +40,20 @@ final class Result */ public static function exitCode(Configuration $configuration, TestResult $result): int { - if ($result->wasSuccessfulIgnoringPhpunitWarnings() - && ! $result->hasTestTriggeredPhpunitWarningEvents()) { - return self::SUCCESS_EXIT; + if ($result->wasSuccessfulIgnoringPhpunitWarnings()) { + if ($configuration->failOnWarning()) { + $warnings = $result->numberOfTestsWithTestTriggeredPhpunitWarningEvents() + + count($result->warnings()) + + count($result->phpWarnings()); + + if ($warnings > 0) { + return self::FAILURE_EXIT; + } + } + + if (! $result->hasTestTriggeredPhpunitWarningEvents()) { + return self::SUCCESS_EXIT; + } } if ($configuration->failOnEmptyTestSuite() && ResultReflection::numberOfTests($result) === 0) { @@ -54,14 +65,6 @@ final class Result $returnCode = self::FAILURE_EXIT; } - $warnings = $result->numberOfTestsWithTestTriggeredPhpunitWarningEvents() - + count($result->warnings()) - + count($result->phpWarnings()); - - if ($configuration->failOnWarning() && $warnings > 0) { - $returnCode = self::FAILURE_EXIT; - } - if ($configuration->failOnIncomplete() && $result->hasTestMarkedIncompleteEvents()) { $returnCode = self::FAILURE_EXIT; } From 1b64fef7bab72d4ec39b63ea40b340aa81a4cda0 Mon Sep 17 00:00:00 2001 From: faissaloux Date: Wed, 21 Feb 2024 16:58:40 +0100 Subject: [PATCH 02/10] fix toUseStrictTypes --- src/Expectation.php | 2 +- src/Expectations/OppositeExpectation.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Expectation.php b/src/Expectation.php index a410cf1d..fcc0b7e9 100644 --- a/src/Expectation.php +++ b/src/Expectation.php @@ -441,7 +441,7 @@ final class Expectation { return Targeted::make( $this, - fn (ObjectDescription $object): bool => str_contains((string) file_get_contents($object->path), 'declare(strict_types=1);'), + fn (ObjectDescription $object): bool => (bool) preg_match('/^<\?php\s+declare\(.*?strict_types\s?=\s?1.*?\);/', (string) file_get_contents($object->path)), 'to use strict types', FileLineFinder::where(fn (string $line): bool => str_contains($line, 'original, - fn (ObjectDescription $object): bool => ! str_contains((string) file_get_contents($object->path), 'declare(strict_types=1);'), + fn (ObjectDescription $object): bool => ! preg_match('/^<\?php\s+declare\(.*?strict_types\s?=\s?1.*?\);/', (string) file_get_contents($object->path)), 'not to use strict types', FileLineFinder::where(fn (string $line): bool => str_contains($line, ' Date: Wed, 21 Feb 2024 17:09:16 +0100 Subject: [PATCH 03/10] fix bool type --- src/Expectations/OppositeExpectation.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Expectations/OppositeExpectation.php b/src/Expectations/OppositeExpectation.php index aa7edcb8..07284e1f 100644 --- a/src/Expectations/OppositeExpectation.php +++ b/src/Expectations/OppositeExpectation.php @@ -84,7 +84,7 @@ final class OppositeExpectation { return Targeted::make( $this->original, - fn (ObjectDescription $object): bool => ! preg_match('/^<\?php\s+declare\(.*?strict_types\s?=\s?1.*?\);/', (string) file_get_contents($object->path)), + fn (ObjectDescription $object): bool => ! (bool) preg_match('/^<\?php\s+declare\(.*?strict_types\s?=\s?1.*?\);/', (string) file_get_contents($object->path)), 'not to use strict types', FileLineFinder::where(fn (string $line): bool => str_contains($line, ' Date: Sun, 17 Mar 2024 12:04:38 +0000 Subject: [PATCH 04/10] Add static closure check --- src/Factories/TestCaseMethodFactory.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Factories/TestCaseMethodFactory.php b/src/Factories/TestCaseMethodFactory.php index 341ec902..4cbc8530 100644 --- a/src/Factories/TestCaseMethodFactory.php +++ b/src/Factories/TestCaseMethodFactory.php @@ -90,6 +90,11 @@ final class TestCaseMethodFactory throw ShouldNotHappen::fromMessage('Description can not be empty.'); } + if (($reflection = new ReflectionClosure($this->closure))->isStatic()) { + $fileAndLine = $reflection->getFileName() . ':' . $reflection->getStartLine(); + throw ShouldNotHappen::fromMessage("The test `$this->description` closure must not be static in $fileAndLine."); + } + $closure = $this->closure; $testCase = TestSuite::getInstance()->tests->get($this->filename); From a4f8ae1a12cc3f22b50b387cdf71e2ae7fb760d2 Mon Sep 17 00:00:00 2001 From: Peter Fox Date: Sun, 17 Mar 2024 16:48:43 +0000 Subject: [PATCH 05/10] Handles tests where a static closure is provided --- composer.json | 1 + src/Exceptions/TestClosureMustNotBeStatic.php | 16 ++++++++++++++++ src/Factories/TestCaseFactory.php | 9 +++++++++ src/Factories/TestCaseMethodFactory.php | 5 ----- tests/Arch.php | 1 + tests/Unit/TestSuite.php | 17 +++++++++++++++++ 6 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 src/Exceptions/TestClosureMustNotBeStatic.php diff --git a/composer.json b/composer.json index db105652..7fec7cf4 100644 --- a/composer.json +++ b/composer.json @@ -19,6 +19,7 @@ "require": { "php": "^8.1.0", "brianium/paratest": "^7.3.1", + "laravel/serializable-closure": "^1.0", "nunomaduro/collision": "^7.10.0|^8.1.1", "nunomaduro/termwind": "^1.15.1|^2.0.1", "pestphp/pest-plugin": "^2.1.1", diff --git a/src/Exceptions/TestClosureMustNotBeStatic.php b/src/Exceptions/TestClosureMustNotBeStatic.php new file mode 100644 index 00000000..498b1913 --- /dev/null +++ b/src/Exceptions/TestClosureMustNotBeStatic.php @@ -0,0 +1,16 @@ +filename, $method->description); } + if ( + $method->closure instanceof \Closure && + (new ReflectionClosure($method->closure))->isStatic() + ) { + throw new TestClosureMustNotBeStatic("The test `$method->description` closure must not be static in $method->filename."); + } + if (! $method->receivesArguments()) { if (! $method->closure instanceof \Closure) { throw ShouldNotHappen::fromMessage('The test closure may not be empty.'); diff --git a/src/Factories/TestCaseMethodFactory.php b/src/Factories/TestCaseMethodFactory.php index 4cbc8530..341ec902 100644 --- a/src/Factories/TestCaseMethodFactory.php +++ b/src/Factories/TestCaseMethodFactory.php @@ -90,11 +90,6 @@ final class TestCaseMethodFactory throw ShouldNotHappen::fromMessage('Description can not be empty.'); } - if (($reflection = new ReflectionClosure($this->closure))->isStatic()) { - $fileAndLine = $reflection->getFileName() . ':' . $reflection->getStartLine(); - throw ShouldNotHappen::fromMessage("The test `$this->description` closure must not be static in $fileAndLine."); - } - $closure = $this->closure; $testCase = TestSuite::getInstance()->tests->get($this->filename); diff --git a/tests/Arch.php b/tests/Arch.php index 7c508cf2..d9bcea4c 100644 --- a/tests/Arch.php +++ b/tests/Arch.php @@ -22,6 +22,7 @@ arch('dependencies') 'Whoops', 'Symfony\Component\Console', 'Symfony\Component\Process', + 'Laravel\SerializableClosure\Support\ReflectionClosure', ])->ignoring(['Composer', 'PHPUnit', 'SebastianBergmann']); arch('contracts') diff --git a/tests/Unit/TestSuite.php b/tests/Unit/TestSuite.php index eb033d0c..3efd964c 100644 --- a/tests/Unit/TestSuite.php +++ b/tests/Unit/TestSuite.php @@ -2,6 +2,7 @@ use Pest\Exceptions\DatasetMissing; use Pest\Exceptions\TestAlreadyExist; +use Pest\Exceptions\TestClosureMustNotBeStatic; use Pest\Factories\TestCaseMethodFactory; use Pest\TestSuite; @@ -16,6 +17,22 @@ it('does not allow to add the same test description twice', function () { sprintf('A test with the description `%s` already exists in the filename `%s`.', 'bar', 'foo'), ); +it('does not allow static closures', function () { + $testSuite = new TestSuite(getcwd(), 'tests'); + $method = new TestCaseMethodFactory('foo', 'bar', static function () { + + }); + + $testSuite->tests->set($method); +})->throws( + TestClosureMustNotBeStatic::class, + sprintf( + 'The test `%s` closure must not be static in %s.', + 'bar', + 'foo', + ) +); + it('alerts users about tests with arguments but no input', function () { $testSuite = new TestSuite(getcwd(), 'tests'); From 0ccbe5c8f0b886d3af5864ed3adb263f50984e0a Mon Sep 17 00:00:00 2001 From: Peter Fox Date: Sun, 17 Mar 2024 17:23:17 +0000 Subject: [PATCH 06/10] Remove Laravel serialisable closure --- composer.json | 1 - src/Factories/TestCaseFactory.php | 3 +-- tests/Arch.php | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/composer.json b/composer.json index 7fec7cf4..db105652 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,6 @@ "require": { "php": "^8.1.0", "brianium/paratest": "^7.3.1", - "laravel/serializable-closure": "^1.0", "nunomaduro/collision": "^7.10.0|^8.1.1", "nunomaduro/termwind": "^1.15.1|^2.0.1", "pestphp/pest-plugin": "^2.1.1", diff --git a/src/Factories/TestCaseFactory.php b/src/Factories/TestCaseFactory.php index 8d91f986..73ba8321 100644 --- a/src/Factories/TestCaseFactory.php +++ b/src/Factories/TestCaseFactory.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Pest\Factories; -use Laravel\SerializableClosure\Support\ReflectionClosure; use ParseError; use Pest\Concerns; use Pest\Contracts\AddsAnnotations; @@ -220,7 +219,7 @@ final class TestCaseFactory if ( $method->closure instanceof \Closure && - (new ReflectionClosure($method->closure))->isStatic() + (new \ReflectionFunction($method->closure))->isStatic() ) { throw new TestClosureMustNotBeStatic("The test `$method->description` closure must not be static in $method->filename."); } diff --git a/tests/Arch.php b/tests/Arch.php index d9bcea4c..7c508cf2 100644 --- a/tests/Arch.php +++ b/tests/Arch.php @@ -22,7 +22,6 @@ arch('dependencies') 'Whoops', 'Symfony\Component\Console', 'Symfony\Component\Process', - 'Laravel\SerializableClosure\Support\ReflectionClosure', ])->ignoring(['Composer', 'PHPUnit', 'SebastianBergmann']); arch('contracts') From adb2fb51dfdd7aa641efc445dc0aa9ddbabc2b11 Mon Sep 17 00:00:00 2001 From: Tom Harper Date: Mon, 8 Apr 2024 10:03:55 +0100 Subject: [PATCH 07/10] Always call parent teardown even if an exception is thrown --- src/Concerns/Testable.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Concerns/Testable.php b/src/Concerns/Testable.php index 4075caf4..4c0250d3 100644 --- a/src/Concerns/Testable.php +++ b/src/Concerns/Testable.php @@ -234,11 +234,13 @@ trait Testable $afterEach = ChainableClosure::bound($this->__afterEach, $afterEach); } - $this->__callClosure($afterEach, func_get_args()); + try { + $this->__callClosure($afterEach, func_get_args()); + } finally { + parent::tearDown(); - parent::tearDown(); - - TestSuite::getInstance()->test = null; + TestSuite::getInstance()->test = null; + } } /** From 748beb17d5028d8fc65465000e63c1bec620ecec Mon Sep 17 00:00:00 2001 From: Nuno Maduro Date: Thu, 22 Aug 2024 20:59:00 +0100 Subject: [PATCH 08/10] chore: adjusts memory limit --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index cb356505..0a4cc173 100644 --- a/composer.json +++ b/composer.json @@ -73,7 +73,7 @@ "test:refacto": "rector --dry-run", "test:lint": "pint --test", "test:type:check": "phpstan analyse --ansi --memory-limit=-1 --debug", - "test:type:coverage": "php bin/pest --type-coverage --min=100", + "test:type:coverage": "php -d memory_limit=-1 bin/pest --type-coverage --min=100", "test:unit": "php bin/pest --colors=always --exclude-group=integration --compact", "test:inline": "php bin/pest --colors=always --configuration=phpunit.inline.xml", "test:parallel": "php bin/pest --colors=always --exclude-group=integration --parallel --processes=3", From 86d2191cae05a1545e2a41aa61d19c5f07f4658d Mon Sep 17 00:00:00 2001 From: Nuno Maduro Date: Thu, 22 Aug 2024 20:59:25 +0100 Subject: [PATCH 09/10] chore: refactor `TestClosureMustNotBeStatic` --- src/Exceptions/TestClosureMustNotBeStatic.php | 17 ++++++++++++++++- src/Factories/TestCaseFactory.php | 3 ++- tests/Unit/TestSuite.php | 4 +--- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/Exceptions/TestClosureMustNotBeStatic.php b/src/Exceptions/TestClosureMustNotBeStatic.php index 498b1913..b1f91e95 100644 --- a/src/Exceptions/TestClosureMustNotBeStatic.php +++ b/src/Exceptions/TestClosureMustNotBeStatic.php @@ -4,13 +4,28 @@ declare(strict_types=1); namespace Pest\Exceptions; +use InvalidArgumentException; use NunoMaduro\Collision\Contracts\RenderlessEditor; use NunoMaduro\Collision\Contracts\RenderlessTrace; +use Pest\Factories\TestCaseMethodFactory; use Symfony\Component\Console\Exception\ExceptionInterface; /** * @internal */ -final class TestClosureMustNotBeStatic extends \InvalidArgumentException implements ExceptionInterface, RenderlessEditor, RenderlessTrace +final class TestClosureMustNotBeStatic extends InvalidArgumentException implements ExceptionInterface, RenderlessEditor, RenderlessTrace { + /** + * Creates a new Exception instance. + */ + public function __construct(TestCaseMethodFactory $method) + { + parent::__construct( + sprintf( + 'Test closure must not be static. Please remove the `static` keyword from the `%s` method in `%s`.', + $method->description, + $method->filename + ) + ); + } } diff --git a/src/Factories/TestCaseFactory.php b/src/Factories/TestCaseFactory.php index 9e166a1f..6d4330e8 100644 --- a/src/Factories/TestCaseFactory.php +++ b/src/Factories/TestCaseFactory.php @@ -221,7 +221,8 @@ final class TestCaseFactory $method->closure instanceof \Closure && (new \ReflectionFunction($method->closure))->isStatic() ) { - throw new TestClosureMustNotBeStatic("The test `$method->description` closure must not be static in $method->filename."); + + throw new TestClosureMustNotBeStatic($method); } if (! $method->receivesArguments()) { diff --git a/tests/Unit/TestSuite.php b/tests/Unit/TestSuite.php index 73fde260..ee78587d 100644 --- a/tests/Unit/TestSuite.php +++ b/tests/Unit/TestSuite.php @@ -19,9 +19,7 @@ it('does not allow to add the same test description twice', function () { it('does not allow static closures', function () { $testSuite = new TestSuite(getcwd(), 'tests'); - $method = new TestCaseMethodFactory('foo', 'bar', static function () { - - }); + $method = new TestCaseMethodFactory('foo', 'bar', static function () {}); $testSuite->tests->set($method); })->throws( From 9ceb0834aeedb771318d86b6033faa0fdb7972f0 Mon Sep 17 00:00:00 2001 From: Nuno Maduro Date: Thu, 22 Aug 2024 21:07:39 +0100 Subject: [PATCH 10/10] chore: updates snapshots --- tests/.snapshots/success.txt | 3 ++- tests/Unit/TestSuite.php | 6 +----- tests/Visual/Parallel.php | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/.snapshots/success.txt b/tests/.snapshots/success.txt index 2fbed06e..3dde887c 100644 --- a/tests/.snapshots/success.txt +++ b/tests/.snapshots/success.txt @@ -1381,6 +1381,7 @@ PASS Tests\Unit\TestSuite ✓ it does not allow to add the same test description twice + ✓ it does not allow static closures ✓ it alerts users about tests with arguments but no input ✓ it can return an array of all test suite filenames @@ -1424,4 +1425,4 @@ WARN Tests\Visual\Version - visual snapshot of help command output - Tests: 2 deprecated, 4 warnings, 5 incomplete, 2 notices, 13 todos, 20 skipped, 1013 passed (2405 assertions) \ No newline at end of file + Tests: 2 deprecated, 4 warnings, 5 incomplete, 2 notices, 13 todos, 20 skipped, 1014 passed (2407 assertions) \ No newline at end of file diff --git a/tests/Unit/TestSuite.php b/tests/Unit/TestSuite.php index ee78587d..d35b4f29 100644 --- a/tests/Unit/TestSuite.php +++ b/tests/Unit/TestSuite.php @@ -24,11 +24,7 @@ it('does not allow static closures', function () { $testSuite->tests->set($method); })->throws( TestClosureMustNotBeStatic::class, - sprintf( - 'The test `%s` closure must not be static in %s.', - 'bar', - 'foo', - ) + 'Test closure must not be static. Please remove the `static` keyword from the `bar` method in `foo`.', ); it('alerts users about tests with arguments but no input', function () { diff --git a/tests/Visual/Parallel.php b/tests/Visual/Parallel.php index 29079723..3dad50db 100644 --- a/tests/Visual/Parallel.php +++ b/tests/Visual/Parallel.php @@ -16,7 +16,7 @@ $run = function () { test('parallel', function () use ($run) { expect($run('--exclude-group=integration')) - ->toContain('Tests: 1 deprecated, 4 warnings, 5 incomplete, 2 notices, 13 todos, 16 skipped, 998 passed (2358 assertions)') + ->toContain('Tests: 1 deprecated, 4 warnings, 5 incomplete, 2 notices, 13 todos, 16 skipped, 999 passed (2360 assertions)') ->toContain('Parallel: 3 processes'); })->skipOnWindows();