From 32881774d2ef1abf645c0d0e706dfa2855af5590 Mon Sep 17 00:00:00 2001 From: Nuno Maduro Date: Wed, 11 Sep 2024 00:40:41 +0100 Subject: [PATCH] fix: global `afterEach` being called twice --- src/Concerns/Testable.php | 12 +++-- src/Factories/TestCaseMethodFactory.php | 27 +++++++--- tests/.snapshots/success.txt | 3 +- tests/Hooks/AfterEachTest.php | 66 +++++++++++++++++++++++-- tests/Pest.php | 17 ++++--- tests/Visual/Parallel.php | 2 +- 6 files changed, 104 insertions(+), 23 deletions(-) diff --git a/src/Concerns/Testable.php b/src/Concerns/Testable.php index 9bd0e007..43b7785b 100644 --- a/src/Concerns/Testable.php +++ b/src/Concerns/Testable.php @@ -116,7 +116,7 @@ trait Testable self::$__latestIssues = $method->issues; self::$__latestPrs = $method->prs; $this->__describing = $method->describing; - $this->__test = $method->getClosure($this); + $this->__test = $method->getClosure(); } } @@ -240,6 +240,8 @@ trait Testable $method = TestSuite::getInstance()->tests->get(self::$__filename)->getMethod($this->name()); + $method->setUp($this); + $description = $method->description; if ($this->dataName()) { $description = str_contains((string) $description, ':dataset') @@ -298,6 +300,9 @@ trait Testable parent::tearDown(); TestSuite::getInstance()->test = null; + + $method = TestSuite::getInstance()->tests->get(self::$__filename)->getMethod($this->name()); + $method->tearDown($this); } } @@ -392,11 +397,12 @@ trait Testable fn (ReflectionParameter $reflectionParameter): string => $reflectionParameter->getName(), array_filter($testReflection->getParameters(), fn (ReflectionParameter $reflectionParameter): bool => ! $reflectionParameter->isOptional()), ); + if (array_diff($testParameterNames, $datasetParameterNames) === []) { return; } - if (isset($testParameterNames[0]) - && $suppliedParametersCount >= $requiredParametersCount) { + + if (isset($testParameterNames[0]) && $suppliedParametersCount >= $requiredParametersCount) { return; } diff --git a/src/Factories/TestCaseMethodFactory.php b/src/Factories/TestCaseMethodFactory.php index 7c2e4248..086a9f3e 100644 --- a/src/Factories/TestCaseMethodFactory.php +++ b/src/Factories/TestCaseMethodFactory.php @@ -118,9 +118,9 @@ final class TestCaseMethodFactory } /** - * Creates the test's closure. + * Sets the test's hooks, and runs any proxy to the test case. */ - public function getClosure(TestCase $concrete): Closure + public function setUp(TestCase $concrete): void { $concrete::flush(); // @phpstan-ignore-line @@ -128,14 +128,29 @@ final class TestCaseMethodFactory throw ShouldNotHappen::fromMessage('Description can not be empty.'); } - $closure = $this->closure; - $testCase = TestSuite::getInstance()->tests->get($this->filename); assert($testCase instanceof TestCaseFactory); $testCase->factoryProxies->proxy($concrete); $this->factoryProxies->proxy($concrete); + } + /** + * Flushes the test case. + */ + public function tearDown(TestCase $concrete): void + { + $concrete::flush(); // @phpstan-ignore-line + } + + /** + * Creates the test's closure. + */ + public function getClosure(): Closure + { + $closure = $this->closure; + $testCase = TestSuite::getInstance()->tests->get($this->filename); + assert($testCase instanceof TestCaseFactory); $method = $this; return function (...$arguments) use ($testCase, $method, $closure): mixed { // @phpstan-ignore-line @@ -209,10 +224,8 @@ final class TestCaseMethodFactory $attributesCode public function $methodName(...\$arguments) { - \$test = \Pest\TestSuite::getInstance()->tests->get(self::\$__filename)->getMethod(\$this->name())->getClosure(\$this); - return \$this->__runTest( - \$test, + \$this->__test, ...\$arguments, ); } diff --git a/tests/.snapshots/success.txt b/tests/.snapshots/success.txt index 114eae10..0c5eda59 100644 --- a/tests/.snapshots/success.txt +++ b/tests/.snapshots/success.txt @@ -1305,6 +1305,7 @@ ✓ it executes tests in the Helpers directory PASS Tests\Hooks\AfterEachTest + ✓ nested → nested afterEach execution order ✓ global afterEach execution order PASS Tests\Hooks\BeforeEachTest @@ -1573,4 +1574,4 @@ WARN Tests\Visual\Version - visual snapshot of help command output - Tests: 2 deprecated, 4 warnings, 5 incomplete, 2 notices, 17 todos, 28 skipped, 1088 passed (2615 assertions) \ No newline at end of file + Tests: 2 deprecated, 4 warnings, 5 incomplete, 2 notices, 17 todos, 28 skipped, 1089 passed (2637 assertions) \ No newline at end of file diff --git a/tests/Hooks/AfterEachTest.php b/tests/Hooks/AfterEachTest.php index db1fa6b3..bbcf98ce 100644 --- a/tests/Hooks/AfterEachTest.php +++ b/tests/Hooks/AfterEachTest.php @@ -1,23 +1,79 @@ ith = 0; +}); + pest()->afterEach(function () { expect($this) ->toHaveProperty('ith') ->and($this->ith) - ->toBe(1); + ->toBe(3); - $this->ith = 2; + $this->ith++; +}); + +pest()->afterEach(function () { + expect($this) + ->toHaveProperty('ith') + ->and($this->ith) + ->toBe(4); + + $this->ith++; }); afterEach(function () { expect($this) ->toHaveProperty('ith') ->and($this->ith) - ->toBe(2); + ->toBe(5); + + $this->ith++; +}); + +describe('nested', function () { + afterEach(function () { + expect($this) + ->toHaveProperty('ith') + ->and($this->ith) + ->toBe(6); + + $this->ith++; + }); + + test('nested afterEach execution order', function () { + expect($this) + ->toHaveProperty('ith') + ->and($this->ith) + ->toBe(0); + + $this->ith++; + }); + + afterEach(function () { + expect($this) + ->toHaveProperty('ith') + ->and($this->ith) + ->toBe(7); + + $this->ith++; + }); +}); + +afterEach(function () { + expect($this) + ->toHaveProperty('ith') + ->and($this->ith) + ->toBeBetween(6, 8); + + $this->ith++; }); test('global afterEach execution order', function () { expect($this) - ->not() - ->toHaveProperty('ith'); + ->toHaveProperty('ith') + ->and($this->ith) + ->toBe(0); + + $this->ith++; }); diff --git a/tests/Pest.php b/tests/Pest.php index 69eddbdf..a938fc7e 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -32,7 +32,12 @@ pest() $_SERVER['globalHook']->calls->beforeAll++; }) ->afterEach(function () { - $this->ith = 0; + if (! isset($this->ith)) { + return; + } + + assert($this->ith === 1, 'Expected $this->ith to be 1, but got '.$this->ith); + $this->ith++; }) ->afterAll(function () { $_SERVER['globalHook']->afterAll = 0; @@ -57,12 +62,12 @@ pest()->in('Hooks') $_SERVER['globalHook']->beforeAll = 1; }) ->afterEach(function () { - expect($this) - ->toHaveProperty('ith') - ->and($this->ith) - ->toBe(0); + if (! isset($this->ith)) { + return; + } - $this->ith = 1; + assert($this->ith === 2, 'Expected $this->ith to be 1, but got '.$this->ith); + $this->ith++; }) ->afterAll(function () { expect($_SERVER['globalHook']) diff --git a/tests/Visual/Parallel.php b/tests/Visual/Parallel.php index 8744afb0..c99f2252 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: 2 deprecated, 4 warnings, 5 incomplete, 2 notices, 17 todos, 19 skipped, 1078 passed (2591 assertions)') + ->toContain('Tests: 2 deprecated, 4 warnings, 5 incomplete, 2 notices, 17 todos, 19 skipped, 1079 passed (2613 assertions)') ->toContain('Parallel: 3 processes'); })->skipOnWindows();